From f192ce5826f88e9ef08f3e239c847130685e326c Mon Sep 17 00:00:00 2001 From: apldev3 Date: Thu, 18 Oct 2018 14:34:53 -0400 Subject: [PATCH] [#23] Update HIRS Utils and ACA to handle certificate padding (#26) --- .../CredentialManagementHelper.java | 10 ++- .../data/persist/certificate/Certificate.java | 51 ++++++++++++ .../certificate/EndorsementCredential.java | 2 + .../persist/certificate/CertificateTest.java | 73 ++++++++++++++++++ .../EndorsementCredentialTest.java | 20 +++++ .../ek_cert_with_padded_bytes.cer | Bin 0 -> 1100 bytes .../ek_cert_with_security_assertions.cer | Bin 0 -> 1704 bytes 7 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 HIRS_Utils/src/test/resources/certificates/ek_cert_with_padded_bytes.cer create mode 100644 HIRS_Utils/src/test/resources/certificates/ek_cert_with_security_assertions.cer diff --git a/HIRS_AttestationCA/src/main/java/hirs/attestationca/CredentialManagementHelper.java b/HIRS_AttestationCA/src/main/java/hirs/attestationca/CredentialManagementHelper.java index 562387c4..6355f0d1 100644 --- a/HIRS_AttestationCA/src/main/java/hirs/attestationca/CredentialManagementHelper.java +++ b/HIRS_AttestationCA/src/main/java/hirs/attestationca/CredentialManagementHelper.java @@ -47,8 +47,14 @@ public final class CredentialManagementHelper { LOG.info("Parsing Endorsement Credential of length " + endorsementBytes.length); - EndorsementCredential endorsementCredential = - EndorsementCredential.parseWithPossibleHeader(endorsementBytes); + EndorsementCredential endorsementCredential; + try { + endorsementCredential = EndorsementCredential + .parseWithPossibleHeader(endorsementBytes); + } catch (IllegalArgumentException iae) { + LOG.error(iae.getMessage()); + throw iae; + } int certificateHash = endorsementCredential.getCertificateHash(); EndorsementCredential existingCredential = EndorsementCredential.select(certificateManager).includeArchived() diff --git a/HIRS_Utils/src/main/java/hirs/data/persist/certificate/Certificate.java b/HIRS_Utils/src/main/java/hirs/data/persist/certificate/Certificate.java index 844c961d..c0023abe 100644 --- a/HIRS_Utils/src/main/java/hirs/data/persist/certificate/Certificate.java +++ b/HIRS_Utils/src/main/java/hirs/data/persist/certificate/Certificate.java @@ -33,6 +33,7 @@ import javax.persistence.Transient; import java.io.ByteArrayInputStream; import java.io.IOException; import java.math.BigInteger; +import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -65,6 +66,7 @@ public abstract class Certificate extends ArchivableEntity { private static final String PEM_FOOTER = "-----END CERTIFICATE-----"; private static final String PEM_ATTRIBUTE_HEADER = "-----BEGIN ATTRIBUTE CERTIFICATE-----"; private static final String PEM_ATTRIBUTE_FOOTER = "-----END ATTRIBUTE CERTIFICATE-----"; + private static final String MALFORMED_CERT_MESSAGE = "Malformed certificate detected."; private static final int MAX_CERT_LENGTH_BYTES = 2048; private static final int MAX_NUMERIC_PRECISION = 49; // Can store up to 160 bit values private static final int MAX_PUB_KEY_MODULUS_HEX_LENGTH = 1024; @@ -265,6 +267,8 @@ public abstract class Certificate extends ArchivableEntity { this.certificateBytes = Base64.decode(possiblePem); } + this.certificateBytes = trimCertificate(this.certificateBytes); + // Extract certificate data switch (getCertificateType()) { case X509_CERTIFICATE: @@ -328,6 +332,53 @@ public abstract class Certificate extends ArchivableEntity { this.certAndTypeHash = Objects.hash(certificateHash, getClass().getSimpleName()); } + @SuppressWarnings("magicnumber") + private byte[] trimCertificate(final byte[] certificateBytes) { + int certificateStart = 0; + int certificateLength = 0; + ByteBuffer certificateByteBuffer = ByteBuffer.wrap(certificateBytes); + + StringBuilder malformedCertStringBuilder = new StringBuilder(MALFORMED_CERT_MESSAGE); + while (certificateByteBuffer.hasRemaining()) { + // Check if there isn't an ASN.1 structure in the provided bytes + if (certificateByteBuffer.remaining() <= 2) { + throw new IllegalArgumentException(malformedCertStringBuilder + .append(" No certificate length field could be found.").toString()); + } + + // Look for first ASN.1 Sequence marked by the two bytes (0x30) and (0x82) + // The check advances our position in the ByteBuffer by one byte + int currentPosition = certificateByteBuffer.position(); + if (certificateByteBuffer.get() == (byte) 0x30 + && certificateByteBuffer.get(currentPosition + 1) == (byte) 0x82) { + // Check if we have anything more in the buffer than an ASN.1 Sequence header + if (certificateByteBuffer.remaining() <= 3) { + throw new IllegalArgumentException(malformedCertStringBuilder + .append(" Certificate is nothing more than ASN.1 Sequence.") + .toString()); + } + // Mark the start of the first ASN.1 Sequence / Certificate Body + certificateStart = currentPosition; + + // Parse the length as the 2-bytes following the start of the ASN.1 Sequence + certificateLength = Short.toUnsignedInt( + certificateByteBuffer.getShort(currentPosition + 2)); + // Add the 4 bytes that comprise the start of the ASN.1 Sequence and the length + certificateLength += 4; + break; + } + } + + if (certificateStart + certificateLength > certificateBytes.length) { + throw new IllegalArgumentException(malformedCertStringBuilder + .append(" Value of certificate length field extends beyond length") + .append(" of provided certificate.").toString()); + } + // Return bytes representing the main certificate body + return Arrays.copyOfRange(certificateBytes, certificateStart, + certificateStart + certificateLength); + } + /** * @return the type of certificate. * @throws java.io.IOException if there is a problem extracting information from the certificate diff --git a/HIRS_Utils/src/main/java/hirs/data/persist/certificate/EndorsementCredential.java b/HIRS_Utils/src/main/java/hirs/data/persist/certificate/EndorsementCredential.java index 22c8fe84..cf44ab0d 100644 --- a/HIRS_Utils/src/main/java/hirs/data/persist/certificate/EndorsementCredential.java +++ b/HIRS_Utils/src/main/java/hirs/data/persist/certificate/EndorsementCredential.java @@ -373,6 +373,8 @@ public class EndorsementCredential extends DeviceAssociatedCertificate { revision.getValue()); LOGGER.debug("Found TPM Spec:" + tpmSpecification.toString()); } else if (addToMapping && key.equals(TPM_SECURITY_ASSERTIONS)) { + // TODO(apldev3): Update this block to properly parse TPM Security Assertions + // per the document "TCG EK Credential Profile For TPM Family 2.0; Level 0" (pg. 19) ASN1Integer ver = (ASN1Integer) seq.getObjectAt(ASN1_VER_INDEX); ASN1Boolean fieldUpgradeable = (ASN1Boolean) seq.getObjectAt(ASN1_UPGRADEABLE_INDEX); tpmSecurityAssertions = new TPMSecurityAssertions(ver.getValue(), diff --git a/HIRS_Utils/src/test/java/hirs/data/persist/certificate/CertificateTest.java b/HIRS_Utils/src/test/java/hirs/data/persist/certificate/CertificateTest.java index e60799c6..21450da8 100644 --- a/HIRS_Utils/src/test/java/hirs/data/persist/certificate/CertificateTest.java +++ b/HIRS_Utils/src/test/java/hirs/data/persist/certificate/CertificateTest.java @@ -7,6 +7,7 @@ import org.testng.annotations.Test; import java.io.FileInputStream; import java.io.IOException; +import java.math.BigInteger; import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; @@ -87,6 +88,9 @@ public class CertificateTest { private static final String RDN_COMMA_SEPARATED_ORGANIZATION = "STMicroelectronics NV"; private static final String RDN_MULTIVALUE_ORGANIZATION = "Nuvoton Technology Corporation"; + private static final String EK_CERT_WITH_PADDED_BYTES = + "/certificates/ek_cert_with_padded_bytes.cer"; + /** * Tests that a certificate can be constructed from a byte array. @@ -268,6 +272,75 @@ public class CertificateTest { Assert.assertEquals(platformCert.getEndValidity(), attrCertHolder.getNotAfter()); } + /** + * Tests that Certificate correctly trims out additional padding from a given certificate. + * + * @throws IOException if there is a problem reading the cert file at the given path + * @throws URISyntaxException if there is a problem constructing the file's URI + */ + @Test + public void testCertificateTrim() throws IOException, URISyntaxException { + byte[] rawFileBytes = Files.readAllBytes(Paths.get(CertificateTest.class + .getResource(EK_CERT_WITH_PADDED_BYTES).toURI())); + byte[] expectedCertBytes = Arrays.copyOfRange(rawFileBytes, 0, 908); + Certificate ekCert = getTestCertificate(EndorsementCredential.class, + EK_CERT_WITH_PADDED_BYTES); + Assert.assertEquals(ekCert.getSerialNumber(), new BigInteger("16842032579184247954")); + Assert.assertEquals(ekCert.getIssuer(), + "CN=Nuvoton TPM Root CA 2010+O=Nuvoton Technology Corporation+C=TW"); + Assert.assertEquals(ekCert.getSubject(), ""); + Assert.assertEquals(ekCert.getRawBytes(), expectedCertBytes); + } + + /** + * Tests that Certificate correctly throws IllegalArgumentException when no length field is + * found in the provided byte array. + * + * @throws IOException if there is a problem reading the cert file at the given path + * @throws URISyntaxException if there is a problem constructing the file's URI + */ + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = ".* No certificate length field could be found\\.") + public void testCertificateTrimThrowsWhenNoLengthFieldFound() throws IOException, + URISyntaxException { + byte[] rawFileBytes = Files.readAllBytes(Paths.get(CertificateTest.class + .getResource(EK_CERT_WITH_PADDED_BYTES).toURI())); + new EndorsementCredential(Arrays.copyOfRange(rawFileBytes, 0, 2)); + } + + /** + * Tests that Certificate correctly throws IllegalArgumentException when the byte array only + * contains a header for an ASN.1 Sequence. + * + * @throws IOException if there is a problem reading the cert file at the given path + * @throws URISyntaxException if there is a problem constructing the file's URI + */ + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = ".* Certificate is nothing more than ASN.1 Sequence\\.") + public void testCertificateTrimThrowsWhenOnlyASN1Sequence() throws IOException, + URISyntaxException { + byte[] rawFileBytes = Files.readAllBytes(Paths.get(CertificateTest.class + .getResource(EK_CERT_WITH_PADDED_BYTES).toURI())); + new EndorsementCredential(Arrays.copyOfRange(rawFileBytes, 0, 4)); + } + + /** + * Tests that Certificate correctly throws IllegalArgumentException when the provided + * Certificate has a length that extends beyond the byte array as a whole. + * + * @throws IOException if there is a problem reading the cert file at the given path + * @throws URISyntaxException if there is a problem constructing the file's URI + */ + @Test(expectedExceptions = IllegalArgumentException.class, + expectedExceptionsMessageRegExp = ".* Value of certificate length field extends beyond" + + " length of provided certificate\\.") + public void testCertificateTrimThrowsWhenLengthIsTooLarge() throws IOException, + URISyntaxException { + byte[] rawFileBytes = Files.readAllBytes(Paths.get(CertificateTest.class + .getResource(EK_CERT_WITH_PADDED_BYTES).toURI())); + new EndorsementCredential(Arrays.copyOfRange(rawFileBytes, 0, 42)); + } + /** * Tests that the equals method on {@link Certificate} works as expected. * diff --git a/HIRS_Utils/src/test/java/hirs/data/persist/certificate/EndorsementCredentialTest.java b/HIRS_Utils/src/test/java/hirs/data/persist/certificate/EndorsementCredentialTest.java index 98bdb908..f719add0 100644 --- a/HIRS_Utils/src/test/java/hirs/data/persist/certificate/EndorsementCredentialTest.java +++ b/HIRS_Utils/src/test/java/hirs/data/persist/certificate/EndorsementCredentialTest.java @@ -21,6 +21,8 @@ public class EndorsementCredentialTest { = "/certificates/nuc-1/tpmcert.pem"; private static final String TEST_ENDORSEMENT_CREDENTIAL_NUC2 = "/certificates/nuc-2/tpmcert.pem"; + private static final String EK_CERT_WITH_SECURITY_ASSERTIONS = + "/certificates/ek_cert_with_security_assertions.cer"; /** * Tests the successful parsing of an EC using a test cert from STM. @@ -182,4 +184,22 @@ public class EndorsementCredentialTest { Assert.assertNotEquals(ec2, ec3); } + /** + * Tests that EndorsementCredential correctly parses out TPM Security Assertions from a + * provided TPM EK Certificate. + * + * @throws IOException if there is a problem reading the cert file at the given path + */ + @Test(enabled = false) + // TODO(apldev3): Reenable test when update to security assertions is made in + // EndorsementCredential + public void testTpmSecurityAssertionsParsing() throws IOException { + Path fPath = Paths.get(CertificateTest.class + .getResource(EK_CERT_WITH_SECURITY_ASSERTIONS).getPath()); + EndorsementCredential ec = new EndorsementCredential(fPath); + + // TODO(apldev3): Make assertions about TPMSecurityAssertions fields + System.out.println(ec); + } + } diff --git a/HIRS_Utils/src/test/resources/certificates/ek_cert_with_padded_bytes.cer b/HIRS_Utils/src/test/resources/certificates/ek_cert_with_padded_bytes.cer new file mode 100644 index 0000000000000000000000000000000000000000..65426766e4ce2c318559e09cac9d8cf38ef4b59a GIT binary patch literal 1100 zcmXqLV(u_#Vk%g`%*4pV#L4h-*Xz`qS6wC<@Un4gwRyCC=VfGMWo0l3H4HY8XJZa! zVHTF~D=o_}$=3Gh`2%Fi!RaCTHMGB7kSPz5RA5|)E2NKMYj%g@PAuT*f(FDl3{ zN-W9D&okfzDPt363JEum6X!KFGcYwWGBPl-G&PPA=QRd$4K1Kt0|tX8#&`oSgj?7R zY!`3?O<>jLV`h?KWmqU-Acm0TK$2M`VjzT&;bdVAaSm`~U~XdM2N}x6)WpchFyn{t zE|Cw#9k1tkIV4y6>dP8dds%GD@C>;Xu*&A6g&mXl-K0Dj;a3$cw0v7i7jQU{ruhLs%bTo(*B7{n2z7s_qSB+Q}`Oc8}}r)e&{w`Tr1dfyqcT4 z?^~4q6<@Q((+|%*Y@2C)o~bD9_mtI}cAwOoYbrO@?e3*2pQo31bcRpYcjqsT;cHr( zqxkYV;|z`ES7&BzjBmYcK6T%5zuUZ5O0`d{eO1mic>M-)UR->i(OY z=Zeo?ZNH|?#LURRxVYG$(7*>6C$fT!jQ?4j3>=o&8Q2(F8Hlj4rZ=)OF$!~MrdXM{ znwppx8yFY}Kt!2@+5G~XL(D2FU?R-I>_Bw}rWS?7cg2F8JKUyd8?Q%FR$v~60?`d>HOK!{|jm#@I!}ynZUTF=W?U zp*OqKDwg_RuARthlD@V-;(Pl4|6lg|wBFyI=y*f*Jk!@w$BJvE;$zpyWUp1=3ZLyV7kJ&vv;)|uZhdXrmZkxd+Bg?G(*jd{?%9XD8*z-H1#(Uf0p48Q*KOHqp&eyJMdX=1TO+O=a_p;he%hpZ( b)Wl@K?scGx=i>j>MR&6Uw)+1R9O3`~^{Qw) literal 0 HcmV?d00001 diff --git a/HIRS_Utils/src/test/resources/certificates/ek_cert_with_security_assertions.cer b/HIRS_Utils/src/test/resources/certificates/ek_cert_with_security_assertions.cer new file mode 100644 index 0000000000000000000000000000000000000000..e05b049d05107ab1884c946f1614f30a06d00b73 GIT binary patch literal 1704 zcmWe&WMC~9U@~Z8Ei`CiiCnA1Dvxa0s&nCsyR=RT?T9$b$s9ge5%l(lYZ>^Yau!Qj;_C@^kXjGgFHd z9Ni6h47fl_xP_S=J$((;3{*foW?@-Rw+Mxh0AB@HZw1f1lGLKy)RfG`l2ip}M+E~5 z137VCLt{fjLsLUDV`D?(C~;mRWUc{&K@+37ffB;C>;|HXgbf4{0-P)?A z+jUAMe7oSi;8QES%X0SW>F+%%dyW6m;s(ZGE!{t@!JBJ$UHGN?y?50L-((lo>le4D zU5$LOESG)$a>Ki6Tjo5!;8!A^*pj5i>3UO8Kl{3M`0cl0+QtXo?9}CAEIYY#w#fG< zmnY-|>u02R3V)cg?%u|{JD& z^3(87t+(Ywk8)=0TC1vJS;)l9$iTR`i7C~fi7C+_6c`nXN+Ld2PPc!GVLEX|BfLqZJoj0}Yg1RxU3JnTRX z2Ee#7-~kyc3^JAp7*CCRKwJe7w{fFE<2r-J)ok1eZ61tmKU^8v^%)HsXB$|uacHvv z6C67eqnK_+NlAf~zJ7Umxn5anQD$*wdY)c#ey)B|YC(Q+W=VcgrG92!N@|5(MoDgt zK^$BwlR=PN07D3aGeZD_BZC5vRm4!rP|Q%mkjjt(1GOXD1a#+eNwKHk15og7c1|3wL0PHtb; zvF_mciiOonDhDj5G_i1G$cY&=P6j!XlcllSps~Zi3dmtG0Y(KV_p)m9F*8X4GqVaY z`NcpC5}yKwyawDn%)mGX6%LHd21XEZAwx|Abzo#JVCG_ETnHo>7J-Px0tS3y%*J|# zT#PJ0E>Hmwp_L}UtjNf)T=ny1iAk)xKh$hh{=4n_3VS8db3emo&Dv3RxSanUg9XRd ziaRnlUYTcvF-^Q!B5+~KxhZy;hg|+Q-JcX+T^+=H=TO+Q(rdd*6PrHyR&&NlIM4cE zw)2CI;nKS)Gd`)+Xfl4g)!SLO=+5jATa11%CUoH*IQi*8is%_*uBT zW!tl=f_Ex0>UL>C!Z!C3k96s4bDh04sbt2N)_47H>xJHwn-ub9SYK*1t7qR*P^h`{ jQA4-(-d*A!`l_DPGIqD`FN!n2@7lAtrHf${5FG*ln9IaW literal 0 HcmV?d00001