Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4638 implement es256 #4705

Draft
wants to merge 39 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
b85ce87
Update CMAKE to be compatible with versions about 3.5
bak-minsu Mar 17, 2024
b51af27
Implement base ACME Algorithm class
bak-minsu Mar 17, 2024
679063e
Remove isSupported as it was not needed
bak-minsu Mar 17, 2024
08510ef
Implement ACMEAlgorithm in ACMEEngine
bak-minsu Mar 17, 2024
cecb00e
Add Author
bak-minsu Mar 17, 2024
9d8ea30
Move RFC comment
bak-minsu Mar 17, 2024
5be877e
Move web app error back to ACMEengine
bak-minsu Mar 17, 2024
c5c23fb
Remove unused imports
bak-minsu Mar 17, 2024
3ba8594
Merge branch 'master' into 4638-implement-ES256
bak-minsu Mar 17, 2024
339a7ea
Simplify ACME Algorithm string conversion
bak-minsu Mar 22, 2024
f90f2b1
Add new line
bak-minsu Mar 22, 2024
8126c16
Specify an exact argument fromString should throw
bak-minsu Mar 22, 2024
79ce58d
Remove unsupported algorithms
bak-minsu Mar 24, 2024
79eb75e
Add test for ES256
bak-minsu Mar 24, 2024
8f1fb62
Check against the correct serial number
bak-minsu Mar 24, 2024
dc6ba9e
Correct parameters for openssl csr generation
bak-minsu Mar 26, 2024
f113955
Add line separator
bak-minsu Mar 26, 2024
87d5d0e
Add install instructions for openssl
bak-minsu Mar 27, 2024
ba81084
Move instructions to near where they're used
bak-minsu Mar 27, 2024
1108684
Mirror tests for RS256
bak-minsu Mar 27, 2024
6a31b85
Remove openssl install step, as it is already inside client's container
bak-minsu Mar 28, 2024
a0f8e74
Add verbosity to open ssl command
bak-minsu Mar 28, 2024
d74ce1d
Replace -nodes for -noenc
bak-minsu Mar 28, 2024
6d74573
Insert pipe character at the start of command
bak-minsu Mar 28, 2024
7170ccd
Update count checks
bak-minsu Mar 28, 2024
69a3a4b
Move algorithm tests to its own file
bak-minsu Mar 28, 2024
2b83e79
Return certbot test to its original form
bak-minsu Mar 28, 2024
6f70a80
Ensure post test counts are valid
bak-minsu Mar 28, 2024
ad8910e
Add TODO for HMAC algorithms
bak-minsu Mar 28, 2024
c36af7d
Remove duplicate tests
bak-minsu Mar 28, 2024
b6c38ea
Merge branch 'dogtagpki:master' into 4638-implement-ES256
bak-minsu Mar 28, 2024
2c3f163
Add verbose flag
bak-minsu Mar 29, 2024
8a39298
Configure cerbot to use correct signature algorithm
bak-minsu Mar 29, 2024
c686329
Set domain for certbot
bak-minsu Mar 29, 2024
a5d7ce9
Add line separator
bak-minsu Mar 29, 2024
248266e
Add RS256
bak-minsu Mar 29, 2024
01931d7
Add a revocation step
bak-minsu Mar 29, 2024
63e3184
Create duplicate cert
bak-minsu Mar 29, 2024
57552c4
Use standard RFC naming
bak-minsu Mar 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions .github/workflows/acme-certbot-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,15 @@ jobs:
- name: Install certbot in client container
run: docker exec client dnf install -y certbot

- name: Create CSR for ES256 Test
run: docker exec client openssl req \
--outform PEM \
--out ES256.csr \
--newkey ec \
--keyout ES256.key \
--sha256 \
--subj '/CN=es256.example.com'

- name: Register ACME account
run: |
docker exec client certbot register \
Expand Down Expand Up @@ -352,6 +361,64 @@ jobs:
sed -n 's/^ *Subject DN: *\(.*\)/\1/p' output > actual
diff expected actual

- name: Enroll using ES256 CSR
run: |
docker exec client certbot certonly \
--server http://pki.example.com:8080/acme/directory \
--key-path ES256.key \
--csr ES256.csr \
--standalone \
--non-interactive

- name: Check ES256 client locally
run: |
docker exec client pki client-cert-import \
--cert /etc/letsencrypt/live/es256.example.com/fullchain.pem \
es256

