Skip to content

Commit

Permalink
Add validation of authority certificates
Browse files Browse the repository at this point in the history 
Added validation of authority certificates prior
to reloading the SSL context.

Signed-off-by: Andrey Pleskach <[email protected]>
  • Loading branch information
@willyborankin
willyborankin committed Oct 28, 2024
1 parent 2e0e64f commit 70463f2
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 45 deletions.
108 changes: 73 additions & 35 deletions src/main/java/org/opensearch/security/ssl/SslContextHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

import io.netty.handler.ssl.SslContext;

import static java.util.function.Predicate.not;

public class SslContextHandler {

private SslContext sslContext;
Expand Down Expand Up @@ -60,6 +62,14 @@ SslContext sslContext() {
return sslContext;
}

public Stream<Certificate> authorityCertificates() {
return authorityCertificates(loadedCertificates);
}

Stream<Certificate> authorityCertificates(final List<Certificate> certificates) {
return certificates.stream().filter(not(Certificate::hasKey));
}

public Stream<Certificate> keyMaterialCertificates() {
return keyMaterialCertificates(loadedCertificates);
}
Expand All @@ -71,37 +81,62 @@ Stream<Certificate> keyMaterialCertificates(final List<Certificate> certificates
void reloadSslContext() throws CertificateException {
final var newCertificates = sslConfiguration.certificates();

if (sameCertificates(newCertificates)) {
return;
boolean hasChanges = false;

final var loadedAuthorityCertificates = authorityCertificates().collect(Collectors.toList());
final var loadedKeyMaterialCertificates = keyMaterialCertificates().collect(Collectors.toList());
final var newAuthorityCertificates = authorityCertificates(newCertificates).collect(Collectors.toList());
final var newKeyMaterialCertificates = keyMaterialCertificates(newCertificates).collect(Collectors.toList());

if (notSameCertificates(loadedAuthorityCertificates, newAuthorityCertificates)) {
hasChanges = true;
validateDates(newAuthorityCertificates);
}
validateNewCertificates(newCertificates, sslConfiguration.sslParameters().shouldValidateNewCertDNs());
invalidateSessions();
if (sslContext.isClient()) {
sslContext = sslConfiguration.buildClientSslContext(false);
} else {
sslContext = sslConfiguration.buildServerSslContext(false);

if (notSameCertificates(loadedKeyMaterialCertificates, newKeyMaterialCertificates)) {
hasChanges = true;
validateNewKeyMaterialCertificates(
loadedKeyMaterialCertificates,
newKeyMaterialCertificates,
sslConfiguration.sslParameters().shouldValidateNewCertDNs()
);
}
if (hasChanges) {
invalidateSessions();
if (sslContext.isClient()) {
sslContext = sslConfiguration.buildClientSslContext(false);
} else {
sslContext = sslConfiguration.buildServerSslContext(false);
}
loadedCertificates.clear();
loadedCertificates.addAll(newCertificates);
}
loadedCertificates.clear();
loadedCertificates.addAll(newCertificates);
}

private boolean sameCertificates(final List<Certificate> newCertificates) {
final Set<String> currentCertSignatureSet = keyMaterialCertificates().map(Certificate::x509Certificate)
private boolean notSameCertificates(final List<Certificate> loadedCertificates, final List<Certificate> newCertificates) {
final Set<String> currentCertSignatureSet = loadedCertificates.stream()
.map(Certificate::x509Certificate)
.map(X509Certificate::getSignature)
.map(s -> new String(s, StandardCharsets.UTF_8))
.collect(Collectors.toSet());
final Set<String> newCertSignatureSet = keyMaterialCertificates(newCertificates).map(Certificate::x509Certificate)
final Set<String> newCertSignatureSet = newCertificates.stream()
.map(Certificate::x509Certificate)
.map(X509Certificate::getSignature)
.map(s -> new String(s, StandardCharsets.UTF_8))
.collect(Collectors.toSet());
return currentCertSignatureSet.equals(newCertSignatureSet);
return !currentCertSignatureSet.equals(newCertSignatureSet);
}

private void validateSubjectDns(final List<Certificate> newCertificates) throws CertificateException {
final List<String> currentSubjectDNs = keyMaterialCertificates().map(Certificate::subject).sorted().collect(Collectors.toList());
final List<String> newSubjectDNs = keyMaterialCertificates(newCertificates).map(Certificate::subject)
.sorted()
.collect(Collectors.toList());
private void validateDates(final List<Certificate> newCertificates) throws CertificateException {
for (final var certificate : newCertificates) {
certificate.x509Certificate().checkValidity();
}
}

private void validateSubjectDns(final List<Certificate> loadedCertificates, final List<Certificate> newCertificates)
throws CertificateException {
final List<String> currentSubjectDNs = loadedCertificates.stream().map(Certificate::subject).sorted().collect(Collectors.toList());
final List<String> newSubjectDNs = newCertificates.stream().map(Certificate::subject).sorted().collect(Collectors.toList());
if (!currentSubjectDNs.equals(newSubjectDNs)) {
throw new CertificateException(
"New certificates do not have valid Subject DNs. Current Subject DNs "
Expand All @@ -112,11 +147,10 @@ private void validateSubjectDns(final List<Certificate> newCertificates) throws
}
}

private void validateIssuerDns(final List<Certificate> newCertificates) throws CertificateException {
final List<String> currentIssuerDNs = keyMaterialCertificates().map(Certificate::issuer).sorted().collect(Collectors.toList());
final List<String> newIssuerDNs = keyMaterialCertificates(newCertificates).map(Certificate::issuer)
.sorted()
.collect(Collectors.toList());
private void validateIssuerDns(final List<Certificate> loadedCertificates, final List<Certificate> newCertificates)
throws CertificateException {
final List<String> currentIssuerDNs = loadedCertificates.stream().map(Certificate::issuer).sorted().collect(Collectors.toList());
final List<String> newIssuerDNs = newCertificates.stream().map(Certificate::issuer).sorted().collect(Collectors.toList());
if (!currentIssuerDNs.equals(newIssuerDNs)) {
throw new CertificateException(
"New certificates do not have valid Issuer DNs. Current Issuer DNs: "
Expand All @@ -127,11 +161,14 @@ private void validateIssuerDns(final List<Certificate> newCertificates) throws C
}
}

private void validateSans(final List<Certificate> newCertificates) throws CertificateException {
final List<String> currentSans = keyMaterialCertificates().map(Certificate::subjectAlternativeNames)
private void validateSans(final List<Certificate> loadedCertificates, final List<Certificate> newCertificates)
throws CertificateException {
final List<String> currentSans = loadedCertificates.stream()
.map(Certificate::subjectAlternativeNames)
.sorted()
.collect(Collectors.toList());
final List<String> newSans = keyMaterialCertificates(newCertificates).map(Certificate::subjectAlternativeNames)
final List<String> newSans = newCertificates.stream()
.map(Certificate::subjectAlternativeNames)
.sorted()
.collect(Collectors.toList());
if (!currentSans.equals(newSans)) {
Expand All @@ -141,15 +178,16 @@ private void validateSans(final List<Certificate> newCertificates) throws Certif
}
}

private void validateNewCertificates(final List<Certificate> newCertificates, boolean shouldValidateNewCertDNs)
throws CertificateException {
for (final var certificate : newCertificates) {
certificate.x509Certificate().checkValidity();
}
private void validateNewKeyMaterialCertificates(
final List<Certificate> loadedCertificates,
final List<Certificate> newCertificates,
boolean shouldValidateNewCertDNs
) throws CertificateException {
validateDates(newCertificates);
if (shouldValidateNewCertDNs) {
validateSubjectDns(newCertificates);
validateIssuerDns(newCertificates);
validateSans(newCertificates);
validateSubjectDns(loadedCertificates, newCertificates);
validateIssuerDns(loadedCertificates, newCertificates);
validateSans(loadedCertificates, newCertificates);
}
}

Expand Down
21 changes: 15 additions & 6 deletions src/test/java/org/opensearch/security/ssl/CertificatesRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,30 @@ public KeyPair generateKeyPair() throws NoSuchAlgorithmException {

public X509CertificateHolder generateCaCertificate(final KeyPair parentKeyPair) throws IOException, NoSuchAlgorithmException,
OperatorCreationException {
return generateCaCertificate(parentKeyPair, generateSerialNumber());
final var startAndEndDate = generateStartAndEndDate();
return generateCaCertificate(parentKeyPair, generateSerialNumber(), startAndEndDate.v1(), startAndEndDate.v2());
}

public X509CertificateHolder generateCaCertificate(final KeyPair parentKeyPair, final BigInteger serialNumber) throws IOException,
NoSuchAlgorithmException, OperatorCreationException {
final var startAndEndDate = generateStartAndEndDate();
public X509CertificateHolder generateCaCertificate(final KeyPair parentKeyPair, final Instant startDate, final Instant endDate)
throws IOException, NoSuchAlgorithmException, OperatorCreationException {
return generateCaCertificate(parentKeyPair, generateSerialNumber(), startDate, endDate);
}

public X509CertificateHolder generateCaCertificate(
final KeyPair parentKeyPair,
final BigInteger serialNumber,
final Instant startDate,
final Instant endDate
) throws IOException, NoSuchAlgorithmException, OperatorCreationException {
// CS-SUPPRESS-SINGLE: RegexpSingleline Extension should only be used sparingly to keep implementations as generic as possible
return createCertificateBuilder(
DEFAULT_SUBJECT_NAME,
DEFAULT_SUBJECT_NAME,
parentKeyPair.getPublic(),
parentKeyPair.getPublic(),
serialNumber,
startAndEndDate.v1(),
startAndEndDate.v2()
startDate,
endDate
).addExtension(Extension.basicConstraints, true, new BasicConstraints(true))
.addExtension(Extension.keyUsage, true, new KeyUsage(KeyUsage.digitalSignature | KeyUsage.keyCertSign | KeyUsage.cRLSign))
.build(new JcaContentSignerBuilder("SHA256withRSA").setProvider(BOUNCY_CASTLE_PROVIDER).build(parentKeyPair.getPrivate()));
Expand Down
35 changes: 31 additions & 4 deletions src/test/java/org/opensearch/security/ssl/SslContextHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,34 @@ public void doesNothingIfCertificatesAreSame() throws Exception {
}

@Test
public void failsIfCertificatesHasInvalidDates() throws Exception {
public void failsIfAuthorityCertificateHasInvalidDates() throws Exception {
final var sslContextHandler = sslContextHandler();
final var keyPair = certificatesRule.generateKeyPair();

final var caCertificate = certificatesRule.caCertificateHolder();

var newCaCertificate = certificatesRule.generateCaCertificate(
keyPair,
caCertificate.getNotAfter().toInstant(),
caCertificate.getNotAfter().toInstant().minus(10, ChronoUnit.DAYS)
);

writeCertificates(newCaCertificate, certificatesRule.accessCertificateHolder(), certificatesRule.accessCertificatePrivateKey());

assertThrows(CertificateException.class, sslContextHandler::reloadSslContext);

newCaCertificate = certificatesRule.generateCaCertificate(
keyPair,
caCertificate.getNotBefore().toInstant().plus(10, ChronoUnit.DAYS),
caCertificate.getNotAfter().toInstant().plus(20, ChronoUnit.DAYS)
);
writeCertificates(newCaCertificate, certificatesRule.accessCertificateHolder(), certificatesRule.accessCertificatePrivateKey());

assertThrows(CertificateException.class, sslContextHandler::reloadSslContext);
}

@Test
public void failsIfKeyMaterialCertificateHasInvalidDates() throws Exception {
final var sslContextHandler = sslContextHandler();

final var accessCertificate = certificatesRule.x509AccessCertificate();
Expand All @@ -111,7 +138,7 @@ public void failsIfCertificatesHasInvalidDates() throws Exception {
}

@Test
public void filesIfHasNotValidSubjectDNs() throws Exception {
public void failsIfKeyMaterialCertificateHasNotValidSubjectDNs() throws Exception {
final var sslContextHandler = sslContextHandler();

final var keyPair = certificatesRule.generateKeyPair();
Expand All @@ -137,7 +164,7 @@ public void filesIfHasNotValidSubjectDNs() throws Exception {
}

@Test
public void filesIfHasNotValidIssuerDNs() throws Exception {
public void failsIfKeyMaterialCertificateHasNotValidIssuerDNs() throws Exception {
final var sslContextHandler = sslContextHandler();

final var keyPair = certificatesRule.generateKeyPair();
Expand All @@ -163,7 +190,7 @@ public void filesIfHasNotValidIssuerDNs() throws Exception {
}

@Test
public void filesIfHasNotValidSans() throws Exception {
public void failsIfKeyMaterialCertificateHasNotValidSans() throws Exception {
final var sslContextHandler = sslContextHandler();

final var keyPair = certificatesRule.generateKeyPair();
Expand Down

0 comments on commit 70463f2

Please sign in to comment.