[#23] Update HIRS Utils and ACA to handle certificate padding (#26)

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 endorsementCredential;
-                EndorsementCredential.parseWithPossibleHeader(endorsementBytes);
+        try {
+            endorsementCredential = EndorsementCredential
+                    .parseWithPossibleHeader(endorsementBytes);
+        } catch (IllegalArgumentException iae) {
+            LOG.error(iae.getMessage());
+            throw iae;
+        }
         int certificateHash = endorsementCredential.getCertificateHash();
         EndorsementCredential existingCredential =
                 EndorsementCredential.select(certificateManager).includeArchived()

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

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(),

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.
      *

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);
+    }
+
 }