# store serial number
docker exec client pki nss-cert-show es256 | tee output
sed -n 's/^ *Serial Number: *\(.*\)/\1/p' output > es256.txt

# subject should be CN=es256.example.com
echo "CN=es256.example.com" > expected
sed -n 's/^ *Subject DN: *\(.*\)/\1/p' output > actual
diff expected actual

- name: Check ES256 Registration
run: |
docker exec pki pki ca-cert-find | tee output

# there should be 8 certs
echo "8" > expected
grep "Serial Number:" output | wc -l > actual
diff expected actual

# check client cert
SERIAL=$(cat rs256.txt)
docker exec pki pki ca-cert-show $SERIAL | tee output

# subject should be CN=client.example.com
echo "CN=client.example.com" > expected
sed -n 's/^ *Subject DN: *\(.*\)/\1/p' output > actual
diff expected actual

- name: Check ACME certs after enrollment
run: |
docker exec ds ldapsearch \
-H ldap://ds.example.com:3389 \
-D "cn=Directory Manager" \
-w Secret.123 \
-b ou=certificates,dc=acme,dc=pki,dc=example,dc=com \
-s one \
-o ldif_wrap=no \
-LLL | tee output

# there should be no certs (they are stored in CA)
echo "0" > expected
grep "^dn:" output | wc -l > actual
diff expected actual

- name: Renew client cert
run: |
docker exec client certbot renew \
Expand Down
8 changes: 4 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Required cmake version
cmake_minimum_required(VERSION 3.0.0)
cmake_minimum_required(VERSION 3.5)

project(pki)

Expand Down Expand Up @@ -35,9 +35,9 @@ if (NOT DEFINED THEME)
set(THEME "dogtag")
endif(NOT DEFINED THEME)

string(REGEX REPLACE "^([0-9]+).*" "\\1" APPLICATION_VERSION_MAJOR ${VERSION})
string(REGEX REPLACE "^[0-9]+\\.([0-9]+).*" "\\1" APPLICATION_VERSION_MINOR ${VERSION})
string(REGEX REPLACE "^[0-9]+\\.[0-9]+\\.([0-9]+).*" "\\1" APPLICATION_VERSION_PATCH ${VERSION})
string(REGEX REPLACE "^([0-9]+).*" "\\1" "APPLICATION_VERSION_MAJOR" "${VERSION}")
string(REGEX REPLACE "^[0-9]+\\.([0-9]+).*" "\\1" "APPLICATION_VERSION_MINOR" "${VERSION}")
string(REGEX REPLACE "^[0-9]+\\.[0-9]+\\.([0-9]+).*" "\\1" "APPLICATION_VERSION_PATCH" "${VERSION}")

set(APP_SERVER "tomcat-9.0" CACHE STRING "Application server")

Expand Down
112 changes: 59 additions & 53 deletions base/acme/src/main/java/org/dogtagpki/acme/server/ACMEEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.StringUtils;
import org.dogtagpki.acme.ACMEAccount;
import org.dogtagpki.acme.ACMEAlgorithm;
import org.dogtagpki.acme.ACMEAuthorization;
import org.dogtagpki.acme.ACMEError;
import org.dogtagpki.acme.ACMEIdentifier;
Expand Down Expand Up @@ -375,39 +376,37 @@ public void initMonitors(String filename) throws Exception {
}

// the default class just sends the default config values
Class<? extends ACMEEngineConfigSource> configSourceClass
= ACMEEngineConfigDefaultSource.class;
Class<? extends ACMEEngineConfigSource> configSourceClass = ACMEEngineConfigDefaultSource.class;

String className = monitorsConfig.getProperty("engine.class");
if (className != null && !className.isEmpty()) {
configSourceClass =
(Class<ACMEEngineConfigSource>) Class.forName(className);
configSourceClass = (Class<ACMEEngineConfigSource>) Class.forName(className);
}
engineConfigSource = configSourceClass.getDeclaredConstructor().newInstance();

// We pass to the ConfigSource only the callbacks needed to set
// the configuration (Consumer<T>). This abstraction ensures the
// the configuration (Consumer<T>). This abstraction ensures the
// ConfigSource has no direct access to the ACMEEngine instance.

