From c78e66a039b6b128d9bedc2a35144aeca5640af9 Mon Sep 17 00:00:00 2001 From: Subrahmanyaman Date: Tue, 19 Apr 2022 03:56:44 +0000 Subject: [PATCH] Added Subject DER validation --- .../keymaster/KMAttestationCertImpl.java | 2 +- .../javacard/keymaster/KMAsn1Parser.java | 90 +++++++++++++------ .../android/javacard/keymaster/KMByteTag.java | 4 +- .../javacard/keymaster/KMKeymasterApplet.java | 4 +- .../android/javacard/keymaster/KMType.java | 4 +- 5 files changed, 70 insertions(+), 34 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java index a5e4953b..0da291eb 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAttestationCertImpl.java @@ -110,7 +110,7 @@ public class KMAttestationCertImpl implements KMAttestationCert { KMType.ATTESTATION_ID_SERIAL, KMType.ATTESTATION_ID_PRODUCT, KMType.ATTESTATION_ID_DEVICE, KMType.ATTESTATION_ID_BRAND, KMType.OS_PATCH_LEVEL, KMType.OS_VERSION, KMType.ROOT_OF_TRUST, - KMType.ORIGIN, KMType.UNLOCKED_DEVICE_REQUIRED, + KMType.ORIGIN, KMType.UNLOCKED_DEVICE_REQUIRED, KMType.TRUSTED_CONFIRMATION_REQUIRED, KMType.AUTH_TIMEOUT, KMType.USER_AUTH_TYPE, KMType.NO_AUTH_REQUIRED, KMType.EARLY_BOOT_ONLY, diff --git a/Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java b/Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java index 6dcaf4f3..07f9ed2e 100644 --- a/Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java +++ b/Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java @@ -32,8 +32,24 @@ public class KMAsn1Parser { 0x3d,0x02,0x01,0x06,0x08,0x2a,(byte)0x86,0x48, (byte)0xce,0x3d,0x03,0x01,0x07 }; - public static final byte[] COMMON_NAME_OID = { - 0x55, 0x04, 0x03 + + //https://datatracker.ietf.org/doc/html/rfc5280, RFC 5280, Page 21 + 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, + }; + // https://datatracker.ietf.org/doc/html/rfc5280, RFC 5280, Page 124 + // TODO Specification does not mention about the DN_QUALIFIER_OID max length. + 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*/ }; private byte[] data; private short start; @@ -58,13 +74,16 @@ public short decodeEc(short blob){ return decodeEcPrivateKey((short)1); } - public short decodeSubject(short blob) { + public void validateDerSubject(short blob) { init(blob); header(ASN1_SEQUENCE); - header(ASN1_SET); - header(ASN1_SEQUENCE); - objectIdentifier(COMMON_NAME_OID); - return subjectHeader(); + while (cur < ((short) (start + length))) { + header(ASN1_SET); + header(ASN1_SEQUENCE); + // Parse and validate OBJECT-IDENTIFIER and Value fields + // Cursor is incremented in validateAttributeTypeAndValue. + validateAttributeTypeAndValue(); + } } public short decodeEcSubjectPublicKeyInfo(short blob) { @@ -205,14 +224,43 @@ private void validateTag0IfPresent(){ incrementCursor(len); } - private short objectIdentifier(byte[] oid) { - short length = header(OBJECT_IDENTIFIER); - if (length != oid.length) { - KMException.throwIt(KMError.UNKNOWN_ERROR); - } - if(Util.arrayCompare(data, cur, oid, (short)0, length) != 0) KMException.throwIt(KMError.UNKNOWN_ERROR); - incrementCursor(length); - return length; + private void validateAttributeTypeAndValue() { + short start = cur; + if (getByte() != OBJECT_IDENTIFIER) { + KMException.throwIt(KMError.UNKNOWN_ERROR); + } + short length = getLength(); + if (length != 3) { + KMException.throwIt(KMError.UNKNOWN_ERROR); + } + cur = start; + boolean found = false; + for(short i = 0; i < (short) attributeOIds1.length; i++) { + if ((Util.arrayCompare(data, cur, COMMON_OID, (short)0, (short) COMMON_OID.length) == 0) && + (attributeOIds1[i] == data[(short)(cur + COMMON_OID.length)])) { + incrementCursor((short) (COMMON_OID.length + 1)); + // Validate the length of the attribute value. + short tag = getByte(); + if(tag != ASN1_UTF8_STRING && + tag != ASN1_TELETEX_STRING && + tag != ASN1_PRINTABLE_STRING && + tag != ASN1_UNIVERSAL_STRING && + tag != ASN1_BMP_STRING) { + KMException.throwIt(KMError.UNKNOWN_ERROR); + } + length = getLength(); + if (length > attributeValueMaxLen[i]) { + KMException.throwIt(KMError.UNKNOWN_ERROR); + } + incrementCursor(length); + found = true; + break; + } + } + if (!found) { + // None of the attributes matches. + KMException.throwIt(KMError.UNKNOWN_ERROR); + } } private short header(short tag){ @@ -221,18 +269,6 @@ private short header(short tag){ return getLength(); } - private short subjectHeader(){ - short t = getByte(); - if(t != ASN1_UTF8_STRING && - t != ASN1_TELETEX_STRING && - t != ASN1_PRINTABLE_STRING && - t != ASN1_UNIVERSAL_STRING && - t != ASN1_BMP_STRING) { - KMException.throwIt(KMError.UNKNOWN_ERROR); - } - return getLength(); - } - private byte getByte(){ byte d = data[cur]; incrementCursor((short)1); diff --git a/Applet/src/com/android/javacard/keymaster/KMByteTag.java b/Applet/src/com/android/javacard/keymaster/KMByteTag.java index 3cb5d34f..88f3ea63 100644 --- a/Applet/src/com/android/javacard/keymaster/KMByteTag.java +++ b/Applet/src/com/android/javacard/keymaster/KMByteTag.java @@ -107,8 +107,8 @@ private static boolean validateKey(short key, short byteBlob) { case CERTIFICATE_SUBJECT_NAME: { KMAsn1Parser asn1Decoder = KMAsn1Parser.instance(); - short length = asn1Decoder.decodeSubject(byteBlob); - if (length > MAX_SUBJECT_CN_LEN) { + asn1Decoder.validateDerSubject(byteBlob); + if (valueLen > MAX_SUBJECT_DER_LEN) { return false; } } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 3d8369a3..b6825173 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -1383,11 +1383,11 @@ private KMAttestationCert makeAttestationCert(short attKeyBlob, short attKeyPara KMAsn1Parser asn1Decoder = KMAsn1Parser.instance(); short length = 0; try { - length = asn1Decoder.decodeSubject(issuer); + asn1Decoder.validateDerSubject(issuer); } catch (KMException e) { KMException.throwIt(KMError.INVALID_ISSUER_SUBJECT_NAME); } - if (length > KMType.MAX_SUBJECT_CN_LEN) { + if (KMByteBlob.cast(issuer).length() > KMType.MAX_SUBJECT_DER_LEN) { KMException.throwIt(KMError.INVALID_ISSUER_SUBJECT_NAME); } // If issuer is not present then it is an error diff --git a/Applet/src/com/android/javacard/keymaster/KMType.java b/Applet/src/com/android/javacard/keymaster/KMType.java index bc5849ab..95806ad3 100644 --- a/Applet/src/com/android/javacard/keymaster/KMType.java +++ b/Applet/src/com/android/javacard/keymaster/KMType.java @@ -360,8 +360,8 @@ public abstract class KMType { public static final short MAX_CERTIFICATE_SERIAL_SIZE = 20; // Attestation Application ID public static final short MAX_ATTESTATION_APP_ID_SIZE = 1024; - // Maximum Certificate Subject Common name length. - public static final short MAX_SUBJECT_CN_LEN = 64; + // DER subject max length. + public static final short MAX_SUBJECT_DER_LEN = 1095; protected static KMRepository repository;