Skip to content

Commit

Permalink
Add validation of authority certificates (#4835)
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrey Pleskach <[email protected]>
(cherry picked from commit b445f43)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@github-actions
github-actions[bot] committed Oct 29, 2024
1 parent 07687a0 commit 8d11772
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 8d11772

Please sign in to comment.