engineConfigSource.setEnabledConsumer(new Consumer<Boolean>() {
@Override public void accept(Boolean b) {
@Override
public void accept(Boolean b) {
config.setEnabled(b);
logger.info(
"ACME service is "
+ (b ? "enabled" : "DISABLED")
+ " by configuration"
);
"ACME service is "
+ (b ? "enabled" : "DISABLED")
+ " by configuration");
}
});

engineConfigSource.setWildcardConsumer(new Consumer<Boolean>() {
@Override public void accept(Boolean b) {
@Override
public void accept(Boolean b) {
config.getPolicyConfig().setEnableWildcards(b);
logger.info(
"ACME wildcard issuance is "
+ (b ? "enabled" : "DISABLED")
+ " by configuration"
);
"ACME wildcard issuance is "
+ (b ? "enabled" : "DISABLED")
+ " by configuration");
}
});

Expand Down Expand Up @@ -455,7 +454,7 @@ public void start() throws Exception {
loadConfig(acmeConfDir + File.separator + "engine.conf");

Boolean noncePersistent = config.getNoncesPersistent();
this.noncesPersistent = noncePersistent != null ? noncePersistent : false;
this.noncesPersistent = noncePersistent != null ? noncePersistent : false;

initRandomGenerator();
initMetadata(acmeConfDir + File.separator + "metadata.conf");
Expand All @@ -470,7 +469,8 @@ public void start() throws Exception {
}

public void shutdownDatabase() throws Exception {
if (database == null) return;
if (database == null)
return;

database.close();
database = null;
Expand All @@ -486,28 +486,32 @@ public void shutdownValidators() throws Exception {
}

public void shutdownIssuer() throws Exception {
if (issuer == null) return;
if (issuer == null)
return;

issuer.close();
issuer = null;
}

public void shutdownScheduler() throws Exception {
if (scheduler == null) return;
if (scheduler == null)
return;

scheduler.shutdown();
scheduler = null;
}

public void shutdownMonitors() throws Exception {
if (engineConfigSource == null) return;
if (engineConfigSource == null)
return;

engineConfigSource.shutdown();
engineConfigSource = null;
}

public void shutdownRealm() throws Exception {
if (realm == null) return;
if (realm == null)
return;

realm.stop();
realm = null;
Expand Down Expand Up @@ -604,40 +608,38 @@ public void removeExpiredRecords(Date currentTime) throws Exception {

public void validateJWS(JWS jws, String alg, JWK jwk) throws Exception {

// TODO: support other algorithms

Signature signer;
PublicKey publicKey;
ACMEAlgorithm acmeAlgo;

if ("RS256".equals(alg)) {

signer = Signature.getInstance("SHA256withRSA", "Mozilla-JSS");

String kty = jwk.getKty();
KeyFactory keyFactory = KeyFactory.getInstance(kty, "Mozilla-JSS");

String n = jwk.getN();
BigInteger modulus = new BigInteger(1, Base64.decodeBase64(n));

String e = jwk.getE();
BigInteger publicExponent = new BigInteger(1, Base64.decodeBase64(e));

RSAPublicKeySpec keySpec = new RSAPublicKeySpec(modulus, publicExponent);
publicKey = keyFactory.generatePublic(keySpec);

} else {
try {
acmeAlgo = ACMEAlgorithm.fromString(alg);
} catch(Exception e) {
ResponseBuilder builder = Response.status(Response.Status.BAD_REQUEST);
builder.type("application/problem+json");

ACMEError error = new ACMEError();
error.setType("urn:ietf:params:acme:error:badSignatureAlgorithm");
error.setDetail("Signature of type " + alg + " not supported\n" +
"Try again with RS256.");
error.setDetail("Signature of type " + alg + " not supported");
builder.entity(error);

throw new WebApplicationException(builder.build());
}

signer = Signature.getInstance(acmeAlgo.getJCA(), "Mozilla-JSS");

String kty = jwk.getKty();
KeyFactory keyFactory = KeyFactory.getInstance(kty, "Mozilla-JSS");

String n = jwk.getN();
BigInteger modulus = new BigInteger(1, Base64.decodeBase64(n));

String e = jwk.getE();
BigInteger publicExponent = new BigInteger(1, Base64.decodeBase64(e));

RSAPublicKeySpec keySpec = new RSAPublicKeySpec(modulus, publicExponent);
publicKey = keyFactory.generatePublic(keySpec);

validateJWS(jws, signer, publicKey);
}

Expand Down Expand Up @@ -791,7 +793,9 @@ public void addOrder(ACMEAccount account, ACMEOrder order) throws Exception {
database.addOrder(order);
}

enum CheckOrderResult { ORDER_ACCOUNT_MISMATCH , ORDER_EXPIRED , ORDER_ACCESS_OK, ORDER_NULL}
enum CheckOrderResult {
ORDER_ACCOUNT_MISMATCH, ORDER_EXPIRED, ORDER_ACCESS_OK, ORDER_NULL
}

public CheckOrderResult checkOrder(ACMEAccount account, ACMEOrder order) {

Expand Down Expand Up @@ -900,8 +904,8 @@ public void validateCSR(ACMEAccount account, ACMEOrder order, PKCS10 pkcs10) thr
if (!unauthorizedDNSNames.isEmpty()) {
// TODO: generate proper exception
throw new Exception(
"Unauthorized DNS names: "
+ StringUtils.join(unauthorizedDNSNames, ", "));
"Unauthorized DNS names: "
+ StringUtils.join(unauthorizedDNSNames, ", "));
}

// check for authorized names missing from CSR
Expand All @@ -910,8 +914,8 @@ public void validateCSR(ACMEAccount account, ACMEOrder order, PKCS10 pkcs10) thr
if (!extraAuthorizedDNSNames.isEmpty()) {
// TODO: generate proper exception
throw new Exception(
"Missing DNS names from order: "
+ StringUtils.join(extraAuthorizedDNSNames, ", "));
"Missing DNS names from order: "
+ StringUtils.join(extraAuthorizedDNSNames, ", "));
}

// TODO: validate other things in CSR
Expand All @@ -925,9 +929,9 @@ public void validateRevocation(ACMEAccount account, ACMERevocation revocation) t
//
// The server MUST consider at least the following accounts authorized
// for a given certificate:
// - the account that issued the certificate.
// - an account that holds authorizations for all of the identifiers in
// the certificate.
// - the account that issued the certificate.
// - an account that holds authorizations for all of the identifiers in
// the certificate.

Date now = new Date();
String certBase64 = revocation.getCertificate();
Expand Down Expand Up @@ -961,11 +965,13 @@ public void validateRevocation(ACMEAccount account, ACMERevocation revocation) t
Collection<ACMEIdentifier> identifiers = getCertIdentifiers(cert);

if (identifiers.isEmpty()) {
/* Protect against vacuous authorisation. If there are no
/*
* Protect against vacuous authorisation. If there are no
* identifiers, it could be e.g. a user or CA certificate.
* Without this check that there are at least /some/ identifiers
* to authorise, every account would be vacuously authorised
* to revoke it. */
* to revoke it.
*/
throw new Exception("Certificate has no ACME identifiers.");
}

Expand Down Expand Up @@ -1010,7 +1016,7 @@ public void validateRevocation(ACMEAccount account, ACMERevocation revocation) t
logger.info("Account has no authorizations for:");
for (ACMEIdentifier identifier : identifiers) {
logger.info("- " + identifier.getType() + ": " + identifier.getValue());
}
}

// TODO: generate proper exception
throw new Exception("Account has no authorizations for " + identifiers);
Expand Down
38 changes: 38 additions & 0 deletions base/common/src/main/java/org/dogtagpki/acme/ACMEAlgorithm.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.dogtagpki.acme;

/**
* @author Minsu Park
*/

public enum ACMEAlgorithm {
// RFC 7518 Appendix A.1
// Digital Signature/MAC Algorithm Identifier Cross-Reference

// Dogtag's JSS has slightly different algorithm names
// than what is in RFC 7518
RS256("RS256", "SHA256withRSA"),
RS384("RS384", "SHA384withRSA"),
RS512("RS512", "SHA512withRSA"),
ES256("ES256", "SHA256withEC"),
ES384("ES384", "SHA384withEC"),
ES512("ES512", "SHA512withEC"),
PS256("PS256", "SHA256withRSA/PSS"),
PS384("PS384", "SHA384withRSA/PSS"),
PS512("PS512", "SHA512withRSA/PSS");

private String alg;
private String jca;

private ACMEAlgorithm(String alg, String jca) {
this.alg = alg;
this.jca = jca;
}

public String getJCA() {
return jca;
}

public static ACMEAlgorithm fromString(String alg) throws IllegalArgumentException {
return ACMEAlgorithm.valueOf(alg.toUpperCase());
}
}
Loading