Skip to content

Commit

Permalink
Fix a prompt trust manager issue
Browse files Browse the repository at this point in the history 
Fixed an issue in the prompt trust manager that could cause it to
display an incorrect warning for some certificates with a basic
constraints extension with a path length constraint.
  • Loading branch information
@dirmgr
dirmgr committed Nov 13, 2019
1 parent ea68d05 commit 5e7e907
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 29 deletions.
7 changes: 7 additions & 0 deletions docs/release-notes.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ <h3>Version 4.0.13</h3>
<br><br>
</li>

<li>
Fixed an issue in the prompt trust manager that could cause it to display an
incorrect warning for some certificates with a basic constraints extension with
a path length constraint.
<br><br>
</li>

<li>
Fixed an issue in the encoding and decoding of X.509 certificates that
included an authority key extension that included the optional
Expand Down
13 changes: 9 additions & 4 deletions messages/unboundid-ldapsdk-ssl.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,15 @@ WARN_PROMPT_PROCESSOR_NO_BC_EXTENSION=WARNING: Issuer certificate ''{0}'' \
WARN_PROMPT_PROCESSOR_BC_NOT_CA=WARNING: Issuer certificate ''{0}'' has a \
basic constraints extension that indicates the certificate should not be \
trusted as an issuer certificate.
WARN_PROMPT_PROCESSOR_BC_PATH_LENGTH_EXCEEDED=WARNING: Issuer certificate \
''{0}'' has a basic constraints extension that indicates a valid \
certificate chain should contain more than {1,number,0} certificates, but \
the presented certificate chain has {2,number,0} certificates.
WARN_PROMPT_PROCESSOR_BC_DISALLOWED_INTERMEDIATE=WARNING: Issuer certificate \
''{0}'' has a basic constraints extension that indicates there should not \
be any intermediate CA certificates between it and the end entity \
certificate, but one or more intermediate CA certificates were found ahead \
of it in the chain.
WARN_PROMPT_PROCESSOR_BC_TOO_MANY_INTERMEDIATES=WARNING: Issuer certificate \
''{0}'' has a basic constraints extension that indicates there should be at \
most {1,number,0} intermediate CA certificate(s) between it and the end \
entity certificate, but {2,number,0} intermediate CA certificates were found.
WARN_PROMPT_PROCESSOR_KU_NO_KEY_CERT_SIGN=WARNING: Issuer certificate \
''{0}'' has a key usage extension that does not include the keyCertSign \
usage. This indicates that the certificate should not be trusted as an \
Expand Down
18 changes: 14 additions & 4 deletions src/com/unboundid/util/ssl/PromptTrustManagerProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,21 @@ else if (! bc.isCA())
chain[i].getSubjectDN()));
}
else if ((bc.getPathLengthConstraint() != null) &&
(bc.getPathLengthConstraint() < chain.length))
((i-1) > bc.getPathLengthConstraint()))
{
warningMessages.add(WARN_PROMPT_PROCESSOR_BC_PATH_LENGTH_EXCEEDED.get(
chain[i].getSubjectDN(), bc.getPathLengthConstraint(),
chain.length));
if (bc.getPathLengthConstraint() == 0)
{
warningMessages.add(
WARN_PROMPT_PROCESSOR_BC_DISALLOWED_INTERMEDIATE.get(
chain[i].getSubjectDN()));
}
else
{
warningMessages.add(
WARN_PROMPT_PROCESSOR_BC_TOO_MANY_INTERMEDIATES.get(
chain[i].getSubjectDN(), bc.getPathLengthConstraint(),
(i-1)));
}
}

if ((ku != null) && (! ku.isKeyCertSignBitSet()))
Expand Down
174 changes: 153 additions & 21 deletions tests/unit/src/com/unboundid/util/ssl/PromptTrustManagerProcessorTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,11 +887,20 @@ public void testIssuerBasicConstraintsPathLengthExceeded()
// during testing.
final File tempDir = createTempDir();

final String caCertificateAlias = "ca-cert";
final String caKeyStorePath = new File(tempDir,
caCertificateAlias + "-keystore.jks").getAbsolutePath();
final String caCertificatePath =
new File(tempDir, caCertificateAlias + ".cert").getAbsolutePath();
final String rootCACertificateAlias = "root-ca-cert";
final String rootCAKeyStorePath = new File(tempDir,
rootCACertificateAlias + "-keystore.jks").getAbsolutePath();
final String rootCACertificatePath =
new File(tempDir, rootCACertificateAlias + ".cert").getAbsolutePath();

