Merge pull request #5 from subrahmanyaman/optimization

Incorporated review comments
subrahmanyaman · Apr 19, 2022 · a48a31d · a48a31d
2 parents 4941008 + 58c1df9
commit a48a31d
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 27 deletions.
diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMConfigurations.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMConfigurations.java
@@ -20,5 +20,5 @@ public class KMConfigurations {
   public static final byte LITTLE_ENDIAN = 0x00;
   public static final byte BIG_ENDIAN = 0x01;
   public static final byte TEE_MACHINE_TYPE = LITTLE_ENDIAN;
-  public static final byte MAX_ATTESTATION_IDS_SIZE = 48;
+  public static final byte MAX_ATTESTATION_IDS_SIZE = 64;
 }
diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMConfigurations.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMConfigurations.java
@@ -20,5 +20,5 @@ public class KMConfigurations {
   public static final byte LITTLE_ENDIAN = 0x00;
   public static final byte BIG_ENDIAN = 0x01;
   public static final byte TEE_MACHINE_TYPE = LITTLE_ENDIAN;
-  public static  final byte MAX_ATTESTATION_IDS_SIZE = 48;
+  public static final byte MAX_ATTESTATION_IDS_SIZE = 64;
 }
diff --git a/Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java b/Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java
@@ -34,22 +34,46 @@ public class KMAsn1Parser {
   };
 
   //https://datatracker.ietf.org/doc/html/rfc5280, RFC 5280, Page 21
+  // 2.5.4
   public byte[] COMMON_OID = new byte[] {
     0x06, 0x03, 0x55, 0x04
   };
   // This array contains the last byte of OID for each oid type.
   // The first 4 bytes are common as shown above in COMMON_OID
