Skip to content

Incorporated review comments#5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AvinashHedage merged 1 commit into from
Apr 19, 2022
Merged

Incorporated review comments #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMConfigurations.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
}
2 changes: 1 addition & 1 deletion Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMConfigurations.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
}
44 changes: 34 additions & 10 deletions Applet/src/com/android/javacard/keymaster/KMAsn1Parser.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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();
Expand All@@ -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);
Expand Down
13 changes: 4 additions & 9 deletions Applet/src/com/android/javacard/keymaster/KMByteTag.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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) {
Expand All@@ -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:
Expand All@@ -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;
Expand Down
9 changes: 3 additions & 6 deletions Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand Down