final String intermediateCACertificateAlias = "intermediate-ca-cert";
final String intermediateCAKeyStorePath = new File(tempDir,
intermediateCACertificateAlias + "-keystore.jks").getAbsolutePath();
final String intermediateCACSRPath = new File(tempDir,
intermediateCACertificateAlias + ".csr").getAbsolutePath();
final String intermediateCACertificatePath =
new File(tempDir, intermediateCACertificateAlias + ".cert").
getAbsolutePath();

final String serverCertificateAlias = "server-cert";
final String serverKeyStorePath = new File(tempDir,
Expand All @@ -901,34 +910,35 @@ public void testIssuerBasicConstraintsPathLengthExceeded()
final String serverCertificatePath =
new File(tempDir, serverCertificateAlias + ".cert").getAbsolutePath();

// Create a JKS keystore with just a CA certificate.
// Create a JKS keystore with just a root CA certificate.
manageCertificates(
"generate-self-signed-certificate",
"--keystore", caKeyStorePath,
"--keystore", rootCAKeyStorePath,
"--keystore-password", "password",
"--keystore-type", "JKS",
"--alias", caCertificateAlias,
"--alias", rootCACertificateAlias,
"--subject-dn", "CN=Example Root CA,O=Example Corporation,C=US",
"--days-valid", "7300",
"--key-algorithm", "RSA",
"--key-size-bits", "2048",
"--signature-algorithm", "SHA256withRSA",
"--subject-alternative-name-email-address", "[email protected]",
"--basic-constraints-is-ca", "true",
"--basic-constraints-maximum-path-length", "1",
"--basic-constraints-maximum-path-length", "0",
"--key-usage", "key-cert-sign",
"--display-keytool-command");
manageCertificates(
"export-certificate",
"--keystore", caKeyStorePath,
"--keystore", rootCAKeyStorePath,
"--keystore-password", "password",
"--alias", caCertificateAlias,
"--alias", rootCACertificateAlias,
"--output-format", "PEM",
"--output-file", caCertificatePath,
"--output-file", rootCACertificatePath,
"--display-keytool-command");


// Create a JKS keystore with a server certificate that is signed by the CA.
// Create a JKS keystore with a server certificate that is signed by the
// root CA.
manageCertificates(
"generate-certificate-signing-request",
"--output-file", serverCSRPath,
Expand All @@ -955,17 +965,17 @@ public void testIssuerBasicConstraintsPathLengthExceeded()
"--request-input-file", serverCSRPath,
"--certificate-output-file", serverCertificatePath,
"--output-format", "PEM",
"--keystore", caKeyStorePath,
"--keystore", rootCAKeyStorePath,
"--keystore-password", "password",
"--signing-certificate-alias", caCertificateAlias,
"--signing-certificate-alias", rootCACertificateAlias,
"--days-valid", "365",
"--include-requested-extensions",
"--no-prompt",
"--display-keytool-command");
manageCertificates(
"import-certificate",
"--certificate-file", serverCertificatePath,
"--certificate-file", caCertificatePath,
"--certificate-file", rootCACertificatePath,
"--keystore", serverKeyStorePath,
"--keystore-password", "password",
"--alias", serverCertificateAlias,
Expand All @@ -974,20 +984,19 @@ public void testIssuerBasicConstraintsPathLengthExceeded()


// Load the keystore and get the certificate chain.
final KeyStore keystore = KeyStore.getInstance("JKS");
KeyStore keystore = KeyStore.getInstance("JKS");
try (FileInputStream inputStream = new FileInputStream(serverKeyStorePath))
{
keystore.load(inputStream, "password".toCharArray());
}

final Certificate[] javaChain =
Certificate[] javaChain =
keystore.getCertificateChain(serverCertificateAlias);
final X509Certificate[] ldapSDKChain =
PromptTrustManager.convertChain(javaChain);
X509Certificate[] ldapSDKChain = PromptTrustManager.convertChain(javaChain);


// Invoke the shouldPrompt method and examine the result.
final ObjectPair<Boolean,List<String>> promptResult =
ObjectPair<Boolean,List<String>> promptResult =
PromptTrustManagerProcessor.shouldPrompt(
PromptTrustManager.getCacheKey(javaChain[0]),
ldapSDKChain, true, true, Collections.<String,Boolean>emptyMap(),
Expand All @@ -996,6 +1005,129 @@ public void testIssuerBasicConstraintsPathLengthExceeded()
assertNotNull(promptResult.getFirst());
assertEquals(promptResult.getFirst(), Boolean.TRUE);

assertNotNull(promptResult.getSecond());
assertTrue(promptResult.getSecond().isEmpty());


// Create a JKS keystore with an intermediate CA certificate that is signed
// by the root CA.
manageCertificates(
"generate-certificate-signing-request",
"--output-file", intermediateCACSRPath,
"--keystore", intermediateCAKeyStorePath,
"--keystore-password", "password",
"--keystore-type", "JKS",
"--alias", intermediateCACertificateAlias,
"--subject-dn",
"CN=Example Intermediate CA,O=Example Corporation,C=US",
"--key-algorithm", "RSA",
"--key-size-bits", "2048",
"--signature-algorithm", "SHA256withRSA",
"--basic-constraints-is-ca", "true",
"--basic-constraints-maximum-path-length", "0",
"--subject-alternative-name-dns", "ldap.example.com",
"--subject-alternative-name-dns", "ldap",
"--subject-alternative-name-dns", "ds.example.com",
"--subject-alternative-name-dns", "ds",
"--subject-alternative-name-dns", "localhost",
"--subject-alternative-name-ip-address", "127.0.0.1",
"--subject-alternative-name-ip-address", "::1",
"--extended-key-usage", "server-auth",
"--extended-key-usage", "client-auth",
"--display-keytool-command");
manageCertificates(
"sign-certificate-signing-request",
"--request-input-file", intermediateCACSRPath,
"--certificate-output-file", intermediateCACertificatePath,
"--output-format", "PEM",
"--keystore", rootCAKeyStorePath,
"--keystore-password", "password",
"--signing-certificate-alias", rootCACertificateAlias,
"--days-valid", "365",
"--include-requested-extensions",
"--no-prompt",
"--display-keytool-command");
manageCertificates(
"import-certificate",
"--certificate-file", intermediateCACertificatePath,
"--certificate-file", rootCACertificatePath,
"--keystore", intermediateCAKeyStorePath,
"--keystore-password", "password",
"--alias", intermediateCACertificateAlias,
"--no-prompt",
"--display-keytool-command");


// Delete the server certificate keystore and recreate it with a server
// certificate that is signed by the intermediate CA.
assertTrue(new File(serverKeyStorePath).delete());
assertTrue(new File(serverCertificatePath).delete());
assertTrue(new File(serverCSRPath).delete());
manageCertificates(
"generate-certificate-signing-request",
"--output-file", serverCSRPath,
"--keystore", serverKeyStorePath,
"--keystore-password", "password",
"--keystore-type", "JKS",
"--alias", serverCertificateAlias,
"--subject-dn", "CN=ldap.example.com,O=Example Corporation,C=US",
"--key-algorithm", "RSA",
"--key-size-bits", "2048",
"--signature-algorithm", "SHA256withRSA",
"--subject-alternative-name-dns", "ldap.example.com",
"--subject-alternative-name-dns", "ldap",
"--subject-alternative-name-dns", "ds.example.com",
"--subject-alternative-name-dns", "ds",
"--subject-alternative-name-dns", "localhost",
"--subject-alternative-name-ip-address", "127.0.0.1",
"--subject-alternative-name-ip-address", "::1",
"--extended-key-usage", "server-auth",
"--extended-key-usage", "client-auth",
"--display-keytool-command");
manageCertificates(
"sign-certificate-signing-request",
"--request-input-file", serverCSRPath,
"--certificate-output-file", serverCertificatePath,
"--output-format", "PEM",
"--keystore", intermediateCAKeyStorePath,
"--keystore-password", "password",
"--signing-certificate-alias", intermediateCACertificateAlias,
"--days-valid", "365",
"--include-requested-extensions",
"--no-prompt",
"--display-keytool-command");
manageCertificates(
"import-certificate",
"--certificate-file", serverCertificatePath,
"--certificate-file", intermediateCACertificatePath,
"--certificate-file", rootCACertificatePath,
"--keystore", serverKeyStorePath,
"--keystore-password", "password",
"--alias", serverCertificateAlias,
"--no-prompt",
"--display-keytool-command");


// Load the keystore and get the certificate chain.
keystore = KeyStore.getInstance("JKS");
try (FileInputStream inputStream = new FileInputStream(serverKeyStorePath))
{
keystore.load(inputStream, "password".toCharArray());
}

javaChain = keystore.getCertificateChain(serverCertificateAlias);
ldapSDKChain = PromptTrustManager.convertChain(javaChain);


// Invoke the shouldPrompt method and examine the result.
promptResult = PromptTrustManagerProcessor.shouldPrompt(
PromptTrustManager.getCacheKey(javaChain[0]),
ldapSDKChain, true, true, Collections.<String,Boolean>emptyMap(),
Collections.singletonList("ldap.example.com"));

assertNotNull(promptResult.getFirst());
assertEquals(promptResult.getFirst(), Boolean.TRUE);

assertNotNull(promptResult.getSecond());
assertFalse(promptResult.getSecond().isEmpty());
assertEquals(promptResult.getSecond().size(), 1);
Expand Down

0 comments on commit 5e7e907

Please sign in to comment.