From 6b68b07ea7a78b42c9a31cfc688215e6491d7c9f Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Wed, 26 Jan 2022 07:43:16 +0000 Subject: [PATCH 1/2] Fixed Import wrapeed key VTS failure --- Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java b/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java index 69828036..791fc4cd 100644 --- a/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java +++ b/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java @@ -299,6 +299,7 @@ private static void initKMDeviceStatics() { KMBoolTag.initStatics(); KMPKCS8Decoder.initStatics(); KMEnumArrayTag.initStatics(); + KMIntegerArrayTag.initStatics(); } @@ -2897,6 +2898,7 @@ private void processImportWrappedKeyCmd(APDU apdu) { KMByteBlob.length(tmpVariables[0]), scratchPad, (short) 0); + data[PUB_KEY] = KMType.INVALID_VALUE; data[SECRET] = KMByteBlob.instance(scratchPad, (short) 0, tmpVariables[1]); // Step 3 - XOR the decrypted AES-GCM key with with masking key From 67d0129ee37971ee74a9c7cf4b1aaeed2dbf45e3 Mon Sep 17 00:00:00 2001 From: "avinash.hedage" Date: Fri, 28 Jan 2022 19:33:32 +0000 Subject: [PATCH 2/2] Keymint spec review fixes --- .../javacard/kmdevice/KMIntegerTag.java | 2 +- .../javacard/kmdevice/KMKeyParameters.java | 2 +- .../javacard/kmdevice/KMKeymasterDevice.java | 72 +++++++++++-------- .../javacard/kmdevice/KMKeymintDevice.java | 2 +- .../com/android/javacard/kmdevice/KMTag.java | 1 + 5 files changed, 48 insertions(+), 31 deletions(-) diff --git a/Applet/src/com/android/javacard/kmdevice/KMIntegerTag.java b/Applet/src/com/android/javacard/kmdevice/KMIntegerTag.java index 2c14f3da..bfcf472c 100644 --- a/Applet/src/com/android/javacard/kmdevice/KMIntegerTag.java +++ b/Applet/src/com/android/javacard/kmdevice/KMIntegerTag.java @@ -198,7 +198,7 @@ public boolean isValidKeySize(byte alg) { } break; case KMType.DES: - if (val == 192 || val == 168) { + if (val == 168) { return true; } break; diff --git a/Applet/src/com/android/javacard/kmdevice/KMKeyParameters.java b/Applet/src/com/android/javacard/kmdevice/KMKeyParameters.java index 74980cbd..a763695f 100644 --- a/Applet/src/com/android/javacard/kmdevice/KMKeyParameters.java +++ b/Applet/src/com/android/javacard/kmdevice/KMKeyParameters.java @@ -198,7 +198,7 @@ public static short findTag(short bPtr, short tagType, short tagKey) { return KMKeyParameters.cast(bPtr).findTag(tagType, tagKey); } - public boolean hasUnsupportedTags(short keyParamsPtr) { + public static boolean hasUnsupportedTags(short keyParamsPtr) { byte index = 0; short tagInd; diff --git a/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java b/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java index 791fc4cd..72ad8ed4 100644 --- a/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java +++ b/Applet/src/com/android/javacard/kmdevice/KMKeymasterDevice.java @@ -1173,10 +1173,10 @@ private short finishImportWrappedKeyCmd(APDU apdu){ //TODO remove cmd later on private void processFinishImportWrappedKeyCmd(APDU apdu){ short cmd = finishImportWrappedKeyCmd(apdu); - short keyParameters = KMArray.get(cmd, (short) 0); + data[KEY_PARAMETERS] = KMArray.get(cmd, (short) 0); short keyFmt = KMArray.get(cmd, (short) 1); keyFmt = KMEnum.getVal(keyFmt); - validateImportKey(keyParameters, keyFmt); + validateImportKey(data[KEY_PARAMETERS], keyFmt); byte[] scratchPad = apdu.getBuffer(); // Step 4 - AES-GCM decrypt the wrapped key data[INPUT_DATA] = KMArray.get(cmd, (short) 2); @@ -1190,8 +1190,7 @@ private void processFinishImportWrappedKeyCmd(APDU apdu){ data[IMPORTED_KEY_BLOB] = aesGCMDecrypt(getWrappingKey(),data[INPUT_DATA],data[NONCE],data[AUTH_DATA], data[AUTH_TAG],scratchPad); resetWrappingKey(); // Step 5 - Import decrypted key - data[ORIGIN] = KMType.SECURELY_IMPORTED; - data[KEY_PARAMETERS] = keyParameters; + data[ORIGIN] = KMType.SECURELY_IMPORTED; // create key blob array importKey(apdu, keyFmt, scratchPad); } @@ -2185,7 +2184,6 @@ private void authorizeDigest(KMOperationState op) { KMKeyParameters.findTag(data[KEY_PARAMETERS], KMType.ENUM_ARRAY_TAG, KMType.PADDING); if (paramPadding != KMType.INVALID_VALUE) { if (KMEnumArrayTag.length(paramPadding) != 1) { - //TODO vts fails because it expects UNSUPPORTED_PADDING_MODE KMException.throwIt(KMError.UNSUPPORTED_PADDING_MODE); } paramPadding = KMEnumArrayTag.get(paramPadding, (short) 0); @@ -2216,7 +2214,7 @@ private void authorizePadding(KMOperationState op) { KMKeyParameters.findTag(data[KEY_PARAMETERS], KMType.ENUM_ARRAY_TAG, KMType.PADDING); if (param != KMType.INVALID_VALUE) { if (KMEnumArrayTag.length(param) != 1) { - KMException.throwIt(KMError.INVALID_ARGUMENT); + KMException.throwIt(KMError.UNSUPPORTED_PADDING_MODE); } param = KMEnumArrayTag.get(param, (short) 0); if (!KMEnumArrayTag.contains(paddings, param)) { @@ -2294,9 +2292,6 @@ private void authorizeBlockModeAndMacLength(KMOperationState op) { default: KMException.throwIt(KMError.UNSUPPORTED_BLOCK_MODE); } - if (param == KMType.INVALID_VALUE) { - KMException.throwIt(KMError.INVALID_ARGUMENT); - } if (param == KMType.GCM) { if (op.getPadding() != KMType.PADDING_NONE || op.getPadding() == KMType.PKCS7) { KMException.throwIt(KMError.INCOMPATIBLE_PADDING_MODE); @@ -2328,9 +2323,6 @@ private void authorizeBlockModeAndMacLength(KMOperationState op) { default: KMException.throwIt(KMError.UNSUPPORTED_BLOCK_MODE); } - if (param == KMType.INVALID_VALUE) { - KMException.throwIt(KMError.INVALID_ARGUMENT); - } break; case KMType.HMAC: if (macLen == KMType.INVALID_VALUE) { @@ -2346,9 +2338,7 @@ private void authorizeBlockModeAndMacLength(KMOperationState op) { < KMIntegerTag.getShortValue( KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, data[HW_PARAMETERS])) { KMException.throwIt(KMError.INVALID_MAC_LENGTH); - } else if (macLen - > KMIntegerTag.getShortValue( - KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, data[HW_PARAMETERS])) { + } else if (macLen % 8 != 0 || macLen > 256) { KMException.throwIt(KMError.UNSUPPORTED_MAC_LENGTH); } op.setMacLength(macLen); @@ -2964,7 +2954,10 @@ private void validateImportKey(short params, short keyFmt){ // As per specification, Early boot keys may not be imported at all, if Tag::EARLY_BOOT_ONLY is // provided to IKeyMintDevice::importKey KMTag.assertAbsence(params, KMType.BOOL_TAG, KMType.EARLY_BOOT_ONLY, KMError.EARLY_BOOT_ENDED); - + //Check if the tags are supported. + if (KMKeyParameters.hasUnsupportedTags(data[KEY_PARAMETERS])) { + KMException.throwIt(KMError.UNSUPPORTED_TAG); + } // Algorithm must be present KMTag.assertPresence(params, KMType.ENUM_TAG, KMType.ALGORITHM, KMError.INVALID_ARGUMENT); short alg = KMEnumTag.getValue(KMType.ALGORITHM, params); @@ -3080,7 +3073,6 @@ private void importECKeys(byte[] scratchPad) { Util.setShort(scratchPad, index, curve); index += 2; } - // Check whether key can be created seProvider.importAsymmetricKey( KMType.EC, @@ -3107,11 +3099,18 @@ private void importHmacKey(byte[] scratchPad) { KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.KEYSIZE, data[KEY_PARAMETERS]); if (keysize != KMType.INVALID_VALUE) { if (!(keysize >= 64 && keysize <= 512 && keysize % 8 == 0)) { + KMException.throwIt(KMError.UNSUPPORTED_KEY_SIZE); + } + if (keysize != (short) (KMByteBlob.length(data[SECRET]) * 8)) { KMException.throwIt(KMError.IMPORT_PARAMETER_MISMATCH); } } else { // add the key size to scratchPad - keysize = KMInteger.uint_16((short) (KMByteBlob.length(data[SECRET]) * 8)); + keysize = (short) (KMByteBlob.length(data[SECRET]) * 8); + if (!(keysize >= 64 && keysize <= 512 && keysize % 8 == 0)) { + KMException.throwIt(KMError.UNSUPPORTED_KEY_SIZE); + } + keysize = KMInteger.uint_16(keysize); short keySizeTag = KMIntegerTag.instance(KMType.UINT_TAG, KMType.KEYSIZE, keysize); Util.setShort(scratchPad, index, keySizeTag); index += 2; @@ -3140,15 +3139,24 @@ private void importTDESKey(byte[] scratchPad) { KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.KEYSIZE, data[KEY_PARAMETERS]); if (keysize != KMType.INVALID_VALUE) { if (keysize != 168) { - KMException.throwIt(KMError.UNSUPPORTED_KEY_SIZE); + KMException.throwIt(KMError.UNSUPPORTED_KEY_SIZE); + } + if(192 != (short)( 8 * KMByteBlob.length(data[SECRET]))) { + KMException.throwIt(KMError.IMPORT_PARAMETER_MISMATCH); } } else { + keysize = (short) (KMByteBlob.length(data[SECRET]) * 8); + if (keysize != 192) { + KMException.throwIt(KMError.UNSUPPORTED_KEY_SIZE); + } // add the key size to scratchPad keysize = KMInteger.uint_16((short) 168); short keysizeTag = KMIntegerTag.instance(KMType.UINT_TAG, KMType.KEYSIZE, keysize); Util.setShort(scratchPad, index, keysizeTag); index += 2; } + // Read Minimum Mac length - it must not be present + KMTag.assertAbsence(data[KEY_PARAMETERS],KMType.UINT_TAG, KMType.MIN_MAC_LENGTH,KMError.INVALID_TAG); // Check whether key can be created seProvider.importSymmetricKey( KMType.DES, @@ -3158,8 +3166,6 @@ private void importTDESKey(byte[] scratchPad) { KMByteBlob.length(data[SECRET])); // update the key parameters list updateKeyParameters(scratchPad, index); - // validate TDES Key parameters - validateTDESKey(); data[KEY_BLOB] = KMArray.instance((short) 4); } @@ -3178,6 +3184,9 @@ private void importAESKey(byte[] scratchPad) { short keysize = KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.KEYSIZE, data[KEY_PARAMETERS]); if (keysize != KMType.INVALID_VALUE) { + if(keysize != (short)( 8 * KMByteBlob.length(data[SECRET]))) { + KMException.throwIt(KMError.IMPORT_PARAMETER_MISMATCH); + } validateAesKeySize(keysize); } else { // add the key size to scratchPad @@ -3215,7 +3224,7 @@ private void importRSAKey(byte[] scratchPad) { } if(Util.arrayCompare(F4, (short)0, KMByteBlob.getBuffer(pubKeyExp), KMByteBlob.getStartOff(pubKeyExp), (short)F4.length) != 0){ - KMException.throwIt(KMError.INVALID_ARGUMENT); + KMException.throwIt(KMError.IMPORT_PARAMETER_MISMATCH); } short index = 0; // index in scratchPad for update parameters. // validate public exponent if present in key params - it must be 0x010001 @@ -3226,7 +3235,7 @@ private void importRSAKey(byte[] scratchPad) { KMType.ULONG_TAG, KMType.RSA_PUBLIC_EXPONENT, data[KEY_PARAMETERS]); - if (len != KMTag.INVALID_VALUE) { + if (len != KMTag.INVALID_VALUE) { if (len != 4 || Util.getShort(scratchPad, (short) 10) != 0x01 || Util.getShort(scratchPad, (short) 12) != 0x01) { @@ -3246,12 +3255,16 @@ private void importRSAKey(byte[] scratchPad) { // check the keysize tag if present in key parameters. short keysize = KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.KEYSIZE, data[KEY_PARAMETERS]); + short kSize = (short) (KMByteBlob.length(data[SECRET]) * 8); if (keysize != KMType.INVALID_VALUE) { if (keysize != 2048 - || keysize != (short) (KMByteBlob.length(data[SECRET]) * 8)) { + || keysize != kSize ) { KMException.throwIt(KMError.IMPORT_PARAMETER_MISMATCH); } } else { + if(2048 != kSize){ + KMException.throwIt(KMError.IMPORT_PARAMETER_MISMATCH); + } // add the key size to scratchPad keysize = KMInteger.uint_16((short) 2048); keysize = KMIntegerTag.instance(KMType.UINT_TAG, KMType.KEYSIZE, keysize); @@ -3272,7 +3285,6 @@ private void importRSAKey(byte[] scratchPad) { // update the key parameters list updateKeyParameters(scratchPad, index); // validate RSA Key parameters - validateRSAKey(scratchPad); data[KEY_BLOB] = KMArray.instance((short) 5); KMArray.add(data[KEY_BLOB], KEY_BLOB_PUB_KEY, data[PUB_KEY]); } @@ -3397,11 +3409,15 @@ private void processGenerateKey(APDU apdu) { KMTag.assertAbsence(data[KEY_PARAMETERS], KMType.BOOL_TAG,KMType.ROLLBACK_RESISTANCE, KMError.ROLLBACK_RESISTANCE_UNAVAILABLE); // BOOTLOADER_ONLY keys not supported. KMTag.assertAbsence(data[KEY_PARAMETERS], KMType.BOOL_TAG, KMType.BOOTLOADER_ONLY, KMError.INVALID_KEY_BLOB); + // Algorithm must be present + KMTag.assertPresence(data[KEY_PARAMETERS], KMType.ENUM_TAG, KMType.ALGORITHM, KMError.INVALID_ARGUMENT); // As per specification Early boot keys may be created after early boot ended. validateEarlyBoot(); - // Algorithm must be present validatePurpose(data[KEY_PARAMETERS]); - KMTag.assertPresence(data[KEY_PARAMETERS], KMType.ENUM_TAG, KMType.ALGORITHM, KMError.INVALID_ARGUMENT); + //Check if the tags are supported. + if (KMKeyParameters.hasUnsupportedTags(data[KEY_PARAMETERS])) { + KMException.throwIt(KMError.UNSUPPORTED_TAG); + } short alg = KMEnumTag.getValue(KMType.ALGORITHM, data[KEY_PARAMETERS]); // Check algorithm and dispatch to appropriate handler. switch (alg) { @@ -3748,7 +3764,7 @@ private static void validateHmacKey() { KMException.throwIt(KMError.UNSUPPORTED_DIGEST); } // Read Minimum Mac length - KMTag.assertPresence(data[KEY_PARAMETERS],KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, KMError.MISSING_MAC_LENGTH); + KMTag.assertPresence(data[KEY_PARAMETERS],KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, KMError.MISSING_MIN_MAC_LENGTH); short minMacLength = KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, data[KEY_PARAMETERS]); diff --git a/Applet/src/com/android/javacard/kmdevice/KMKeymintDevice.java b/Applet/src/com/android/javacard/kmdevice/KMKeymintDevice.java index 6c159c38..461d4b59 100644 --- a/Applet/src/com/android/javacard/kmdevice/KMKeymintDevice.java +++ b/Applet/src/com/android/javacard/kmdevice/KMKeymintDevice.java @@ -404,7 +404,7 @@ public void updateAAD(KMOperationState op, byte finish) { public void validatePurpose(short params) { short attKeyPurpose = KMKeyParameters.findTag(params, KMType.ENUM_ARRAY_TAG, KMType.PURPOSE); - // ATTEST_KEY cannot be combined with any other purpose. + // ATTEST_KEY purpose cannot be combined with any other purpose. if (attKeyPurpose != KMType.INVALID_VALUE && KMEnumArrayTag.contains(attKeyPurpose, KMType.ATTEST_KEY) && KMEnumArrayTag.length(attKeyPurpose) > 1) { diff --git a/Applet/src/com/android/javacard/kmdevice/KMTag.java b/Applet/src/com/android/javacard/kmdevice/KMTag.java index 670ada1d..abfbcee9 100644 --- a/Applet/src/com/android/javacard/kmdevice/KMTag.java +++ b/Applet/src/com/android/javacard/kmdevice/KMTag.java @@ -79,6 +79,7 @@ public static boolean isValidPublicExponent(short params) { if(pubExp == KMType.INVALID_VALUE){ return false; } + // Only exponent support is F4 - 65537 which is 0x00010001. pubExp = KMIntegerTag.getValue(pubExp); if(!(KMInteger.getShort(pubExp) == 0x01 && KMInteger.getSignificantShort(pubExp) == 0x01)){