From 30caf57edb64fe74426dd8acac80b81df1266215 Mon Sep 17 00:00:00 2001 From: Cyrus <24922493+cyrus-dev@users.noreply.github.com> Date: Tue, 19 Feb 2019 10:26:25 -0500 Subject: [PATCH] [#19] General Name/DN equals functionality (#93) * Adding new class GeneralNames, I will be changing it to adjust to the bc class as to not confuse the two. This class takes the subject string and parse out the information for comparsion. * Adding file I didn't have tracked in the previous commit. * Updating code to handle the instance of multiple organization units. * A null exception was being thrown from the unit tests for the organization unit variable. * Add some comments * continued testing and updates are needed. * Cleanup - removed excess commented code and debug lines. * Updating code base to use X500Name for name compares, removing GeneralNamesParser.java file as it is not necessary * Updated for final changes. * Modification to previous changes per request on github. Separated out compare method into its own class and created unit tests. --- .../util/CertificateStringMapBuilder.java | 6 +- .../WEB-INF/jsp/certificate-details.jsp | 4 +- .../java/hirs/utils/BouncyCastleUtils.java | 53 +++++++++++++ .../src/main/java/hirs/utils/Functional.java | 1 + .../hirs/utils/BouncyCastleUtilsTest.java | 79 +++++++++++++++++++ 5 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 HIRS_Utils/src/main/java/hirs/utils/BouncyCastleUtils.java create mode 100644 HIRS_Utils/src/test/java/hirs/utils/BouncyCastleUtilsTest.java diff --git a/HIRS_AttestationCAPortal/src/main/java/hirs/attestationca/portal/util/CertificateStringMapBuilder.java b/HIRS_AttestationCAPortal/src/main/java/hirs/attestationca/portal/util/CertificateStringMapBuilder.java index d838b976..ef9c9788 100644 --- a/HIRS_AttestationCAPortal/src/main/java/hirs/attestationca/portal/util/CertificateStringMapBuilder.java +++ b/HIRS_AttestationCAPortal/src/main/java/hirs/attestationca/portal/util/CertificateStringMapBuilder.java @@ -16,6 +16,7 @@ import hirs.data.persist.certificate.IssuedAttestationCertificate; import hirs.data.persist.certificate.PlatformCredential; import hirs.data.persist.certificate.attributes.PlatformConfiguration; import hirs.persist.CertificateManager; +import hirs.utils.BouncyCastleUtils; /** * Utility class for mapping certificate information in to string maps. These are used to display @@ -141,12 +142,13 @@ public final class CertificateStringMapBuilder { .getCertificates(); } - for (Certificate issuerCert: issuerCertificates) { + for (Certificate issuerCert : issuerCertificates) { try { // Find the certificate that actually signed this cert if (certificate.isIssuer(issuerCert)) { //Check if it's root certificate - if (issuerCert.getIssuer().equals(issuerCert.getSubject())) { + if (BouncyCastleUtils.x500NameCompare(issuerCert.getIssuer(), + issuerCert.getSubject())) { return null; } return containsAllChain(issuerCert, certificateManager); diff --git a/HIRS_AttestationCAPortal/src/main/webapp/WEB-INF/jsp/certificate-details.jsp b/HIRS_AttestationCAPortal/src/main/webapp/WEB-INF/jsp/certificate-details.jsp index d0c69da7..3af3c754 100644 --- a/HIRS_AttestationCAPortal/src/main/webapp/WEB-INF/jsp/certificate-details.jsp +++ b/HIRS_AttestationCAPortal/src/main/webapp/WEB-INF/jsp/certificate-details.jsp @@ -237,7 +237,7 @@
-
TPM
+
System Information
Manufacturer: ${initialData.manufacturer}
Model: ${initialData.model}
@@ -355,7 +355,7 @@
-
TPM
+
System Information
Manufacturer: ${initialData.manufacturer}
Model: ${initialData.model}
diff --git a/HIRS_Utils/src/main/java/hirs/utils/BouncyCastleUtils.java b/HIRS_Utils/src/main/java/hirs/utils/BouncyCastleUtils.java new file mode 100644 index 00000000..9a55bb73 --- /dev/null +++ b/HIRS_Utils/src/main/java/hirs/utils/BouncyCastleUtils.java @@ -0,0 +1,53 @@ +package hirs.utils; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.bouncycastle.asn1.x500.X500Name; + +/** + * Utilities class specific for additional Bouncy Castle functionality. + */ +public final class BouncyCastleUtils { + + private static final String SEPARATOR_COMMA = ","; + private static final String SEPARATOR_PLUS = "+"; + + private static final Logger LOGGER = LogManager.getLogger(BouncyCastleUtils.class); + + /** + * Prevents construction of an instance of this class. + */ + private BouncyCastleUtils() { + + } + + /** + * This method can be used to compare the distinguished names given from + * certificates. This compare uses X500Name class in bouncy castle, which + * compares the RDNs and not the string itself. The method will check for + * '+' and replace them, X500Name doesn't do this. + * + * @param nameValue1 first general name to be used + * @param nameValue2 second general name to be used + * @return true if the values match based on the RDNs, false if not + */ + public static boolean x500NameCompare(final String nameValue1, final String nameValue2) { + if (nameValue1 == null || nameValue2 == null) { + throw new IllegalArgumentException("Provided DN string is null."); + } + + boolean result = false; + X500Name x500Name1; + X500Name x500Name2; + + try { + x500Name1 = new X500Name(nameValue1.replace(SEPARATOR_PLUS, SEPARATOR_COMMA)); + x500Name2 = new X500Name(nameValue2.replace(SEPARATOR_PLUS, SEPARATOR_COMMA)); + result = x500Name1.equals(x500Name2); + } catch (IllegalArgumentException iaEx) { + LOGGER.error(iaEx.toString()); + } + + return result; + } +} diff --git a/HIRS_Utils/src/main/java/hirs/utils/Functional.java b/HIRS_Utils/src/main/java/hirs/utils/Functional.java index bdd487f2..a0a2207e 100644 --- a/HIRS_Utils/src/main/java/hirs/utils/Functional.java +++ b/HIRS_Utils/src/main/java/hirs/utils/Functional.java @@ -8,6 +8,7 @@ import java.util.List; * Simple utility class to house some functional methods. */ public final class Functional { + /** * Prevents construction of an instance of this class. */ diff --git a/HIRS_Utils/src/test/java/hirs/utils/BouncyCastleUtilsTest.java b/HIRS_Utils/src/test/java/hirs/utils/BouncyCastleUtilsTest.java new file mode 100644 index 00000000..7d062484 --- /dev/null +++ b/HIRS_Utils/src/test/java/hirs/utils/BouncyCastleUtilsTest.java @@ -0,0 +1,79 @@ +package hirs.utils; + +import org.apache.logging.log4j.util.Strings; +import org.testng.Assert; +import org.testng.annotations.Test; + +/** + * Tests methods in the (@link BouncyCastleUtils) utility class. + */ +public class BouncyCastleUtilsTest { + + private static final String VALID_RDN_STRING = "OU=PCTest,O=example.com,C=US"; + + private static final String VALID_RDN_STRING_SWITCHED = "C=US,OU=PCTest,O=example.com"; + private static final String VALID_RDN_STRING_UPPERCASE = "OU=PCTEST,O=EXAMPLE.COM,C=US"; + private static final String VALID_RDN_STRING_PLUS = "OU=PCTest+O=example.com+C=US"; + private static final String UNEQUAL_RDN_STRING = "OU=PCTest,O=example1.com,C=US"; + private static final String MALFORMED_RDN_STRING = "OU=PCTest,OZ=example1.com,C=US"; + + /** + * True Test of x500NameCompare method, of class BouncyCastleUtils. + */ + @Test + public void testX500NameCompareTrue() { + Assert.assertTrue(BouncyCastleUtils.x500NameCompare( + VALID_RDN_STRING, VALID_RDN_STRING_SWITCHED)); + Assert.assertTrue(BouncyCastleUtils.x500NameCompare( + VALID_RDN_STRING, VALID_RDN_STRING_UPPERCASE)); + Assert.assertTrue(BouncyCastleUtils.x500NameCompare( + VALID_RDN_STRING, VALID_RDN_STRING_PLUS)); + } + + /** + * False Test of x500NameCompare method, of class BouncyCastleUtils. + */ + @Test + public void testX500NameCompareFalse() { + Assert.assertFalse(BouncyCastleUtils.x500NameCompare( + VALID_RDN_STRING, UNEQUAL_RDN_STRING)); + // Error that aren't thrown but logged + Assert.assertFalse(BouncyCastleUtils.x500NameCompare(VALID_RDN_STRING, Strings.EMPTY)); + Assert.assertFalse(BouncyCastleUtils.x500NameCompare(Strings.EMPTY, VALID_RDN_STRING)); + Assert.assertFalse(BouncyCastleUtils.x500NameCompare(Strings.EMPTY, Strings.EMPTY)); + Assert.assertFalse(BouncyCastleUtils.x500NameCompare( + VALID_RDN_STRING, MALFORMED_RDN_STRING)); + Assert.assertFalse(BouncyCastleUtils.x500NameCompare( + MALFORMED_RDN_STRING, VALID_RDN_STRING)); + Assert.assertFalse(BouncyCastleUtils.x500NameCompare( + MALFORMED_RDN_STRING, MALFORMED_RDN_STRING)); + } + + /** + * Null String Error Test of x500NameCompare method, of class + * BouncyCastleUtils. + */ + @Test + public void testX500NameCompareNullError() { + try { + BouncyCastleUtils.x500NameCompare(VALID_RDN_STRING, null); + Assert.fail("No IllegalArgumentException thrown."); + } catch (Exception ex) { + Assert.assertEquals(ex.getMessage(), "Provided DN string is null."); + } + + try { + BouncyCastleUtils.x500NameCompare(null, VALID_RDN_STRING); + Assert.fail("No IllegalArgumentException thrown."); + } catch (Exception ex) { + Assert.assertEquals(ex.getMessage(), "Provided DN string is null."); + } + + try { + BouncyCastleUtils.x500NameCompare(null, null); + Assert.fail("No IllegalArgumentException thrown."); + } catch (Exception ex) { + Assert.assertEquals(ex.getMessage(), "Provided DN string is null."); + } + } +}