-  private static final byte[] attributeOIds1 = {
-      0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x0A, 0x0B, 0x0C, 0x2A,
-      0x2B, 0x2C, 0x2E, 0x41,
+  private static final byte[] attributeOIds = {
+      0x03, /* commonName COMMON_OID.3 */
+      0x04, /* surName COMMON_OID.4*/ 
+      0x05, /* serialNumber COMMON_OID.5 */ 
+      0x06, /* countryName COMMON_OID.6 */  
+      0x07, /* locality COMMON_OID.7 */  
+      0x08, /* stateOrProviince COMMON_OID.8 */  
+      0x0A, /* organizationName COMMON_OID.10 */ 
+      0x0B, /* organizationalUnitName COMMON_OID.11 */  
+      0x0C, /* title COMMON_OID.10 */  
+      0x2A, /* givenName COMMON_OID.42 */ 
+      0x2B, /* initials COMMON_OID.43 */  
+      0x2C, /* generationQualifier COMMON_OID.44 */  
+      0x2E, /* dnQualifer COMMON_OID.46 */  
+      0x41, /* pseudonym COMMON_OID.65 */
   };
   // https://datatracker.ietf.org/doc/html/rfc5280, RFC 5280, Page 124
   // TODO Specification does not mention about the DN_QUALIFIER_OID max length.
+  // So the max limit is set at 64.
   private static final byte[] attributeValueMaxLen = {
-      0x40/*64 commonName*/, 0x28/*40 surname*/, 0x40/*64 serial*/, 0x02/*64 country*/,
-      (byte)0x80/*128 locality*/, (byte)0x80/*128 state*/,  0x40/*64 organization*/, 0x40/*64 organization unit*/,
-      0x40/*64 title*/,  0x10/*16 givenName*/, 0x05/* initials*/, 0x03/* gen qualifier*/, 0x40,/*64 dn-qualifier*/ 
-      (byte)0x80/*128 pseudonym*/
+      0x40, /* 1-64 commonName */
+      0x28, /* 1-40 surname */
+      0x40, /* 1-64 serial */
+      0x02, /* 1-2 country */
+      (byte) 0x80, /* 1-128 locality */
+      (byte) 0x80,  /* 1-128 state */
+      0x40, /* 1-64 organization */
+      0x40, /* 1-64 organization unit*/
+      0x40, /* 1-64 title */
+      0x10, /* 1-16 givenName */
+      0x05, /* 1-5 initials */
+      0x03, /* 1-3 gen qualifier */
+      0x40, /* 1-64 dn-qualifier */ 
+      (byte) 0x80 /* 1-128 pseudonym */
   };
   private byte[] data;
   private short start;
@@ -235,9 +259,9 @@ private void validateAttributeTypeAndValue() {
     }
     cur = start;
     boolean found = false;
-    for(short i = 0; i < (short) attributeOIds1.length; i++) {
+    for(short i = 0; i < (short) attributeOIds.length; i++) {
       if ((Util.arrayCompare(data, cur, COMMON_OID, (short)0, (short) COMMON_OID.length) == 0) &&
-          (attributeOIds1[i] == data[(short)(cur + COMMON_OID.length)])) {
+          (attributeOIds[i] == data[(short)(cur + COMMON_OID.length)])) {
         incrementCursor((short) (COMMON_OID.length + 1));
         // Validate the length of the attribute value.
         short tag = getByte();
@@ -249,7 +273,7 @@ private void validateAttributeTypeAndValue() {
            KMException.throwIt(KMError.UNKNOWN_ERROR);
          }
         length = getLength();
-        if (length > attributeValueMaxLen[i]) {
+        if (length <= 0 && length > attributeValueMaxLen[i]) {
           KMException.throwIt(KMError.UNKNOWN_ERROR);
         }
         incrementCursor(length);

diff --git a/Applet/src/com/android/javacard/keymaster/KMByteTag.java b/Applet/src/com/android/javacard/keymaster/KMByteTag.java
@@ -95,7 +95,6 @@ public short length() {
     return KMByteBlob.cast(blobPtr).length();
   }
 
-  // TODO Review this function
   private static boolean validateKey(short key, short byteBlob) {
     short valueLen = KMByteBlob.cast(byteBlob).length();
     switch (key) {
@@ -106,11 +105,11 @@ private static boolean validateKey(short key, short byteBlob) {
         break;
       case CERTIFICATE_SUBJECT_NAME:
       {
-        KMAsn1Parser asn1Decoder = KMAsn1Parser.instance();
-        asn1Decoder.validateDerSubject(byteBlob);
         if (valueLen > MAX_SUBJECT_DER_LEN) {
           return false;
         }
+        KMAsn1Parser asn1Decoder = KMAsn1Parser.instance();
+        asn1Decoder.validateDerSubject(byteBlob);
       }
         break;
       case APPLICATION_ID:
@@ -136,12 +135,8 @@ private static boolean validateKey(short key, short byteBlob) {
           return false;
         }
         break;
-      case ROOT_OF_TRUST: // TODO : Not adding it as ByteTag in HiddenParamters.
-      //case UNIQUE_ID: This tag never used in keyParamters.
-      case NONCE: // Validation of nonce happends in begin operation.
-      // Below two tags are obsolete in keymint.
-      //case ASSOCIATED_DATA: 
-      //case CONFIRMATION_TOKEN:
+      case ROOT_OF_TRUST:
+      case NONCE:
         break;
       default:
         return false;

diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java
@@ -668,12 +668,9 @@ public void sendOutgoing(APDU apdu, KMAttestationCert cert, short certStart, sho
     encodeKeyCharacteristics(keyChars);
     // and encode it to the end of the buffer before KEY_CHARACTERISTICS
     encodeKeyBlob(keyblob);
-    // Write Error code before data[KEY_BLOB]
-    short bufferStartOffset = repository.allocReclaimableMemory((short) 1);
-    buffer[bufferStartOffset] = 0x00;
-    // Write Array header before ErrorCode.
-    bufferStartOffset = repository.allocReclaimableMemory((short) 1);
-    buffer[bufferStartOffset] = (byte) 0x84;
+    // Write Array header and ErrorCode before data[KEY_BLOB]
+    short bufferStartOffset = repository.allocReclaimableMemory((short) 2);
+    Util.setShort(buffer, bufferStartOffset, (short) 0x8400);
 
     short bufferLength = (short) (KMRepository.HEAP_SIZE - bufferStartOffset);
     // Send data