From e1cae02a2cbafcafd1de05813ee136b076ad04b0 Mon Sep 17 00:00:00 2001 From: bvenkateswarlu Date: Fri, 16 Apr 2021 19:48:16 +0100 Subject: [PATCH 01/21] Move operationtable to RAM and notifiy back to HAL about se reset --- .../android/javacard/keymaster/KMEncoder.java | 17 +-- .../javacard/keymaster/KMKeymasterApplet.java | 101 +++++++++++------- .../javacard/keymaster/KMOperationState.java | 39 +++---- 3 files changed, 84 insertions(+), 73 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMEncoder.java b/Applet/src/com/android/javacard/keymaster/KMEncoder.java index 47e6d305..0dc31b56 100644 --- a/Applet/src/com/android/javacard/keymaster/KMEncoder.java +++ b/Applet/src/com/android/javacard/keymaster/KMEncoder.java @@ -90,24 +90,27 @@ public short encode(short object, byte[] buffer, short startOff) { } // array{KMError.OK,Array{KMByteBlobs}} - public void encodeCertChain(byte[] buffer, short offset, short length) { + public void encodeCertChain(byte[] buffer, short offset, short length, short errInt32Ptr) { bufferRef[0] = buffer; scratchBuf[START_OFFSET] = offset; - scratchBuf[LEN_OFFSET] = (short) (offset + 3); + scratchBuf[LEN_OFFSET] += (offset + 1); + //Total length is ArrayHeader + UIntHeader + length(errInt32Ptr) + scratchBuf[LEN_OFFSET] += (short) (2 + KMInteger.cast(errInt32Ptr).length()); writeMajorTypeWithLength(ARRAY_TYPE, (short) 2); // Array of 2 elements - writeByte(UINT_TYPE); // Error.OK + encodeInteger(errInt32Ptr); } //array{KMError.OK,Array{KMByteBlobs}} - public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, short certLength) { + public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, short certLength, short errInt32Ptr) { bufferRef[0] = certBuffer; scratchBuf[START_OFFSET] = certStart; scratchBuf[LEN_OFFSET] = (short) (certStart + 1); //Array header - 2 elements i.e. 1 byte scratchBuf[START_OFFSET]--; - // Error.Ok - 1 byte - scratchBuf[START_OFFSET]--; + // errInt32Ptr - CanaryBit + ErrorCode - 4 bytes + // Integer header - 1 byte + scratchBuf[START_OFFSET] -= (KMInteger.cast(errInt32Ptr).length() + 1); //Array header - 2 elements i.e. 1 byte scratchBuf[START_OFFSET]--; // Cert Byte blob - typically 2 bytes length i.e. 3 bytes header @@ -120,7 +123,7 @@ public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, s ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); } writeMajorTypeWithLength(ARRAY_TYPE, (short) 2); // Array of 2 elements - writeByte(UINT_TYPE); // Error.OK + encodeInteger(errInt32Ptr); //CanaryBit + ErrorCode writeMajorTypeWithLength(ARRAY_TYPE, (short) 1); // Array of 1 element writeMajorTypeWithLength(BYTES_TYPE, certLength); // Cert Byte Blob of length return bufferStart; diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index b1a78178..459a678a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -41,6 +41,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe private static final short KM_HAL_VERSION = (short) 0x4000; private static final short MAX_AUTH_DATA_SIZE = (short) 512; private static final short DERIVE_KEY_INPUT_SIZE = (short) 256; + private static final short CANARY_BIT_FLAG = (short) 0x40; // "Keymaster HMAC Verification" - used for HMAC key verification. public static final byte[] sharingCheck = { @@ -214,6 +215,7 @@ private void initializeTransientArrays() { bufferProp[BUF_START_OFFSET] = 0; bufferProp[BUF_LEN_OFFSET] = 0; } + /** * Selects this applet. * @@ -644,17 +646,20 @@ private void processAddRngEntropyCmd(APDU apdu) { private void processGetCertChainCmd(APDU apdu) { // Make the response tmpVariables[0] = seProvider.getCertificateChainLength(); - // Add arrayHeader and KMError.OK - tmpVariables[0] += 2; + short int32Ptr = getErrorStatusWithCanaryBitSet(KMError.OK); + //Total Extra length + // Add arrayHeader and (CanaryBitSet + KMError.OK) + tmpVariables[2] = (short) (2 + KMInteger.cast(int32Ptr).length()); + tmpVariables[0] += tmpVariables[2]; tmpVariables[1] = KMByteBlob.instance(tmpVariables[0]); bufferRef[0] = KMByteBlob.cast(tmpVariables[1]).getBuffer(); bufferProp[BUF_START_OFFSET] = KMByteBlob.cast(tmpVariables[1]).getStartOff(); bufferProp[BUF_LEN_OFFSET] = KMByteBlob.cast(tmpVariables[1]).length(); // read the cert chain from non-volatile memory. Cert chain is already in // CBOR format. - seProvider.readCertificateChain((byte[]) bufferRef[0], (short) (bufferProp[BUF_START_OFFSET] + 2)); + seProvider.readCertificateChain((byte[]) bufferRef[0], (short) (bufferProp[BUF_START_OFFSET] + tmpVariables[2])); // Encode cert chain. - encoder.encodeCertChain((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], bufferProp[BUF_LEN_OFFSET]); + encoder.encodeCertChain((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], bufferProp[BUF_LEN_OFFSET], int32Ptr); sendOutgoing(apdu); } @@ -837,7 +842,7 @@ private void processProvisionSharedSecretCmd(APDU apdu) { private void processGetProvisionStatusCmd(APDU apdu) { tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, KMInteger.uint_16(provisionStatus)); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -912,7 +917,7 @@ private void processGetKeyCharacteristicsCmd(APDU apdu) { checkVersionAndPatchLevel(scratchPad); // make response. tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_CHARACTERISTICS]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -929,7 +934,7 @@ private void processGetHmacSharingParamCmd(APDU apdu) { KMHmacSharingParameters.cast(tmpVariables[2]).setSeed(KMByteBlob.instance((short) 0)); // prepare the response tmpVariables[3] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[3]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[3]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[3]).add((short) 1, tmpVariables[2]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1095,7 +1100,7 @@ private void processComputeSharedHmacCmd(APDU apdu) { tmpVariables[1] = KMByteBlob.instance(scratchPad, tmpVariables[6], tmpVariables[5]); // prepare the response tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, tmpVariables[1]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1113,8 +1118,8 @@ private boolean isKeyUpgradeRequired(short tag, short systemParam) { // OS version in key characteristics must be less the OS version stored in Javacard or the // stored version must be zero. Then only upgrade is allowed else it is invalid argument. if ((tag == KMType.OS_VERSION - && KMInteger.compare(tmpVariables[0], systemParam) == 1 - && KMInteger.compare(systemParam, tmpVariables[1]) == 0)) { + && KMInteger.compare(tmpVariables[0], systemParam) == 1 + && KMInteger.compare(systemParam, tmpVariables[1]) == 0)) { // Key needs upgrade. return true; } else if ((KMInteger.compare(tmpVariables[0], systemParam) == -1)) { @@ -1173,7 +1178,7 @@ private void processUpgradeKeyCmd(APDU apdu) { } // prepare the response tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1450,7 +1455,8 @@ private void processAttestKeyCmd(APDU apdu) { cert.buffer((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], bufferProp[BUF_LEN_OFFSET]); cert.build(); bufferProp[BUF_START_OFFSET] = - encoder.encodeCert((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], cert.getCertStart(), cert.getCertLength()); + encoder.encodeCert((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], cert.getCertStart(), cert.getCertLength(), + getErrorStatusWithCanaryBitSet(KMError.OK)); bufferProp[BUF_LEN_OFFSET] = (short) (cert.getCertLength() + (cert.getCertStart() - bufferProp[BUF_START_OFFSET])); sendOutgoing(apdu); } @@ -1615,7 +1621,7 @@ private void processFinishOperationCmd(APDU apdu) { if (data[OUTPUT_DATA] == KMType.INVALID_VALUE) { data[OUTPUT_DATA] = KMByteBlob.instance((short) 0); } - KMArray.cast(tmpVariables[2]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[2]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 2, data[OUTPUT_DATA]); @@ -1773,18 +1779,18 @@ private void finishSigningVerifyingOperation(KMOperationState op, byte[] scratch if (op.getPurpose() == KMType.SIGN) { // len of signature will be 256 bytes short len = op.getOperation().sign( - KMByteBlob.cast(data[INPUT_DATA]).getBuffer(), - KMByteBlob.cast(data[INPUT_DATA]).getStartOff(), - KMByteBlob.cast(data[INPUT_DATA]).length(), scratchPad, - (short) 0); + KMByteBlob.cast(data[INPUT_DATA]).getBuffer(), + KMByteBlob.cast(data[INPUT_DATA]).getStartOff(), + KMByteBlob.cast(data[INPUT_DATA]).length(), scratchPad, + (short) 0); // Maximum output size of signature is 256 bytes. data[OUTPUT_DATA] = KMByteBlob.instance((short) 256); Util.arrayCopyNonAtomic( - scratchPad, - (short) 0, - KMByteBlob.cast(data[OUTPUT_DATA]).getBuffer(), - (short) (KMByteBlob.cast(data[OUTPUT_DATA]).getStartOff() + 256 - len), - len); + scratchPad, + (short) 0, + KMByteBlob.cast(data[OUTPUT_DATA]).getBuffer(), + (short) (KMByteBlob.cast(data[OUTPUT_DATA]).getStartOff() + 256 - len), + len); } else { KMException.throwIt(KMError.UNSUPPORTED_PURPOSE); } @@ -2097,7 +2103,7 @@ private void processUpdateOperationCmd(APDU apdu) { if (data[OUTPUT_DATA] == KMType.INVALID_VALUE) { data[OUTPUT_DATA] = KMByteBlob.instance((short) 0); } - KMArray.cast(tmpVariables[2]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[2]).add((short) 1, KMInteger.uint_16(tmpVariables[3])); KMArray.cast(tmpVariables[2]).add((short) 2, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 3, data[OUTPUT_DATA]); @@ -2203,7 +2209,7 @@ private void processBeginOperationCmd(APDU apdu) { } tmpVariables[1] = KMKeyParameters.instance(tmpVariables[2]); tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[0]).add((short) 2, data[OP_HANDLE]); @@ -2813,7 +2819,7 @@ private void importKey(APDU apdu, byte[] scratchPad) { // prepare the response tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); KMArray.cast(tmpVariables[0]).add((short) 2, data[KEY_CHARACTERISTICS]); @@ -3315,7 +3321,7 @@ private static void processGenerateKey(APDU apdu) { // prepare the response tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, KMInteger.uint_16(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); KMArray.cast(tmpVariables[0]).add((short) 2, data[KEY_CHARACTERISTICS]); @@ -3623,20 +3629,20 @@ private static void parseEncryptedKeyBlob(byte[] scratchPad) { tmpVariables[0] = KMByteBlob.cast(data[KEY_BLOB]).getStartOff(); tmpVariables[1] = KMArray.instance((short) 5); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_SECRET, - KMByteBlob.exp()); + KMByteBlob.exp()); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_AUTH_TAG, - KMByteBlob.exp()); + KMByteBlob.exp()); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_NONCE, - KMByteBlob.exp()); + KMByteBlob.exp()); tmpVariables[2] = KMKeyCharacteristics.exp(); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_KEYCHAR, - tmpVariables[2]); + tmpVariables[2]); KMArray.cast(tmpVariables[1]).add(KMKeymasterApplet.KEY_BLOB_PUB_KEY, - KMByteBlob.exp()); + KMByteBlob.exp()); data[KEY_BLOB] = decoder.decodeArray(tmpVariables[1], - KMByteBlob.cast(data[KEY_BLOB]).getBuffer(), - KMByteBlob.cast(data[KEY_BLOB]).getStartOff(), - KMByteBlob.cast(data[KEY_BLOB]).length()); + KMByteBlob.cast(data[KEY_BLOB]).getBuffer(), + KMByteBlob.cast(data[KEY_BLOB]).getStartOff(), + KMByteBlob.cast(data[KEY_BLOB]).length()); tmpVariables[0] = KMArray.cast(data[KEY_BLOB]).length(); if (tmpVariables[0] < 4) { KMException.throwIt(KMError.INVALID_KEY_BLOB); @@ -3647,18 +3653,18 @@ private static void parseEncryptedKeyBlob(byte[] scratchPad) { data[NONCE] = KMArray.cast(data[KEY_BLOB]).get(KEY_BLOB_NONCE); data[SECRET] = KMArray.cast(data[KEY_BLOB]).get(KEY_BLOB_SECRET); data[KEY_CHARACTERISTICS] = KMArray.cast(data[KEY_BLOB]).get( - KEY_BLOB_KEYCHAR); + KEY_BLOB_KEYCHAR); data[PUB_KEY] = KMType.INVALID_VALUE; if (tmpVariables[0] == 5) { data[PUB_KEY] = KMArray.cast(data[KEY_BLOB]).get(KEY_BLOB_PUB_KEY); } data[HW_PARAMETERS] = KMKeyCharacteristics - .cast(data[KEY_CHARACTERISTICS]).getHardwareEnforced(); + .cast(data[KEY_CHARACTERISTICS]).getHardwareEnforced(); data[SW_PARAMETERS] = KMKeyCharacteristics - .cast(data[KEY_CHARACTERISTICS]).getSoftwareEnforced(); + .cast(data[KEY_CHARACTERISTICS]).getSoftwareEnforced(); data[HIDDEN_PARAMETERS] = KMKeyParameters.makeHidden(data[APP_ID], - data[APP_DATA], data[ROT], scratchPad); + data[APP_DATA], data[ROT], scratchPad); // make auth data makeAuthData(scratchPad); // Decrypt Secret and verify auth tag @@ -3823,9 +3829,26 @@ private static short deriveKey(byte[] scratchPad) { return tmpVariables[3]; } + private static short getErrorStatusWithCanaryBitSet(short err) { + short int32Ptr = KMInteger.instance((short) 4); + if (repository.isResetEventOccurred()) { + repository.resetSeStatusFlag(); + //Set the canary bit + Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), + KMInteger.cast(int32Ptr).getStartOff(), + CANARY_BIT_FLAG); + } + Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), + (short) (KMInteger.cast(int32Ptr).getStartOff() + 2), + err); + return int32Ptr; + } + private static void sendError(APDU apdu, short err) { bufferProp[BUF_START_OFFSET] = repository.alloc((short) 2); - bufferProp[BUF_LEN_OFFSET] = encoder.encodeError(err, (byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], (short) 5); + short int32Ptr = getErrorStatusWithCanaryBitSet(err); + bufferProp[BUF_LEN_OFFSET] = encoder.encode(int32Ptr, (byte[]) bufferRef[0], + bufferProp[BUF_START_OFFSET]); sendOutgoing(apdu); } diff --git a/Applet/src/com/android/javacard/keymaster/KMOperationState.java b/Applet/src/com/android/javacard/keymaster/KMOperationState.java index 3fbe47f2..d8705de4 100644 --- a/Applet/src/com/android/javacard/keymaster/KMOperationState.java +++ b/Applet/src/com/android/javacard/keymaster/KMOperationState.java @@ -28,11 +28,7 @@ public class KMOperationState { public static final byte MAX_DATA = 20; - public static final byte MAX_REFS = 1; - private static final byte DATA = 0; - private static final byte REFS = 1; - private static final byte KMOPERATION = 0; - private static final byte SLOT = 1; + private static final byte OPERATION = 0; private static final byte TRUE = 1; private static final byte FALSE = 0; // byte type @@ -56,7 +52,6 @@ public class KMOperationState { private static final byte AES_GCM_UPDATE_ALLOWED = 8; // Object References - private static final byte OPERATION = 0; private byte[] data; private Object[] objRefs; private static KMOperationState prototype; @@ -64,7 +59,7 @@ public class KMOperationState { private KMOperationState() { data = JCSystem.makeTransientByteArray(MAX_DATA, JCSystem.CLEAR_ON_RESET); - objRefs = JCSystem.makeTransientObjectArray((short) 2, JCSystem.CLEAR_ON_RESET); + objRefs = JCSystem.makeTransientObjectArray((short) 1, JCSystem.CLEAR_ON_RESET); isDataUpdated = JCSystem.makeTransientByteArray((short) 1, JCSystem.CLEAR_ON_RESET); } @@ -75,22 +70,19 @@ private static KMOperationState proto() { return prototype; } - public static KMOperationState instance(short opHandle, Object[] slot) { + public static KMOperationState instance(short opHandle) { KMOperationState opState = proto(); opState.reset(); Util.setShort(prototype.data, OP_HANDLE, opHandle); - prototype.objRefs[SLOT] = slot; return opState; } - public static KMOperationState read(byte[] oprHandle, short off, Object[] slot) { + public static KMOperationState read(byte[] oprHandle, short off, byte[] data, short dataOff, Object opr) { KMOperationState opState = proto(); opState.reset(); - Util.arrayCopy((byte[]) slot[DATA], (short) 0, prototype.data, (short) 0, (short) prototype.data.length); - Object[] ops = ((Object[]) slot[REFS]); - prototype.objRefs[KMOPERATION] = ops[OPERATION]; + Util.arrayCopy(data, dataOff, prototype.data, (short) 0, (short) prototype.data.length); + prototype.objRefs[OPERATION] = opr; Util.setShort(prototype.data, OP_HANDLE, KMInteger.uint_64(oprHandle, off)); - prototype.objRefs[SLOT] = slot; return opState; } @@ -100,7 +92,7 @@ public void persist() { } KMRepository.instance().persistOperation(data, Util.getShort(data, OP_HANDLE), - (KMOperation) objRefs[KMOPERATION]); + (KMOperation) objRefs[OPERATION]); isDataUpdated[0] = FALSE; } @@ -114,8 +106,7 @@ public short getKeySize() { public void reset() { isDataUpdated[0] = FALSE; - objRefs[KMOPERATION] = null; - objRefs[SLOT] = null; + objRefs[OPERATION] = null; Util.arrayFillNonAtomic( data, (short) 0, (short) data.length, (byte) 0); } @@ -125,14 +116,8 @@ private void dataUpdated() { } public void release() { - Object[] slots = (Object[]) objRefs[SLOT]; - Object[] ops = ((Object[]) slots[REFS]); - ((KMOperation) ops[OPERATION]).abort(); - JCSystem.beginTransaction(); - Util.arrayFillNonAtomic( - (byte[]) slots[0], (short) 0, (short) ((byte[]) slots[0]).length, (byte) 0); - ops[OPERATION] = null; - JCSystem.commitTransaction(); + if (objRefs[OPERATION] != null) + ((KMOperation) objRefs[OPERATION]).abort(); reset(); } @@ -150,13 +135,13 @@ public void setPurpose(byte purpose) { } public void setOperation(KMOperation opr) { - objRefs[KMOPERATION] = opr; + objRefs[OPERATION] = opr; dataUpdated(); persist(); } public KMOperation getOperation() { - return (KMOperation) objRefs[KMOPERATION]; + return (KMOperation) objRefs[OPERATION]; } public boolean isAuthPerOperationReqd() { From db72f539d893a04f74978171d948aee6b5db0b8d Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Sat, 17 Apr 2021 00:23:16 +0530 Subject: [PATCH 02/21] Handle the reset event from Applet --- .../4.1/JavacardKeymaster4Device.cpp | 57 ++++++++++++++----- HAL/keymaster/include/CborConverter.h | 9 +-- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index eb8ccdd8..2e3f97b3 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -51,6 +51,9 @@ #define INS_END_KM_CMD 0x7F #define SW_KM_OPR 0UL #define SB_KM_OPR 1UL +#define CANARY_BIT_FLAG ( 1 << 30) +#define UNSET_CANARY_BIT(a) (a &= ~CANARY_BIT_FLAG) +#define TWOS_COMPLIMENT(a) (a = ~a + 1) namespace keymaster { namespace V4_1 { @@ -181,18 +184,6 @@ static T translateExtendedErrorsToHalErrors(T& errorCode) { return err; } -template -static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response, bool - hasErrorCode) { - std::unique_ptr item(nullptr); - T errorCode = T::OK; - std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); - - if (T::OK != errorCode) - errorCode = translateExtendedErrorsToHalErrors(errorCode); - return {std::move(item), errorCode}; -} - /* Generate new operation handle */ static ErrorCode generateOperationHandle(uint64_t& oprHandle) { std::map>::iterator it; @@ -240,6 +231,43 @@ static void deleteOprHandleEntry(uint64_t halGeneratedOperationHandle) { operationTable.erase(halGeneratedOperationHandle); } +/* Clears all the strongbox operation handle entries from operation table */ +static void clearStrongboxOprHandleEntries() { + std::map>::iterator it = operationTable.begin(); + for(; it != operationTable.end(); ++it) { + if (isStrongboxOperation(it->first)) + operationTable.erase(it); + } +} + +template +static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response, bool + hasErrorCode) { + std::unique_ptr item(nullptr); + T errorCode = T::OK; + std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); + int32_t temp = static_cast(errorCode); + + //Check if secure element is reset + bool canaryBitSet = (0 != (temp & CANARY_BIT_FLAG)); + + if (canaryBitSet) { + //Clear the operation table for Strongbox operations entries. + clearStrongboxOprHandleEntries(); + UNSET_CANARY_BIT(temp); + } + // SE sends errocode as unsigned value so convert the unsigned value + // into a signed value of same magnitude. + TWOS_COMPLIMENT(temp); + + //Write back temp to errorCode + errorCode = static_cast(temp); + + if (T::OK != errorCode) + errorCode = translateExtendedErrorsToHalErrors(errorCode); + return {std::move(item), errorCode}; +} + ErrorCode encodeParametersVerified(const VerificationToken& verificationToken, std::vector& asn1ParamsVerified) { if (verificationToken.parametersVerified.size() > 0) { AuthorizationSet paramSet; @@ -394,7 +422,7 @@ static bool isSEProvisioned() { return ret; } //Check if SE is provisioned. - std::tie(item, errorCode) = cborConverter.decodeData(std::vector(response.begin(), response.end()-2), + std::tie(item, errorCode) = decodeData(cborConverter, std::vector(response.begin(), response.end()-2), true); if(item != NULL) { uint64_t status; @@ -463,7 +491,7 @@ Return JavacardKeymaster4Device::getHardwareInfo(getHardwareInfo_cb _hidl_ ErrorCode ret = sendData(Instruction::INS_GET_HW_INFO_CMD, input, resp); if (ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, ret) = cborConverter_.decodeData(std::vector(resp.begin(), resp.end()-2), + std::tie(item, ret) = decodeData(cborConverter_, std::vector(resp.begin(), resp.end()-2), false); if (item != nullptr) { std::vector temp; @@ -630,7 +658,6 @@ Return JavacardKeymaster4Device::generateKey(const hidl_vec& std::vector cborData = array.encode(); errorCode = sendData(Instruction::INS_GENERATE_KEY_CMD, cborData, cborOutData); - if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), diff --git a/HAL/keymaster/include/CborConverter.h b/HAL/keymaster/include/CborConverter.h index ecbe2783..45855244 100644 --- a/HAL/keymaster/include/CborConverter.h +++ b/HAL/keymaster/include/CborConverter.h @@ -65,7 +65,7 @@ class CborConverter } else if (MajorType::UINT == getType(item)) { uint64_t err; if(getUint64(item, err)) { - errorCode = static_cast(get2sCompliment(static_cast(err))); + errorCode = static_cast(err); } item = nullptr; /*Already read the errorCode. So no need of sending item to client */ } @@ -162,18 +162,13 @@ class CborConverter if (!getUint64(item, pos, errorVal)) { return ret; } - errorCode = static_cast(get2sCompliment(static_cast(errorVal))); + errorCode = static_cast(errorVal); ret = true; return ret; } private: - /** - * Returns the negative value of the same number. - */ - inline int32_t get2sCompliment(uint32_t value) { return static_cast(~value+1); } - /** * Get the type of the Item pointer. */ From 9d922da05905f58d8ef10d4e1d5e5f2df0648333 Mon Sep 17 00:00:00 2001 From: bvenkateswarlu Date: Fri, 16 Apr 2021 19:58:00 +0100 Subject: [PATCH 03/21] Move operationtable to RAM --- .../android/javacard/keymaster/KMEncoder.java | 2 +- .../javacard/keymaster/KMRepository.java | 146 ++++++++++-------- 2 files changed, 80 insertions(+), 68 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMEncoder.java b/Applet/src/com/android/javacard/keymaster/KMEncoder.java index 0dc31b56..b79349ff 100644 --- a/Applet/src/com/android/javacard/keymaster/KMEncoder.java +++ b/Applet/src/com/android/javacard/keymaster/KMEncoder.java @@ -56,7 +56,7 @@ public KMEncoder() { bufferRef[0] = null; scratchBuf[START_OFFSET] = (short) 0; scratchBuf[LEN_OFFSET] = (short) 0; - scratchBuf[STACK_PTR_OFFSET] = (short) 0; + scratchBuf[STACK_PTR_OFFSET] = (short) 0; } private void push(short objPtr) { diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 07caec72..9e365ada 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -91,6 +91,16 @@ public class KMRepository implements KMUpgradable { private short dataIndex; private short[] reclaimIndex; + // Operation table. + private static final short OPER_TABLE_DATA_OFFSET = 0; + private static final short OPER_TABLE_OPR_OFFSET = 1; + private static final short OPER_DATA_LEN = OPERATION_HANDLE_ENTRY_SIZE + KMOperationState.MAX_DATA + 1; + private static final short DATA_ARRAY_LENGTH = MAX_OPS * OPER_DATA_LEN; + // Last byte in the operation data table represents the SE Reset flag. + private static final short SE_RESET_FLAG_OFFSET = DATA_ARRAY_LENGTH - 1; + public static final byte SE_RESET_STATUS_FLAG = 0x7F; + + // Singleton instance private static KMRepository repository; @@ -105,17 +115,13 @@ public KMRepository(boolean isUpgrading) { heapIndex[0] = (short) 0; reclaimIndex[0] = HEAP_SIZE; newDataTable(isUpgrading); - operationStateTable = new Object[MAX_OPS]; - // create and initialize operation state table. - //First byte in the operation handle buffer denotes whether the operation is - //reserved or unreserved. - byte index = 0; - while (index < MAX_OPS) { - operationStateTable[index] = new Object[]{new byte[OPERATION_HANDLE_ENTRY_SIZE], - new Object[]{new byte[KMOperationState.MAX_DATA], - new Object[KMOperationState.MAX_REFS]}}; - index++; - } + + operationStateTable = new Object[2]; + operationStateTable[0] = JCSystem.makeTransientByteArray(DATA_ARRAY_LENGTH, JCSystem.CLEAR_ON_RESET); + operationStateTable[1] = JCSystem.makeTransientObjectArray(MAX_OPS, JCSystem.CLEAR_ON_RESET); + //Set the reset flag + resetSeStatusFlag(); + //Initialize the device locked status if (!isUpgrading) { setDeviceLock(false); @@ -124,6 +130,17 @@ public KMRepository(boolean isUpgrading) { repository = this; } + public void resetSeStatusFlag() { + ((byte[]) operationStateTable[0])[SE_RESET_FLAG_OFFSET] = SE_RESET_STATUS_FLAG; + } + + public boolean isResetEventOccurred() { + if (((byte[]) operationStateTable[0])[SE_RESET_FLAG_OFFSET] == SE_RESET_STATUS_FLAG) { + return false; + } + return true; + } + public void getOperationHandle(short oprHandle, byte[] buf, short off, short len) { if (KMInteger.cast(oprHandle).length() != OPERATION_HANDLE_SIZE) { KMException.throwIt(KMError.INVALID_OPERATION_HANDLE); @@ -133,17 +150,19 @@ public void getOperationHandle(short oprHandle, byte[] buf, short off, short len public KMOperationState findOperation(byte[] buf, short off, short len) { short index = 0; - byte[] opId; + byte[] oprTableData; + short offset = 0; + oprTableData = (byte[]) operationStateTable[OPER_TABLE_DATA_OFFSET]; + Object[] operations = (Object[]) operationStateTable[OPER_TABLE_OPR_OFFSET]; while (index < MAX_OPS) { - opId = ((byte[]) ((Object[]) operationStateTable[index])[0]); - if (0 == Util.arrayCompare(buf, off, opId, OPERATION_HANDLE_OFFSET, len)) { - return KMOperationState - .read(opId, OPERATION_HANDLE_OFFSET, - (Object[]) ((Object[]) operationStateTable[index])[1]); + offset = (short) (index * OPER_DATA_LEN); + if (0 == Util.arrayCompare(buf, off, oprTableData, (short) (offset + OPERATION_HANDLE_OFFSET), len)) { + return KMOperationState.read(oprTableData, (short) (offset + OPERATION_HANDLE_OFFSET), oprTableData, + (short) (offset + OPERATION_HANDLE_ENTRY_SIZE), + operations[index]); } index++; } - return null; } @@ -164,13 +183,13 @@ public KMOperationState findOperation(short operationHandle) { /* opHandle is a KMInteger */ public KMOperationState reserveOperation(short opHandle) { short index = 0; - byte[] opId; + byte[] oprTableData = (byte[]) operationStateTable[OPER_TABLE_DATA_OFFSET]; + short offset = 0; while (index < MAX_OPS) { - opId = (byte[]) ((Object[]) operationStateTable[index])[0]; + offset = (short) (index * OPER_DATA_LEN); /* Check for unreserved operation state */ - if (opId[OPERATION_HANDLE_STATUS_OFFSET] == 0) { - return KMOperationState - .instance(opHandle, (Object[]) ((Object[]) operationStateTable[index])[1]); + if (oprTableData[(short) (offset + OPERATION_HANDLE_STATUS_OFFSET)] == 0) { + return KMOperationState.instance(opHandle); } index++; } @@ -179,7 +198,9 @@ public KMOperationState reserveOperation(short opHandle) { public void persistOperation(byte[] data, short opHandle, KMOperation op) { short index = 0; - byte[] opId; + byte[] oprTableData = (byte[]) operationStateTable[OPER_TABLE_DATA_OFFSET]; + Object[] operations = (Object[]) operationStateTable[OPER_TABLE_OPR_OFFSET]; + short offset = 0; short buf = KMByteBlob.instance(OPERATION_HANDLE_SIZE); getOperationHandle( opHandle, @@ -188,21 +209,17 @@ public void persistOperation(byte[] data, short opHandle, KMOperation op) { KMByteBlob.cast(buf).length()); //Update an existing operation state. while (index < MAX_OPS) { - opId = (byte[]) ((Object[]) operationStateTable[index])[0]; - if ((1 == opId[OPERATION_HANDLE_STATUS_OFFSET]) + offset = (short) (index * OPER_DATA_LEN); + if ((1 == oprTableData[(short) (offset + OPERATION_HANDLE_STATUS_OFFSET)]) && (0 == Util.arrayCompare( - opId, - OPERATION_HANDLE_OFFSET, + oprTableData, + (short) (offset + OPERATION_HANDLE_OFFSET), KMByteBlob.cast(buf).getBuffer(), KMByteBlob.cast(buf).getStartOff(), KMByteBlob.cast(buf).length()))) { - Object[] slot = (Object[]) ((Object[]) operationStateTable[index])[1]; - JCSystem.beginTransaction(); - Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, - (short) ((byte[]) slot[0]).length); - Object[] ops = ((Object[]) slot[1]); - ops[0] = op; - JCSystem.commitTransaction(); + Util.arrayCopy(data, (short) 0, oprTableData, (short) (offset + OPERATION_HANDLE_ENTRY_SIZE), + KMOperationState.MAX_DATA); + operations[index] = op; return; } index++; @@ -211,22 +228,18 @@ public void persistOperation(byte[] data, short opHandle, KMOperation op) { index = 0; //Persist a new operation. while (index < MAX_OPS) { - opId = (byte[]) ((Object[]) operationStateTable[index])[0]; - if (0 == opId[OPERATION_HANDLE_STATUS_OFFSET]) { - Object[] slot = (Object[]) ((Object[]) operationStateTable[index])[1]; - JCSystem.beginTransaction(); - opId[OPERATION_HANDLE_STATUS_OFFSET] = 1;/*reserved */ + offset = (short) (index * OPER_DATA_LEN); + if (0 == oprTableData[(short) (offset + OPERATION_HANDLE_STATUS_OFFSET)]) { + oprTableData[(short) (offset + OPERATION_HANDLE_STATUS_OFFSET)] = 1;/*reserved */ Util.arrayCopy( KMByteBlob.cast(buf).getBuffer(), KMByteBlob.cast(buf).getStartOff(), - opId, - OPERATION_HANDLE_OFFSET, + oprTableData, + (short) (offset + OPERATION_HANDLE_OFFSET), OPERATION_HANDLE_SIZE); - Util.arrayCopy(data, (short) 0, (byte[]) slot[0], (short) 0, - (short) ((byte[]) slot[0]).length); - Object[] ops = ((Object[]) slot[1]); - ops[0] = op; - JCSystem.commitTransaction(); + Util.arrayCopy(data, (short) 0, oprTableData, (short) (offset + OPERATION_HANDLE_ENTRY_SIZE), + KMOperationState.MAX_DATA); + operations[index] = op; break; } index++; @@ -235,7 +248,9 @@ public void persistOperation(byte[] data, short opHandle, KMOperation op) { public void releaseOperation(KMOperationState op) { short index = 0; - byte[] oprHandleBuf; + byte[] oprTableData = (byte[]) operationStateTable[OPER_TABLE_DATA_OFFSET]; + Object[] operations = (Object[]) operationStateTable[OPER_TABLE_OPR_OFFSET]; + short offset = 0; short buf = KMByteBlob.instance(OPERATION_HANDLE_SIZE); getOperationHandle( op.getHandle(), @@ -243,17 +258,16 @@ public void releaseOperation(KMOperationState op) { KMByteBlob.cast(buf).getStartOff(), KMByteBlob.cast(buf).length()); while (index < MAX_OPS) { - oprHandleBuf = ((byte[]) ((Object[]) operationStateTable[index])[0]); - if ((oprHandleBuf[OPERATION_HANDLE_STATUS_OFFSET] == 1) && - (0 == Util.arrayCompare(oprHandleBuf, - OPERATION_HANDLE_OFFSET, + offset = (short) (index * OPER_DATA_LEN); + if ((oprTableData[(short) (offset + OPERATION_HANDLE_STATUS_OFFSET)] == 1) && + (0 == Util.arrayCompare(oprTableData, + (short) (offset + OPERATION_HANDLE_OFFSET), KMByteBlob.cast(buf).getBuffer(), KMByteBlob.cast(buf).getStartOff(), KMByteBlob.cast(buf).length()))) { - JCSystem.beginTransaction(); - Util.arrayFillNonAtomic(oprHandleBuf, (short) 0, (short) oprHandleBuf.length, (byte) 0); - JCSystem.commitTransaction(); + Util.arrayFillNonAtomic(oprTableData, offset, OPER_DATA_LEN, (byte) 0); op.release(); + operations[index] = null; break; } index++; @@ -262,19 +276,17 @@ public void releaseOperation(KMOperationState op) { public void releaseAllOperations() { short index = 0; - byte[] oprHandleBuf; + byte[] oprTableData = (byte[]) operationStateTable[OPER_TABLE_DATA_OFFSET]; + Object[] operations = (Object[]) operationStateTable[OPER_TABLE_OPR_OFFSET]; + short offset = 0; while (index < MAX_OPS) { - oprHandleBuf = ((byte[]) ((Object[]) operationStateTable[index])[0]); - if (oprHandleBuf[OPERATION_HANDLE_STATUS_OFFSET] == 1) { - Object[] slot = (Object[]) ((Object[]) operationStateTable[index])[1]; - Object[] ops = ((Object[]) slot[1]); - ((KMOperation) ops[0]).abort(); - JCSystem.beginTransaction(); - Util.arrayFillNonAtomic((byte[]) slot[0], (short) 0, - (short) ((byte[]) slot[0]).length, (byte) 0); - Util.arrayFillNonAtomic(oprHandleBuf, (short) 0, (short) oprHandleBuf.length, (byte) 0); - ops[0] = null; - JCSystem.commitTransaction(); + offset = (short) (index * OPER_DATA_LEN); + if (oprTableData[(short) (offset + OPERATION_HANDLE_STATUS_OFFSET)] == 1) { + Util.arrayFillNonAtomic(oprTableData, offset, OPER_DATA_LEN, (byte) 0); + if (operations[index] != null) { + ((KMOperation) operations[index]).abort(); + operations[index] = null; + } } index++; } From eb1c2c6ff152bfaa9a57362ec95336fbb720b293 Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Mon, 19 Apr 2021 14:48:51 +0530 Subject: [PATCH 04/21] Card Reset change --- .../javacard/keymaster/KMKeymasterApplet.java | 36 ++++++++++++++----- .../javacard/keymaster/KMRepository.java | 24 +++++++------ 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 459a678a..b374824a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -41,7 +41,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe private static final short KM_HAL_VERSION = (short) 0x4000; private static final short MAX_AUTH_DATA_SIZE = (short) 512; private static final short DERIVE_KEY_INPUT_SIZE = (short) 256; - private static final short CANARY_BIT_FLAG = (short) 0x40; + private static final short CANARY_BIT_FLAG = (short) 0x4000; // "Keymaster HMAC Verification" - used for HMAC key verification. public static final byte[] sharingCheck = { @@ -156,7 +156,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe public static final byte OUTPUT_DATA = 25; public static final byte HW_TOKEN = 26; public static final byte VERIFICATION_TOKEN = 27; - protected static final byte SIGNATURE = 28; + public static final byte SIGNATURE = 28; // AddRngEntropy protected static final short MAX_SEED_SIZE = 2048; @@ -188,6 +188,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe protected static short[] tmpVariables; protected static short[] data; protected static byte provisionStatus = NOT_PROVISIONED; + protected static short cardResetStatus = 0x00; /** * Registers this applet. @@ -200,6 +201,8 @@ protected KMKeymasterApplet(KMSEProvider seImpl) { if (!isUpgrading) { keymasterState = KMKeymasterApplet.INIT_STATE; seProvider.createMasterKey((short) (KMRepository.MASTER_KEY_SIZE * 8)); + } else { + cardResetStatus = CANARY_BIT_FLAG; } KMType.initialize(); encoder = new KMEncoder(); @@ -301,6 +304,15 @@ protected void validateApduHeader(APDU apdu) { ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2); } } + + // Call this fucntion before processing the apdu. + private void updateCardResetFlag() { + if (repository.isResetEventOccurred()) { + JCSystem.beginTransaction(); + cardResetStatus = CANARY_BIT_FLAG; + JCSystem.commitTransaction(); + } + } /** * Processes an incoming APDU and handles it using command objects. @@ -310,6 +322,8 @@ protected void validateApduHeader(APDU apdu) { @Override public void process(APDU apdu) { try { + // Get and udpate the card reset status before processing apdu. + updateCardResetFlag(); repository.onProcess(); // Verify whether applet is in correct state. if ((keymasterState == KMKeymasterApplet.INIT_STATE) @@ -3831,16 +3845,20 @@ private static short deriveKey(byte[] scratchPad) { private static short getErrorStatusWithCanaryBitSet(short err) { short int32Ptr = KMInteger.instance((short) 4); - if (repository.isResetEventOccurred()) { - repository.resetSeStatusFlag(); - //Set the canary bit - Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), - KMInteger.cast(int32Ptr).getStartOff(), - CANARY_BIT_FLAG); - } + + Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), + KMInteger.cast(int32Ptr).getStartOff(), + cardResetStatus); + Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), (short) (KMInteger.cast(int32Ptr).getStartOff() + 2), err); + + if (cardResetStatus != 0x00) { + JCSystem.beginTransaction(); + cardResetStatus = 0x00; + JCSystem.commitTransaction(); + } return int32Ptr; } diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 9e365ada..d4a6278f 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -94,11 +94,8 @@ public class KMRepository implements KMUpgradable { // Operation table. private static final short OPER_TABLE_DATA_OFFSET = 0; private static final short OPER_TABLE_OPR_OFFSET = 1; - private static final short OPER_DATA_LEN = OPERATION_HANDLE_ENTRY_SIZE + KMOperationState.MAX_DATA + 1; + private static final short OPER_DATA_LEN = OPERATION_HANDLE_ENTRY_SIZE + KMOperationState.MAX_DATA; private static final short DATA_ARRAY_LENGTH = MAX_OPS * OPER_DATA_LEN; - // Last byte in the operation data table represents the SE Reset flag. - private static final short SE_RESET_FLAG_OFFSET = DATA_ARRAY_LENGTH - 1; - public static final byte SE_RESET_STATUS_FLAG = 0x7F; // Singleton instance @@ -113,14 +110,13 @@ public KMRepository(boolean isUpgrading) { heapIndex = JCSystem.makeTransientShortArray((short) 1, JCSystem.CLEAR_ON_RESET); reclaimIndex = JCSystem.makeTransientShortArray((short) 1, JCSystem.CLEAR_ON_RESET); heapIndex[0] = (short) 0; - reclaimIndex[0] = HEAP_SIZE; newDataTable(isUpgrading); operationStateTable = new Object[2]; operationStateTable[0] = JCSystem.makeTransientByteArray(DATA_ARRAY_LENGTH, JCSystem.CLEAR_ON_RESET); operationStateTable[1] = JCSystem.makeTransientObjectArray(MAX_OPS, JCSystem.CLEAR_ON_RESET); - //Set the reset flag - resetSeStatusFlag(); + //Set the reset flags + resetHeapEndIndex(); //Initialize the device locked status if (!isUpgrading) { @@ -130,12 +126,15 @@ public KMRepository(boolean isUpgrading) { repository = this; } - public void resetSeStatusFlag() { - ((byte[]) operationStateTable[0])[SE_RESET_FLAG_OFFSET] = SE_RESET_STATUS_FLAG; + public void resetHeapEndIndex() { + reclaimIndex[0] = HEAP_SIZE; } + // This function should only be called before processing any of the APUs. + // Once we start processing the APDU the reclainIndex[0] will change to + // a lesser value than HEAP_SIZE public boolean isResetEventOccurred() { - if (((byte[]) operationStateTable[0])[SE_RESET_FLAG_OFFSET] == SE_RESET_STATUS_FLAG) { + if (reclaimIndex[0] == HEAP_SIZE) { return false; } return true; @@ -320,12 +319,15 @@ public void onUninstall() { } public void onProcess() { + // When card reset happens reclaimIndex[0] will be equal to 0. + // So make sure the reclaimIndex[0] is always equal to HEAP_SIZE + resetHeapEndIndex(); } public void clean() { Util.arrayFillNonAtomic(heap, (short) 0, heapIndex[0], (byte) 0); heapIndex[0] = (short) 0; - reclaimIndex[0] = HEAP_SIZE; + resetHeapEndIndex(); } public void onDeselect() { From 12ae971553436d06ad2810c4e340a48ab239729c Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Mon, 19 Apr 2021 20:15:03 +0530 Subject: [PATCH 05/21] Added a new test case to test the card reset functionality --- .../javacard/test/KMFunctionalTest.java | 204 +++++++++++++++--- 1 file changed, 179 insertions(+), 25 deletions(-) diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index 9e9c2c6c..32a3a066 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -450,6 +450,10 @@ public class KMFunctionalTest { private static final int OS_PATCH_LEVEL = 1; private static final int VENDOR_PATCH_LEVEL = 1; private static final int BOOT_PATCH_LEVEL = 1; + private static final short MAJOR_TYPE_MASK = 0xE0; + private static final byte CBOR_ARRAY_MAJOR_TYPE = (byte) 0x80; + private static final byte CBOR_UINT_MAJOR_TYPE = 0x00; + private static final short CANARY_BIT_FLAG = (short) 0x4000; private CardSimulator simulator; private KMEncoder encoder; @@ -930,7 +934,7 @@ public void testDeviceLocked() { inParams = getAesDesParams(KMType.AES, KMType.ECB, KMType.PKCS7, null); short beginResp = begin(KMType.DECRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(inParams), (short) 0); + KMKeyParameters.instance(inParams), (short) 0, false); Assert.assertEquals(beginResp, KMError.DEVICE_LOCKED); short hwToken = KMHardwareAuthToken.instance(); KMHardwareAuthToken.cast(hwToken).setTimestamp(KMInteger.uint_16((byte) 2)); @@ -2383,7 +2387,7 @@ public boolean rsaVerifyMessage(byte[] input, short inputOff, short inputlen, by public byte[] EncryptMessage(byte[] input, short params, byte[] keyBlob) { short ret = begin(KMType.ENCRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(params), (short) 0); + KMKeyParameters.instance(params), (short) 0, false); // Get the operation handle. short opHandle = KMArray.cast(ret).get((short) 2); byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; @@ -2393,7 +2397,7 @@ public byte[] EncryptMessage(byte[] input, short params, byte[] keyBlob) { ret = finish(opHandle, KMByteBlob.instance(input, (short) 0, (short) input.length), null, - (short) 0, (short) 0, (short) 0, KMError.OK); + (short) 0, (short) 0, (short) 0, KMError.OK, false); short dataPtr = KMArray.cast(ret).get((short) 2); byte[] output = new byte[KMByteBlob.cast(dataPtr).length()]; if (KMByteBlob.cast(dataPtr).length() > 0) { @@ -2407,7 +2411,7 @@ public byte[] EncryptMessage(byte[] input, short params, byte[] keyBlob) { public byte[] DecryptMessage(byte[] input, short params, byte[] keyBlob) { short ret = begin(KMType.DECRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(params), (short) 0); + KMKeyParameters.instance(params), (short) 0, false); // Get the operation handle. short opHandle = KMArray.cast(ret).get((short) 2); byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; @@ -2417,7 +2421,7 @@ public byte[] DecryptMessage(byte[] input, short params, byte[] keyBlob) { ret = finish(opHandle, KMByteBlob.instance(input, (short) 0, (short) input.length), null, - (short) 0, (short) 0, (short) 0, KMError.OK); + (short) 0, (short) 0, (short) 0, KMError.OK, false); short dataPtr = KMArray.cast(ret).get((short) 2); byte[] output = new byte[KMByteBlob.cast(dataPtr).length()]; if (KMByteBlob.cast(dataPtr).length() > 0) { @@ -2447,7 +2451,7 @@ public void testUnsupportedBlockMode() { KMType.PKCS7, new byte[12]); short ret = begin(KMType.ENCRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(desPkcs7Params), (short) 0); + KMKeyParameters.instance(desPkcs7Params), (short) 0, false); Assert.assertTrue(ret == KMError.UNSUPPORTED_BLOCK_MODE); cleanUp(); } @@ -2479,7 +2483,7 @@ public void testDesEcbPkcs7PaddingCorrupted() { short ret = begin(KMType.DECRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(desPkcs7Params), (short) 0); + KMKeyParameters.instance(desPkcs7Params), (short) 0, false); // Get the operation handle. short opHandle = KMArray.cast(ret).get((short) 2); byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; @@ -2492,7 +2496,7 @@ public void testDesEcbPkcs7PaddingCorrupted() { (short) cipherText1.length); opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, - KMError.INVALID_ARGUMENT); + KMError.INVALID_ARGUMENT, false); cleanUp(); } @@ -2550,7 +2554,7 @@ public void testVtsRsaPkcs1Success() { // Do Begin operation. short ret = begin(KMType.DECRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(pkcs1Params), (short) 0); + KMKeyParameters.instance(pkcs1Params), (short) 0, false); // Get the operation handle. short opHandle = KMArray.cast(ret).get((short) 2); @@ -2563,7 +2567,7 @@ public void testVtsRsaPkcs1Success() { (short) cipherText1.length); // Finish should return UNKNOWN_ERROR. ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, - KMError.UNKNOWN_ERROR); + KMError.UNKNOWN_ERROR, false); } cleanUp(); } @@ -2763,6 +2767,106 @@ public void testUpgradeKey() { } cleanUp(); } + + /*public void testBeginResetUpdate() { + byte[] input = new byte[] {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + // Generate Key + short ret = generateHmacKey(null, null); + // Store the generated key in a new byte blob. + short keyBlobPtr = KMArray.cast(ret).get((short) 1); + byte[] keyBlob = new byte[KMByteBlob.cast(keyBlobPtr).length()]; + Util.arrayCopyNonAtomic(KMByteBlob.cast(keyBlobPtr).getBuffer(), + KMByteBlob.cast(keyBlobPtr).getStartOff(), keyBlob, + (short) 0, (short) keyBlob.length); + short inParams = getHmacParams(KMType.SHA2_256, true); + + //Call begin operation. + ret = begin(KMType.SIGN, keyBlobPtr, KMKeyParameters.instance(inParams), (short) 0); + // Get the operation handle. + short opHandle = KMArray.cast(ret).get((short) 2); + byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; + KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); + //Get the keyblobptr again. + keyBlobPtr = KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length); + + // Call update operation and check if the canary bit is set or not. + short dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); + // update with trigger reset. + ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0, true); + short canaryBitFlag = KMInteger.cast(ret).getSignificantShort(); + short err = KMInteger.cast(ret).getShort(); + Assert.assertEquals(CANARY_BIT_FLAG , canaryBitFlag); + Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE , err); + + // Call finish operation and check if the canary bit is set or not. + dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); + // finish with trigger reset. + ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, KMError.OK, true); + short canaryBitFlag = KMInteger.cast(ret).getSignificantShort(); + short err = KMInteger.cast(ret).getShort(); + Assert.assertEquals(CANARY_BIT_FLAG , canaryBitFlag); + Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE , err); + }*/ + + public void testBeginResetUpdate() { + byte[] input = new byte[] {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + + //begin, begin1, update, update1, finish, begin2, abort + boolean[][] resetEvents = { + {false, true, false, false, false, false, false}, + }; + for(int i = 0; i < resetEvents.length; i++) { + // Generate Key---------------- + short ret = generateHmacKey(null, null); + // Store the generated key in a new byte blob. + short keyBlobPtr = KMArray.cast(ret).get((short) 1); + byte[] keyBlob = new byte[KMByteBlob.cast(keyBlobPtr).length()]; + Util.arrayCopyNonAtomic(KMByteBlob.cast(keyBlobPtr).getBuffer(), + KMByteBlob.cast(keyBlobPtr).getStartOff(), keyBlob, + (short) 0, (short) keyBlob.length); + short inParams = getHmacParams(KMType.SHA2_256, true); + // Generate Key---------------- + + //Call begin operation---------------- + ret = begin(KMType.SIGN, keyBlobPtr, KMKeyParameters.instance(inParams), (short) 0, resetEvents[i][0]); + // Get the operation handle. + short opHandle = KMArray.cast(ret).get((short) 2); + byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; + KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); + //Get the keyblobptr again. + keyBlobPtr = KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length); + //Call begin operation---------------- + + //Call begin1 operation---------------- + inParams = getHmacParams(KMType.SHA2_256, true); + ret = begin(KMType.SIGN, keyBlobPtr, KMKeyParameters.instance(inParams), (short) 0, resetEvents[i][1]); + // Get the operation handle. + short opHandle1 = KMArray.cast(ret).get((short) 2); + byte[] opHandleBuf1 = new byte[KMRepository.OPERATION_HANDLE_SIZE]; + KMInteger.cast(opHandle1).getValue(opHandleBuf1, (short) 0, (short) opHandleBuf1.length); + //Get the keyblobptr again. + keyBlobPtr = KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length); + //Call begin1 operation---------------- + + //Call update operation---------------- + // Call update operation and check if the canary bit is set or not. + short dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); + // update with trigger reset. + ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0, resetEvents[i][2]); + if (resetEvents[i][1] || resetEvents[i][2]) { + short err = KMInteger.cast(ret).getShort(); + Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE, err); + } + //Call update operation---------------- + } + } + + @Test + public void testCardResetFunctionality() { + init(); + testBeginResetUpdate(); + cleanUp(); + } @Test public void testDestroyAttIds() { @@ -2843,7 +2947,7 @@ public void testAbortOperation() { byte[] plainData = "Hello World 123!".getBytes(); short ret = begin(KMType.ENCRYPT, KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length), - KMKeyParameters.instance(inParams), (short) 0); + KMKeyParameters.instance(inParams), (short) 0, false); short opHandle = KMArray.cast(ret).get((short) 2); byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); @@ -2851,7 +2955,8 @@ public void testAbortOperation() { abort(opHandle); short dataPtr = KMByteBlob.instance(plainData, (short) 0, (short) plainData.length); opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); - ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0); + ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0, false); + ret = KMInteger.cast(ret).getShort(); Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE, ret); cleanUp(); } @@ -3140,7 +3245,7 @@ public short processMessage( byte[] signature, boolean updateFlag, boolean aesGcmFlag) { - short beginResp = begin(keyPurpose, keyBlob, inParams, hwToken); + short beginResp = begin(keyPurpose, keyBlob, inParams, hwToken, false); short opHandle = KMArray.cast(beginResp).get((short) 2); byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); @@ -3168,7 +3273,7 @@ public short processMessage( inParams = KMKeyParameters.instance(inParams); } opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); - ret = update(opHandle, dataPtr, inParams, (short) 0, (short) 0); + ret = update(opHandle, dataPtr, inParams, (short) 0, (short) 0, false); dataPtr = KMArray.cast(ret).get((short) 3); if (KMByteBlob.cast(dataPtr).length() > 0) { Util.arrayCopyNonAtomic( @@ -3187,9 +3292,9 @@ public short processMessage( opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); if (keyPurpose == KMType.VERIFY) { - ret = finish(opHandle, dataPtr, signature, (short) 0, (short) 0, (short) 0, KMError.OK); + ret = finish(opHandle, dataPtr, signature, (short) 0, (short) 0, (short) 0, KMError.OK, false); } else { - ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, KMError.OK); + ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, KMError.OK, false); } if (len > 0) { dataPtr = KMArray.cast(ret).get((short) 2); @@ -3207,7 +3312,7 @@ public short processMessage( return ret; } - public short begin(byte keyPurpose, short keyBlob, short keyParmas, short hwToken) { + public short begin(byte keyPurpose, short keyBlob, short keyParmas, short hwToken, boolean triggerReset) { short arrPtr = KMArray.instance((short) 4); KMArray.cast(arrPtr).add((short) 0, KMEnum.instance(KMType.PURPOSE, keyPurpose)); KMArray.cast(arrPtr).add((short) 1, keyBlob); @@ -3217,6 +3322,11 @@ public short begin(byte keyPurpose, short keyBlob, short keyParmas, short hwToke } KMArray.cast(arrPtr).add((short) 3, hwToken); CommandAPDU apdu = encodeApdu((byte) INS_BEGIN_OPERATION_CMD, arrPtr); + if (triggerReset) { + simulator.reset(); + AID appletAID = AIDUtil.create("A000000062"); + simulator.selectApplet(appletAID); + } //print(apdu.getBytes(),(short)0,(short)apdu.getBytes().length); ResponseAPDU response = simulator.transmitCommand(apdu); short ret = KMArray.instance((short) 3); @@ -3226,19 +3336,31 @@ public short begin(byte keyPurpose, short keyBlob, short keyParmas, short hwToke KMArray.cast(ret).add((short) 2, KMInteger.exp()); byte[] respBuf = response.getBytes(); short len = (short) respBuf.length; - if (len > 5) { + byte majorType = readMajorType(respBuf); + //if (len > 5) { + if (majorType == CBOR_ARRAY_MAJOR_TYPE) { ret = decoder.decode(ret, respBuf, (short) 0, len); short error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getShort(); Assert.assertEquals(error, KMError.OK); + if (triggerReset) { + error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); + Assert.assertEquals(error, CANARY_BIT_FLAG); + } return ret; - } else { - if (len == 3) { + } else {//Major type UINT. + ret = decoder.decode(KMInteger.exp(), respBuf, (short) 0, len); + if (triggerReset) { + short error = KMInteger.cast(ret).getSignificantShort(); + Assert.assertEquals(error, CANARY_BIT_FLAG); + } + return KMInteger.cast(ret).getShort(); + /*if (len == 3) { return respBuf[0]; } if (len == 4) { return respBuf[1]; } - return Util.getShort(respBuf, (short) 0); + return Util.getShort(respBuf, (short) 0);*/ } } @@ -3270,7 +3392,7 @@ public short translateExtendedErrorCodes(short err) { } public short finish(short operationHandle, short data, byte[] signature, short inParams, - short hwToken, short verToken, short expectedErr) { + short hwToken, short verToken, short expectedErr, boolean triggerReset) { if (hwToken == 0) { hwToken = KMHardwareAuthToken.instance(); } @@ -3296,6 +3418,11 @@ public short finish(short operationHandle, short data, byte[] signature, short i KMArray.cast(arrPtr).add((short) 5, verToken); CommandAPDU apdu = encodeApdu((byte) INS_FINISH_OPERATION_CMD, arrPtr); // print(commandAPDU.getBytes()); + if (triggerReset) { + simulator.reset(); + AID appletAID = AIDUtil.create("A000000062"); + simulator.selectApplet(appletAID); + } ResponseAPDU response = simulator.transmitCommand(apdu); byte[] respBuf = response.getBytes(); short len = (short) respBuf.length; @@ -3313,16 +3440,24 @@ public short finish(short operationHandle, short data, byte[] signature, short i ret = decoder.decode(ret, respBuf, (short) 0, len); if (expectedErr == KMError.OK) { error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getShort(); + if (triggerReset) { + short canaryBit = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); + Assert.assertEquals(canaryBit, CANARY_BIT_FLAG); + } } else { error = KMInteger.cast(ret).getShort(); error = translateExtendedErrorCodes(error); + if (triggerReset) { + short canaryBit = KMInteger.cast(ret).getSignificantShort(); + Assert.assertEquals(canaryBit, CANARY_BIT_FLAG); + } } Assert.assertEquals(error, expectedErr); return ret; } public short update(short operationHandle, short data, short inParams, short hwToken, - short verToken) { + short verToken, boolean triggerReset) { if (hwToken == 0) { hwToken = KMHardwareAuthToken.instance(); } @@ -3340,6 +3475,11 @@ public short update(short operationHandle, short data, short inParams, short hwT KMArray.cast(arrPtr).add((short) 3, hwToken); KMArray.cast(arrPtr).add((short) 4, verToken); CommandAPDU apdu = encodeApdu((byte) INS_UPDATE_OPERATION_CMD, arrPtr); + if (triggerReset) { + simulator.reset(); + AID appletAID = AIDUtil.create("A000000062"); + simulator.selectApplet(appletAID); + } // print(commandAPDU.getBytes()); ResponseAPDU response = simulator.transmitCommand(apdu); short ret = KMArray.instance((short) 4); @@ -3350,15 +3490,29 @@ public short update(short operationHandle, short data, short inParams, short hwT KMArray.cast(ret).add((short) 3, KMByteBlob.exp()); byte[] respBuf = response.getBytes(); short len = (short) respBuf.length; - if (len > 5) { + byte majorType = readMajorType(respBuf); + if (majorType == CBOR_ARRAY_MAJOR_TYPE) { ret = decoder.decode(ret, respBuf, (short) 0, len); short error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getShort(); Assert.assertEquals(error, KMError.OK); + if (triggerReset) { + error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); + Assert.assertEquals(error, CANARY_BIT_FLAG); + } } else { - ret = respBuf[1]; + ret = decoder.decode(KMInteger.exp(), respBuf, (short)0, len); + if (triggerReset) { + short canaryBit = KMInteger.cast(ret).getSignificantShort(); + Assert.assertEquals(canaryBit, CANARY_BIT_FLAG); + } } return ret; } + + private byte readMajorType(byte[] resp) { + byte val = resp[0]; + return (byte) (val & MAJOR_TYPE_MASK); + } private void print(short blob) { print(KMByteBlob.cast(blob).getBuffer(), KMByteBlob.cast(blob).getStartOff(), From 88bf1e69e0c20e1f254a7fe91402f8e4c0f8194e Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Mon, 19 Apr 2021 20:38:05 +0100 Subject: [PATCH 06/21] Added a new test case to test reset events Moved KmoperationImpl to RAM --- .../keymaster/KMAndroidSEProvider.java | 33 ++++- .../javacard/keymaster/KMOperationImpl.java | 52 +++---- .../javacard/keymaster/KMJCardSimulator.java | 5 + .../javacard/test/KMFunctionalTest.java | 128 +++++++++++------- .../javacard/keymaster/KMKeymasterApplet.java | 4 +- .../javacard/keymaster/KMSEProvider.java | 6 + 6 files changed, 145 insertions(+), 83 deletions(-) diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java index 49929346..f64858f5 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMAndroidSEProvider.java @@ -416,20 +416,23 @@ private Object getInstanceFromPool(Object[] pool, byte alg) { return object; } + private void releaseInstance(Object[] pool, short index) { + if (((KMInstance) pool[index]).reserved != 0) { + JCSystem.beginTransaction(); + ((KMInstance) pool[index]).reserved = 0; + JCSystem.commitTransaction(); + } + } + private void releaseInstance(Object[] pool, Object object) { short index = 0; short len = (short) pool.length; while (index < len) { if (pool[index] != null) { if (object == ((KMInstance) pool[index]).object) { - JCSystem.beginTransaction(); - ((KMInstance) pool[index]).reserved = 0; - JCSystem.commitTransaction(); + releaseInstance(pool, index); break; } - } else { - // Reached end. - break; } index++; } @@ -1275,4 +1278,22 @@ public KMAttestationKey getAttestationKey() { public KMPreSharedKey getPresharedKey() { return (KMPreSharedKey) preSharedKey; } + + private void releasePool(Object[] pool) { + short index = 0; + short len = (short) pool.length; + while (index < len) { + if (pool[index] != null) { + releaseInstance(pool, index); + } + index++; + } + } + + @Override + public void releaseAllOperations() { + releasePool(cipherPool); + releasePool(sigPool); + releasePool(operationPool); + } } diff --git a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java index 32741eed..aa133bda 100644 --- a/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java +++ b/Applet/AndroidSEProvider/src/com/android/javacard/keymaster/KMOperationImpl.java @@ -23,8 +23,6 @@ public class KMOperationImpl implements KMOperation { - private Cipher cipher; - private Signature signature; private static final short CIPHER_ALG_OFFSET = 0x00; private static final short PADDING_OFFSET = 0x01; private static final short OPER_MODE_OFFSET = 0x02; @@ -34,9 +32,12 @@ public class KMOperationImpl implements KMOperation { //Java Card after the GCM update operation. private static final short AES_GCM_UPDATE_LEN_OFFSET = 0x05; private short[] parameters; + // Either one of Cipher/Signature instance is stored. + private Object[] operationInst; public KMOperationImpl() { parameters = JCSystem.makeTransientShortArray((short) 6, JCSystem.CLEAR_ON_RESET); + operationInst = JCSystem.makeTransientObjectArray((short) 1, JCSystem.CLEAR_ON_RESET); } public short getMode() { @@ -80,19 +81,15 @@ public void setCipherAlgorithm(short cipherAlg) { } public void setCipher(Cipher cipher) { - JCSystem.beginTransaction(); - this.cipher = cipher; - JCSystem.commitTransaction(); + operationInst[0] = cipher; } public void setSignature(Signature signer) { - JCSystem.beginTransaction(); - this.signature = signer; - JCSystem.commitTransaction(); + operationInst[0] = signer; } private void resetCipher() { - setCipher(null); + operationInst[0] = null; parameters[MAC_LENGTH_OFFSET] = 0; parameters[AES_GCM_UPDATE_LEN_OFFSET] = 0; parameters[BLOCK_MODE_OFFSET] = 0; @@ -104,7 +101,7 @@ private void resetCipher() { @Override public short update(byte[] inputDataBuf, short inputDataStart, short inputDataLength, byte[] outputDataBuf, short outputDataStart) { - short len = cipher.update(inputDataBuf, inputDataStart, inputDataLength, + short len = ((Cipher) operationInst[0]).update(inputDataBuf, inputDataStart, inputDataLength, outputDataBuf, outputDataStart); if (parameters[CIPHER_ALG_OFFSET] == KMType.AES && parameters[BLOCK_MODE_OFFSET] == KMType.GCM) { // Every time Block size data is stored as intermediate result. @@ -116,7 +113,7 @@ public short update(byte[] inputDataBuf, short inputDataStart, @Override public short update(byte[] inputDataBuf, short inputDataStart, short inputDataLength) { - signature.update(inputDataBuf, inputDataStart, inputDataLength); + ((Signature) operationInst[0]).update(inputDataBuf, inputDataStart, inputDataLength); return 0; } @@ -124,6 +121,7 @@ public short update(byte[] inputDataBuf, short inputDataStart, public short finish(byte[] inputDataBuf, short inputDataStart, short inputDataLen, byte[] outputDataBuf, short outputDataStart) { byte[] tmpArray = KMAndroidSEProvider.getInstance().tmpArray; + Cipher cipher = (Cipher) operationInst[0]; short cipherAlg = parameters[CIPHER_ALG_OFFSET]; short blockMode = parameters[BLOCK_MODE_OFFSET]; short mode = parameters[OPER_MODE_OFFSET]; @@ -209,11 +207,11 @@ public short sign(byte[] inputDataBuf, short inputDataStart, short inputDataLength, byte[] signBuf, short signStart) { short len = 0; try { - len = signature.sign(inputDataBuf, inputDataStart, inputDataLength, + len = ((Signature) operationInst[0]).sign(inputDataBuf, inputDataStart, inputDataLength, signBuf, signStart); } finally { - KMAndroidSEProvider.getInstance().releaseSignatureInstance(signature); - setSignature(null); + KMAndroidSEProvider.getInstance().releaseSignatureInstance((Signature) operationInst[0]); + operationInst[0] = null; } return len; } @@ -223,31 +221,33 @@ public boolean verify(byte[] inputDataBuf, short inputDataStart, short inputDataLength, byte[] signBuf, short signStart, short signLength) { boolean ret = false; try { - ret = signature.verify(inputDataBuf, inputDataStart, inputDataLength, + ret = ((Signature) operationInst[0]).verify(inputDataBuf, inputDataStart, inputDataLength, signBuf, signStart, signLength); } finally { - KMAndroidSEProvider.getInstance().releaseSignatureInstance(signature); - setSignature(null); + KMAndroidSEProvider.getInstance().releaseSignatureInstance((Signature) operationInst[0]); + operationInst[0] = null; } return ret; } @Override public void abort() { - if (cipher != null) { - KMAndroidSEProvider.getInstance().releaseCipherInstance(cipher); - resetCipher(); - } - if (signature != null) { - KMAndroidSEProvider.getInstance().releaseSignatureInstance(signature); - setSignature(null); + if (operationInst[0] != null) { + if (parameters[OPER_MODE_OFFSET] == KMType.ENCRYPT || + parameters[OPER_MODE_OFFSET] == KMType.DECRYPT) { + KMAndroidSEProvider.getInstance().releaseCipherInstance((Cipher) operationInst[0]); + resetCipher(); + } else { + KMAndroidSEProvider.getInstance().releaseSignatureInstance((Signature) operationInst[0]); + } + operationInst[0] = null; } KMAndroidSEProvider.getInstance().releaseOperationInstance(this); } @Override public void updateAAD(byte[] dataBuf, short dataStart, short dataLength) { - ((AEADCipher) cipher).updateAAD(dataBuf, dataStart, dataLength); + ((AEADCipher) operationInst[0]).updateAAD(dataBuf, dataStart, dataLength); } @Override @@ -258,4 +258,4 @@ public short getAESGCMOutputSize(short dataSize, short macLength) { return (short) (parameters[AES_GCM_UPDATE_LEN_OFFSET] + dataSize - macLength); } } -} +} \ No newline at end of file diff --git a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java index c6948e63..46bd03aa 100644 --- a/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java +++ b/Applet/JCardSimProvider/src/com/android/javacard/keymaster/KMJCardSimulator.java @@ -1315,4 +1315,9 @@ public KMAttestationKey getAttestationKey() { public KMPreSharedKey getPresharedKey() { return (KMPreSharedKey) preSharedKey; } + + @Override + public void releaseAllOperations() { + //Do nothing. + } } diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index 32a3a066..bcde9f5b 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -454,6 +454,8 @@ public class KMFunctionalTest { private static final byte CBOR_ARRAY_MAJOR_TYPE = (byte) 0x80; private static final byte CBOR_UINT_MAJOR_TYPE = 0x00; private static final short CANARY_BIT_FLAG = (short) 0x4000; + private static final boolean RESET = true; + private static final boolean NO_RESET = false; private CardSimulator simulator; private KMEncoder encoder; @@ -1947,14 +1949,20 @@ private short deleteKey(short keyBlob) { return respBuf[0]; } - private short abort(short opHandle) { + private short abort(short opHandle, boolean triggerReset) { short arrPtr = KMArray.instance((short) 1); KMArray.cast(arrPtr).add((short) 0, opHandle); CommandAPDU apdu = encodeApdu((byte) INS_ABORT_OPERATION_CMD, arrPtr); // print(commandAPDU.getBytes()); + if (triggerReset) { + simulator.reset(); + AID appletAID1 = AIDUtil.create("A000000062"); + simulator.selectApplet(appletAID1); + } ResponseAPDU response = simulator.transmitCommand(apdu); byte[] respBuf = response.getBytes(); - return respBuf[0]; + short ret = decoder.decode(KMInteger.exp(), respBuf, (short) 0, (short) respBuf.length); + return ret; } public short getKeyCharacteristics(short keyBlob) { @@ -2767,53 +2775,24 @@ public void testUpgradeKey() { } cleanUp(); } - - /*public void testBeginResetUpdate() { - byte[] input = new byte[] {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; - // Generate Key - short ret = generateHmacKey(null, null); - // Store the generated key in a new byte blob. - short keyBlobPtr = KMArray.cast(ret).get((short) 1); - byte[] keyBlob = new byte[KMByteBlob.cast(keyBlobPtr).length()]; - Util.arrayCopyNonAtomic(KMByteBlob.cast(keyBlobPtr).getBuffer(), - KMByteBlob.cast(keyBlobPtr).getStartOff(), keyBlob, - (short) 0, (short) keyBlob.length); - short inParams = getHmacParams(KMType.SHA2_256, true); - //Call begin operation. - ret = begin(KMType.SIGN, keyBlobPtr, KMKeyParameters.instance(inParams), (short) 0); - // Get the operation handle. - short opHandle = KMArray.cast(ret).get((short) 2); - byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; - KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); - //Get the keyblobptr again. - keyBlobPtr = KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length); - - // Call update operation and check if the canary bit is set or not. - short dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); - // update with trigger reset. - ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0, true); - short canaryBitFlag = KMInteger.cast(ret).getSignificantShort(); - short err = KMInteger.cast(ret).getShort(); - Assert.assertEquals(CANARY_BIT_FLAG , canaryBitFlag); - Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE , err); - - // Call finish operation and check if the canary bit is set or not. - dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); - // finish with trigger reset. - ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, KMError.OK, true); - short canaryBitFlag = KMInteger.cast(ret).getSignificantShort(); - short err = KMInteger.cast(ret).getShort(); - Assert.assertEquals(CANARY_BIT_FLAG , canaryBitFlag); - Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE , err); - }*/ - public void testBeginResetUpdate() { byte[] input = new byte[] {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; - - //begin, begin1, update, update1, finish, begin2, abort + // Test different combinations of reset events happening in the ordered flow of + // begin - begin1 - update - update1 - finish - finish1 - abort boolean[][] resetEvents = { - {false, true, false, false, false, false, false}, + //begin, begin1, update, update1, finish, finish1, abort + {NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET}, + {RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET}, + {NO_RESET, RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET}, + {NO_RESET, NO_RESET, RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET}, + {NO_RESET, NO_RESET, NO_RESET, RESET, NO_RESET, NO_RESET, NO_RESET}, + {NO_RESET, NO_RESET, NO_RESET, NO_RESET, RESET, NO_RESET, NO_RESET}, + {NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, RESET, NO_RESET}, + {NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET, RESET}, + {NO_RESET, NO_RESET, NO_RESET, RESET, RESET, NO_RESET, NO_RESET}, + {NO_RESET, RESET, RESET, NO_RESET, NO_RESET, NO_RESET, NO_RESET}, + {RESET, RESET, RESET, RESET, RESET, RESET, RESET}, }; for(int i = 0; i < resetEvents.length; i++) { // Generate Key---------------- @@ -2835,7 +2814,7 @@ public void testBeginResetUpdate() { KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); //Get the keyblobptr again. keyBlobPtr = KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length); - //Call begin operation---------------- + //Call begin end---------------- //Call begin1 operation---------------- inParams = getHmacParams(KMType.SHA2_256, true); @@ -2846,18 +2825,66 @@ public void testBeginResetUpdate() { KMInteger.cast(opHandle1).getValue(opHandleBuf1, (short) 0, (short) opHandleBuf1.length); //Get the keyblobptr again. keyBlobPtr = KMByteBlob.instance(keyBlob, (short) 0, (short) keyBlob.length); - //Call begin1 operation---------------- - + //Call begin1 end---------------- + //Call update operation---------------- // Call update operation and check if the canary bit is set or not. short dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); + opHandle = KMInteger.instance(opHandleBuf, (short) 0, (short) opHandleBuf.length); // update with trigger reset. ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0, resetEvents[i][2]); + // If a reset event occurred then expect INVALID_OPERATION_HANDLE. if (resetEvents[i][1] || resetEvents[i][2]) { short err = KMInteger.cast(ret).getShort(); Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE, err); } - //Call update operation---------------- + //Call update end---------------- + + //Call update1 operation---------------- + // Call update1 operation and check if the canary bit is set or not. + dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); + opHandle1 = KMInteger.instance(opHandleBuf1, (short) 0, (short) opHandleBuf1.length); + // update with trigger reset. + ret = update(opHandle1, dataPtr, (short) 0, (short) 0, (short) 0, resetEvents[i][3]); + // If a reset event occurred then expect INVALID_OPERATION_HANDLE. + if (resetEvents[i][2] || resetEvents[i][3]) { + short err = KMInteger.cast(ret).getShort(); + Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE, err); + } + //Call update end---------------- + + //Call finish operation---------------- + // Call finish operation and check if the canary bit is set or not. + dataPtr = KMByteBlob.instance((short) 0); + opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); + short expectedErr = KMError.OK; + // If a reset event occurred then expect INVALID_OPERATION_HANDLE. + if (resetEvents[i][1] | resetEvents[i][2] | resetEvents[i][3] | resetEvents[i][4]) + expectedErr = KMError.INVALID_OPERATION_HANDLE; + ret = finish(opHandle, dataPtr, null, (short) 0, (short) 0, (short) 0, expectedErr, resetEvents[i][4]); + //Call finish end---------------- + + //Call finish1 operation---------------- + // Call finish1 operation and check if the canary bit is set or not. + dataPtr = KMByteBlob.instance((short) 0); + opHandle1 = KMInteger.instance(opHandleBuf1, (short) 0, (short) opHandleBuf1.length); + expectedErr = KMError.OK; + // If a reset event occurred then expect INVALID_OPERATION_HANDLE. + if (resetEvents[i][2] | resetEvents[i][3] | resetEvents[i][4] | resetEvents[i][5]) + expectedErr = KMError.INVALID_OPERATION_HANDLE; + ret = finish(opHandle1, dataPtr, null, (short) 0, (short) 0, (short) 0, expectedErr, resetEvents[i][5]); + //Call finish end---------------- + + //Call abort operation---------------- + // Call abort operation and check if the canary bit is set or not. + opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); + ret = abort(opHandle, resetEvents[i][6]); + if (resetEvents[i][2] | resetEvents[i][3] | resetEvents[i][4] | resetEvents[i][5] | resetEvents[i][6]) { + short err = KMInteger.cast(ret).getShort(); + Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE, err); + } + //Call finish end---------------- + KMRepository.instance().clean(); } } @@ -2952,7 +2979,8 @@ public void testAbortOperation() { byte[] opHandleBuf = new byte[KMRepository.OPERATION_HANDLE_SIZE]; KMInteger.cast(opHandle).getValue(opHandleBuf, (short) 0, (short) opHandleBuf.length); opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); - abort(opHandle); + ret = abort(opHandle, false); + Assert.assertEquals(KMError.OK, KMInteger.cast(ret).getShort()); short dataPtr = KMByteBlob.instance(plainData, (short) 0, (short) plainData.length); opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); ret = update(opHandle, dataPtr, (short) 0, (short) 0, (short) 0, false); diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index b374824a..5e59f43c 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -308,6 +308,8 @@ protected void validateApduHeader(APDU apdu) { // Call this fucntion before processing the apdu. private void updateCardResetFlag() { if (repository.isResetEventOccurred()) { + // Release all the operation instances. + seProvider.releaseAllOperations(); JCSystem.beginTransaction(); cardResetStatus = CANARY_BIT_FLAG; JCSystem.commitTransaction(); @@ -322,7 +324,7 @@ private void updateCardResetFlag() { @Override public void process(APDU apdu) { try { - // Get and udpate the card reset status before processing apdu. + // Get and update the card reset status before processing apdu. updateCardResetFlag(); repository.onProcess(); // Verify whether applet is in correct state. diff --git a/Applet/src/com/android/javacard/keymaster/KMSEProvider.java b/Applet/src/com/android/javacard/keymaster/KMSEProvider.java index 65ae8fb3..167aa5b2 100644 --- a/Applet/src/com/android/javacard/keymaster/KMSEProvider.java +++ b/Applet/src/com/android/javacard/keymaster/KMSEProvider.java @@ -559,4 +559,10 @@ KMOperation initAsymmetricOperation( */ KMPreSharedKey getPresharedKey(); + /** + * Releases all the instance back to pool. + * Generally this is used when card is reset. + */ + void releaseAllOperations(); + } From 8f46ec64fbef4e90b8a3c0854fcadf9797b433a4 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Mon, 19 Apr 2021 21:04:37 +0100 Subject: [PATCH 07/21] Updated the unit test --- .../javacard/test/KMFunctionalTest.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index bcde9f5b..67ff19ac 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -678,6 +678,13 @@ private void cleanUp() { simulator.deleteApplet(appletAID); } + private void resetAndSelect() { + simulator.reset(); + AID appletAID = AIDUtil.create("A000000062"); + // Select applet + simulator.selectApplet(appletAID); + } + private CommandAPDU encodeApdu(byte ins, short cmd) { byte[] buf = new byte[2500]; @@ -1955,9 +1962,7 @@ private short abort(short opHandle, boolean triggerReset) { CommandAPDU apdu = encodeApdu((byte) INS_ABORT_OPERATION_CMD, arrPtr); // print(commandAPDU.getBytes()); if (triggerReset) { - simulator.reset(); - AID appletAID1 = AIDUtil.create("A000000062"); - simulator.selectApplet(appletAID1); + resetAndSelect(); } ResponseAPDU response = simulator.transmitCommand(apdu); byte[] respBuf = response.getBytes(); @@ -2776,7 +2781,7 @@ public void testUpgradeKey() { cleanUp(); } - public void testBeginResetUpdate() { + public void testCardRest() { byte[] input = new byte[] {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; // Test different combinations of reset events happening in the ordered flow of // begin - begin1 - update - update1 - finish - finish1 - abort @@ -2891,7 +2896,7 @@ public void testBeginResetUpdate() { @Test public void testCardResetFunctionality() { init(); - testBeginResetUpdate(); + testCardRest(); cleanUp(); } @@ -3351,9 +3356,7 @@ public short begin(byte keyPurpose, short keyBlob, short keyParmas, short hwToke KMArray.cast(arrPtr).add((short) 3, hwToken); CommandAPDU apdu = encodeApdu((byte) INS_BEGIN_OPERATION_CMD, arrPtr); if (triggerReset) { - simulator.reset(); - AID appletAID = AIDUtil.create("A000000062"); - simulator.selectApplet(appletAID); + resetAndSelect(); } //print(apdu.getBytes(),(short)0,(short)apdu.getBytes().length); ResponseAPDU response = simulator.transmitCommand(apdu); @@ -3447,9 +3450,7 @@ public short finish(short operationHandle, short data, byte[] signature, short i CommandAPDU apdu = encodeApdu((byte) INS_FINISH_OPERATION_CMD, arrPtr); // print(commandAPDU.getBytes()); if (triggerReset) { - simulator.reset(); - AID appletAID = AIDUtil.create("A000000062"); - simulator.selectApplet(appletAID); + resetAndSelect(); } ResponseAPDU response = simulator.transmitCommand(apdu); byte[] respBuf = response.getBytes(); @@ -3504,9 +3505,7 @@ public short update(short operationHandle, short data, short inParams, short hwT KMArray.cast(arrPtr).add((short) 4, verToken); CommandAPDU apdu = encodeApdu((byte) INS_UPDATE_OPERATION_CMD, arrPtr); if (triggerReset) { - simulator.reset(); - AID appletAID = AIDUtil.create("A000000062"); - simulator.selectApplet(appletAID); + resetAndSelect(); } // print(commandAPDU.getBytes()); ResponseAPDU response = simulator.transmitCommand(apdu); From 75378369915c1dc61fcab97f6aacfea67a373aa2 Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Tue, 20 Apr 2021 01:44:54 +0530 Subject: [PATCH 08/21] updated the unittest --- .../test/com/android/javacard/test/KMFunctionalTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index 67ff19ac..83a37087 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -1967,6 +1967,10 @@ private short abort(short opHandle, boolean triggerReset) { ResponseAPDU response = simulator.transmitCommand(apdu); byte[] respBuf = response.getBytes(); short ret = decoder.decode(KMInteger.exp(), respBuf, (short) 0, (short) respBuf.length); + if (triggerReset) { + short error = KMInteger.cast(ret).getSignificantShort(); + Assert.assertEquals(error, CANARY_BIT_FLAG); + } return ret; } From 6c445f028fc8c63b09fe806c176cf5e9550cf3fc Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Tue, 20 Apr 2021 23:18:24 +0530 Subject: [PATCH 09/21] Fixed the issue observed while executing vts --- .../android/javacard/keymaster/KMEncoder.java | 64 ++++++++++++++----- .../javacard/keymaster/KMKeymasterApplet.java | 8 +-- 2 files changed, 51 insertions(+), 21 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMEncoder.java b/Applet/src/com/android/javacard/keymaster/KMEncoder.java index b79349ff..53c30793 100644 --- a/Applet/src/com/android/javacard/keymaster/KMEncoder.java +++ b/Applet/src/com/android/javacard/keymaster/KMEncoder.java @@ -93,9 +93,9 @@ public short encode(short object, byte[] buffer, short startOff) { public void encodeCertChain(byte[] buffer, short offset, short length, short errInt32Ptr) { bufferRef[0] = buffer; scratchBuf[START_OFFSET] = offset; - scratchBuf[LEN_OFFSET] += (offset + 1); - //Total length is ArrayHeader + UIntHeader + length(errInt32Ptr) - scratchBuf[LEN_OFFSET] += (short) (2 + KMInteger.cast(errInt32Ptr).length()); + scratchBuf[LEN_OFFSET] = (short) (offset + 1); + //Total length is ArrayHeader + [UIntHeader + length(errInt32Ptr)] + scratchBuf[LEN_OFFSET] += (short) (1 + getEncodedIntegerLength(errInt32Ptr)); writeMajorTypeWithLength(ARRAY_TYPE, (short) 2); // Array of 2 elements encodeInteger(errInt32Ptr); @@ -110,7 +110,7 @@ public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, s scratchBuf[START_OFFSET]--; // errInt32Ptr - CanaryBit + ErrorCode - 4 bytes // Integer header - 1 byte - scratchBuf[START_OFFSET] -= (KMInteger.cast(errInt32Ptr).length() + 1); + scratchBuf[START_OFFSET] -= getEncodedIntegerLength(errInt32Ptr); //Array header - 2 elements i.e. 1 byte scratchBuf[START_OFFSET]--; // Cert Byte blob - typically 2 bytes length i.e. 3 bytes header @@ -118,10 +118,10 @@ public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, s if (certLength >= SHORT_PAYLOAD) { scratchBuf[START_OFFSET]--; } - bufferStart = scratchBuf[START_OFFSET]; if (scratchBuf[START_OFFSET] < bufferStart) { ISOException.throwIt(ISO7816.SW_WRONG_LENGTH); } + bufferStart = scratchBuf[START_OFFSET]; writeMajorTypeWithLength(ARRAY_TYPE, (short) 2); // Array of 2 elements encodeInteger(errInt32Ptr); //CanaryBit + ErrorCode writeMajorTypeWithLength(ARRAY_TYPE, (short) 1); // Array of 1 element @@ -129,20 +129,11 @@ public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, s return bufferStart; } - public short encodeError(short err, byte[] buffer, short startOff, short length) { + public short encodeError(short errInt32Ptr, byte[] buffer, short startOff, short length) { bufferRef[0] = buffer; scratchBuf[START_OFFSET] = startOff; - scratchBuf[LEN_OFFSET] = (short) (startOff + length); - // encode the err as UINT with value in err - should not be greater then 5 bytes. - if (err < UINT8_LENGTH) { - writeByte((byte) (UINT_TYPE | err)); - } else if (err < 0x100) { - writeByte((byte) (UINT_TYPE | UINT8_LENGTH)); - writeByte((byte) err); - } else { - writeByte((byte) (UINT_TYPE | UINT16_LENGTH)); - writeShort(err); - } + scratchBuf[LEN_OFFSET] = (short) (startOff + length + 1); + encodeInteger(errInt32Ptr); return (short) (scratchBuf[START_OFFSET] - startOff); } @@ -292,6 +283,45 @@ private void encodeEnum(short obj) { writeByteValue(KMEnum.cast(obj).getVal()); } + /* The total length of UINT Major type along with actual length of + * integer is returned. + */ + public short getEncodedIntegerLength(short obj) { + byte[] val = KMInteger.cast(obj).getBuffer(); + short len = KMInteger.cast(obj).length(); + short startOff = KMInteger.cast(obj).getStartOff(); + byte index = 0; + // find out the most significant byte + while (index < len) { + if (val[(short) (startOff + index)] > 0) { + break; + } else if (val[(short) (startOff + index)] < 0) { + break; + } + index++; // index will be equal to len if value is 0. + } + // find the difference between most significant byte and len + short diff = (short) (len - index); + switch (diff) { + case 0: case 1: //Byte | Short + if ((val[(short) (startOff + index)] < UINT8_LENGTH) && + (val[(short) (startOff + index)] >= 0)) { + return (short) 1; + } else { + return (short) 2; + } + case 2: //Short + return (short) 3; + case 3: case 4: //Uint32 + return (short) 5; + case 5: case 6: case 7: case 8: //Uint64 + return (short) 5; + default: + ISOException.throwIt(ISO7816.SW_DATA_INVALID); + } + return 0; + } + private void encodeInteger(short obj) { byte[] val = KMInteger.cast(obj).getBuffer(); short len = KMInteger.cast(obj).length(); diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 5e59f43c..726f1f96 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -665,7 +665,7 @@ private void processGetCertChainCmd(APDU apdu) { short int32Ptr = getErrorStatusWithCanaryBitSet(KMError.OK); //Total Extra length // Add arrayHeader and (CanaryBitSet + KMError.OK) - tmpVariables[2] = (short) (2 + KMInteger.cast(int32Ptr).length()); + tmpVariables[2] = (short) (1 + encoder.getEncodedIntegerLength(int32Ptr)); tmpVariables[0] += tmpVariables[2]; tmpVariables[1] = KMByteBlob.instance(tmpVariables[0]); bufferRef[0] = KMByteBlob.cast(tmpVariables[1]).getBuffer(); @@ -3865,10 +3865,10 @@ private static short getErrorStatusWithCanaryBitSet(short err) { } private static void sendError(APDU apdu, short err) { - bufferProp[BUF_START_OFFSET] = repository.alloc((short) 2); + bufferProp[BUF_START_OFFSET] = repository.alloc((short) 5); short int32Ptr = getErrorStatusWithCanaryBitSet(err); - bufferProp[BUF_LEN_OFFSET] = encoder.encode(int32Ptr, (byte[]) bufferRef[0], - bufferProp[BUF_START_OFFSET]); + bufferProp[BUF_LEN_OFFSET] = encoder.encodeError(int32Ptr, (byte[]) bufferRef[0], + bufferProp[BUF_START_OFFSET], (short) 5); sendOutgoing(apdu); } From 5dd4508198ae27df8c7c77273581f6b6c1d95ef9 Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Wed, 21 Apr 2021 00:08:05 +0530 Subject: [PATCH 10/21] Added comment and fixed one issue --- Applet/src/com/android/javacard/keymaster/KMEncoder.java | 2 +- .../src/com/android/javacard/keymaster/KMKeymasterApplet.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMEncoder.java b/Applet/src/com/android/javacard/keymaster/KMEncoder.java index 53c30793..0e4666f2 100644 --- a/Applet/src/com/android/javacard/keymaster/KMEncoder.java +++ b/Applet/src/com/android/javacard/keymaster/KMEncoder.java @@ -315,7 +315,7 @@ public short getEncodedIntegerLength(short obj) { case 3: case 4: //Uint32 return (short) 5; case 5: case 6: case 7: case 8: //Uint64 - return (short) 5; + return (short) 9; default: ISOException.throwIt(ISO7816.SW_DATA_INVALID); } diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 726f1f96..0fd6622a 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -202,6 +202,8 @@ protected KMKeymasterApplet(KMSEProvider seImpl) { keymasterState = KMKeymasterApplet.INIT_STATE; seProvider.createMasterKey((short) (KMRepository.MASTER_KEY_SIZE * 8)); } else { + // This flag is explicitly set after upgrade, to make sure that + // Keymaster HAL releases its operation state table. cardResetStatus = CANARY_BIT_FLAG; } KMType.initialize(); From 4884c9855749a3738f5501fd4465edfea8638923 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 21 Apr 2021 18:30:59 +0530 Subject: [PATCH 11/21] Clear the operation data associated with operation handle --- .../4.1/JavacardKeymaster4Device.cpp | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 2e3f97b3..612e408c 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -232,17 +232,21 @@ static void deleteOprHandleEntry(uint64_t halGeneratedOperationHandle) { } /* Clears all the strongbox operation handle entries from operation table */ -static void clearStrongboxOprHandleEntries() { - std::map>::iterator it = operationTable.begin(); - for(; it != operationTable.end(); ++it) { - if (isStrongboxOperation(it->first)) - operationTable.erase(it); +static void clearStrongboxOprHandleEntries(const std::unique_ptr& oprCtx) { + if (nullptr != oprCtx) { + std::map>::iterator it = operationTable.begin(); + for(; it != operationTable.end(); ++it) { + if (isStrongboxOperation(it->first)) { + oprCtx->clearOperationData(it->second.first); + operationTable.erase(it); + } + } } } template static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response, bool - hasErrorCode) { + hasErrorCode, const std::unique_ptr& oprCtx=std::unique_ptr(nullptr)) { std::unique_ptr item(nullptr); T errorCode = T::OK; std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); @@ -253,7 +257,7 @@ static std::tuple, T> decodeData(CborConverter& cb, const if (canaryBitSet) { //Clear the operation table for Strongbox operations entries. - clearStrongboxOprHandleEntries(); + clearStrongboxOprHandleEntries(oprCtx); UNSET_CANARY_BIT(temp); } // SE sends errocode as unsigned value so convert the unsigned value @@ -422,6 +426,7 @@ static bool isSEProvisioned() { return ret; } //Check if SE is provisioned. + std::unique_ptr oprCtx(nullptr); std::tie(item, errorCode) = decodeData(cborConverter, std::vector(response.begin(), response.end()-2), true); if(item != NULL) { @@ -791,7 +796,6 @@ Return JavacardKeymaster4Device::getKeyCharacteristics(const hidl_vec cborData = array.encode(); errorCode = sendData(Instruction::INS_GET_KEY_CHARACTERISTICS_CMD, cborData, cborOutData); - if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), @@ -1050,7 +1054,7 @@ Return JavacardKeymaster4Device::begin(KeyPurpose purpose, const hidl_vec< if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getKeyParameters(item, 1, outParams) || !cborConverter_.getUint64(item, 2, operationHandle)) { @@ -1139,7 +1143,7 @@ Return JavacardKeymaster4Device::update(uint64_t halGeneratedOprHandle, co if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { /*Ignore inputConsumed from javacard SE since HAL consumes all the input */ //cborConverter_.getUint64(item, 1, inputConsumed); @@ -1267,7 +1271,7 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { //There is a change that this finish callback may gets called multiple times if the input data size //is larger the MAX_ALLOWED_INPUT_SIZE (Refer OperationContext) so parse and get the outParams only @@ -1326,7 +1330,7 @@ Return JavacardKeymaster4Device::abort(uint64_t halGeneratedOprHandle if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); } } /* Delete the entry on this operationHandle */ From a53f8e7e797df4543bef84a77632e70a716df263 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 22 Apr 2021 18:14:39 +0530 Subject: [PATCH 12/21] 1. Handled the reset event in provision tool. 2. Fixed the issue in HAL while clearing the operation table. 3. isSEProvisioned call is removed. --- .../4.1/JavacardKeymaster4Device.cpp | 97 +++++-------------- ProvisioningTool/Provision.cpp | 85 +++++++++++++++- 2 files changed, 105 insertions(+), 77 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 612e408c..728dcc14 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -233,20 +233,20 @@ static void deleteOprHandleEntry(uint64_t halGeneratedOperationHandle) { /* Clears all the strongbox operation handle entries from operation table */ static void clearStrongboxOprHandleEntries(const std::unique_ptr& oprCtx) { - if (nullptr != oprCtx) { - std::map>::iterator it = operationTable.begin(); - for(; it != operationTable.end(); ++it) { - if (isStrongboxOperation(it->first)) { - oprCtx->clearOperationData(it->second.first); - operationTable.erase(it); - } + auto it = operationTable.begin(); + while (it != operationTable.end()) { + if (it->second.second == SB_KM_OPR) { //Strongbox operation + oprCtx->clearOperationData(it->second.first); + it = operationTable.erase(it); + } else { + ++it; } } } template static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response, bool - hasErrorCode, const std::unique_ptr& oprCtx=std::unique_ptr(nullptr)) { + hasErrorCode, const std::unique_ptr& oprCtx) { std::unique_ptr item(nullptr); T errorCode = T::OK; std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); @@ -404,59 +404,10 @@ uint16_t getStatus(std::vector& inputData) { return (inputData.at(inputData.size()-2) << 8) | (inputData.at(inputData.size()-1)); } -static bool isSEProvisioned() { - ErrorCode errorCode = ErrorCode::OK; - Instruction ins = Instruction::INS_GET_PROVISION_STATUS_CMD; - std::vector cborData; - std::vector response; - std::unique_ptr item; - CborConverter cborConverter; - std::vector apdu; - bool ret = false; - - errorCode = constructApduMessage(ins, cborData, apdu); - if(errorCode != ErrorCode::OK) return ret; - - if(!getTransportFactoryInstance()->sendData(apdu.data(), apdu.size(), response)) { - LOG(ERROR) << " Failed to send GET_PROVISION_STATUS_CMD "; - return ret; - } - - if((response.size() <= 2) || (getStatus(response) != APDU_RESP_STATUS_OK)) { - return ret; - } - //Check if SE is provisioned. - std::unique_ptr oprCtx(nullptr); - std::tie(item, errorCode) = decodeData(cborConverter, std::vector(response.begin(), response.end()-2), - true); - if(item != NULL) { - uint64_t status; - - if(!cborConverter.getUint64(item, 1, status)) { - LOG(ERROR) << "Failed to parse the status from cbor data"; - return ret; - } - - if ( (0 != (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_KEY)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_CHAIN)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_ATTESTATION_CERT_PARAMS)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_PRESHARED_SECRET)) && - (0 != (status & ProvisionStatus::PROVISION_STATUS_BOOT_PARAM))) { - ret = true; - } - } - return ret; -} - - ErrorCode sendData(Instruction ins, std::vector& inData, std::vector& response) { ErrorCode ret = ErrorCode::UNKNOWN_ERROR; std::vector apdu; - if (!isSEProvisioned()) { - LOG(ERROR) << "Javacard applet is not provisioned."; - return ret; - } ret = constructApduMessage(ins, inData, apdu); if(ret != ErrorCode::OK) return ret; @@ -497,7 +448,7 @@ Return JavacardKeymaster4Device::getHardwareInfo(getHardwareInfo_cb _hidl_ if (ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, ret) = decodeData(cborConverter_, std::vector(resp.begin(), resp.end()-2), - false); + false, oprCtx_); if (item != nullptr) { std::vector temp; if(!cborConverter_.getUint64(item, 0, securityLevel) || @@ -527,7 +478,7 @@ Return JavacardKeymaster4Device::getHmacSharingParameters(getHmacSharingPa if (ErrorCode::OK == errorCode) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborData.begin(), cborData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getHmacSharingParameters(item, 1, hmacSharingParameters)) { errorCode = ErrorCode::UNKNOWN_ERROR; @@ -578,7 +529,7 @@ Return JavacardKeymaster4Device::computeSharedHmac(const hidl_vec(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { std::vector bstr; if(!cborConverter_.getBinaryArray(item, 1, bstr)) { @@ -635,7 +586,7 @@ Return JavacardKeymaster4Device::addRngEntropy(const hidl_vec(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); } return errorCode; } @@ -666,7 +617,7 @@ Return JavacardKeymaster4Device::generateKey(const hidl_vec& if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, keyBlob) || !cborConverter_.getKeyCharacteristics(item, 2, keyCharacteristics)) { @@ -712,7 +663,7 @@ Return JavacardKeymaster4Device::importKey(const hidl_vec& k if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, keyBlob) || !cborConverter_.getKeyCharacteristics(item, 2, keyCharacteristics)) { @@ -767,7 +718,7 @@ Return JavacardKeymaster4Device::importWrappedKey(const hidl_vec& if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, keyBlob) || !cborConverter_.getKeyCharacteristics(item, 2, keyCharacteristics)) { @@ -799,7 +750,7 @@ Return JavacardKeymaster4Device::getKeyCharacteristics(const hidl_vec(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getKeyCharacteristics(item, 1, keyCharacteristics)) { keyCharacteristics.softwareEnforced.setToExternal(nullptr, 0); @@ -865,7 +816,7 @@ Return JavacardKeymaster4Device::attestKey(const hidl_vec& keyToA std::vector rootCert; //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getMultiBinaryArray(item, 1, temp)) { errorCode = ErrorCode::UNKNOWN_ERROR; @@ -877,7 +828,7 @@ Return JavacardKeymaster4Device::attestKey(const hidl_vec& keyToA //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { std::vector chain; if(!cborConverter_.getBinaryArray(item, 1, chain)) { @@ -915,7 +866,7 @@ Return JavacardKeymaster4Device::upgradeKey(const hidl_vec& keyBl if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); if (item != nullptr) { if(!cborConverter_.getBinaryArray(item, 1, upgradedKeyBlob)) errorCode = ErrorCode::UNKNOWN_ERROR; @@ -938,7 +889,7 @@ Return JavacardKeymaster4Device::deleteKey(const hidl_vec& k if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); } return errorCode; } @@ -954,7 +905,7 @@ Return JavacardKeymaster4Device::deleteAllKeys() { if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); } return errorCode; } @@ -970,7 +921,7 @@ Return JavacardKeymaster4Device::destroyAttestationIds() { if(errorCode == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData(cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), - true); + true, oprCtx_); } return errorCode; } @@ -1362,7 +1313,7 @@ Return<::android::hardware::keymaster::V4_1::ErrorCode> JavacardKeymaster4Device if(ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( - cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); + cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true, oprCtx_); } return errorCode; } @@ -1379,7 +1330,7 @@ Return<::android::hardware::keymaster::V4_1::ErrorCode> JavacardKeymaster4Device if(ret == ErrorCode::OK) { //Skip last 2 bytes in cborData, it contains status. std::tie(item, errorCode) = decodeData<::android::hardware::keymaster::V4_1::ErrorCode>( - cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true); + cborConverter_, std::vector(cborOutData.begin(), cborOutData.end()-2), true, oprCtx_); } return errorCode; } diff --git a/ProvisioningTool/Provision.cpp b/ProvisioningTool/Provision.cpp index 45425975..f1762430 100644 --- a/ProvisioningTool/Provision.cpp +++ b/ProvisioningTool/Provision.cpp @@ -35,6 +35,9 @@ #define APDU_P1 0x40 #define APDU_P2 0x00 #define APDU_RESP_STATUS_OK 0x9000 +#define CANARY_BIT_FLAG ( 1 << 30) +#define UNSET_CANARY_BIT(a) (a &= ~CANARY_BIT_FLAG) +#define TWOS_COMPLIMENT(a) (a = ~a + 1) namespace keymaster { namespace V4_1 { @@ -52,6 +55,23 @@ enum class Instruction { INS_GET_PROVISION_STATUS_CMD = INS_BEGIN_KM_CMD+8, }; +//Extended error codes +enum ExtendedErrors { + SW_CONDITIONS_NOT_SATISFIED = -10001, + UNSUPPORTED_CLA = -10002, + INVALID_P1P2 = -10003, + UNSUPPORTED_INSTRUCTION = -10004, + CMD_NOT_ALLOWED = -10005, + SW_WRONG_LENGTH = -10006, + INVALID_DATA = -10007, + CRYPTO_ILLEGAL_USE = -10008, + CRYPTO_ILLEGAL_VALUE = -10009, + CRYPTO_INVALID_INIT = -10010, + CRYPTO_NO_SUCH_ALGORITHM = -10011, + CRYPTO_UNINITIALIZED_KEY = -10012, + GENERIC_UNKNOWN_ERROR = -10013 +}; + enum ProvisionStatus { NOT_PROVISIONED = 0x00, PROVISION_STATUS_ATTESTATION_KEY = 0x01, @@ -67,6 +87,65 @@ enum ProvisionStatus { static ErrorCode constructApduMessage(Instruction& ins, std::vector& inputData, std::vector& apduOut); static ErrorCode sendProvisionData(std::unique_ptr& transport, Instruction ins, std::vector& inData, std::vector& response); static uint16_t getStatus(std::vector& inputData); +template +static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response); +template +static T translateExtendedErrorsToHalErrors(T& errorCode); + +template +static T translateExtendedErrorsToHalErrors(T& errorCode) { + T err; + switch(static_cast(errorCode)) { + case SW_CONDITIONS_NOT_SATISFIED: + case UNSUPPORTED_CLA: + case INVALID_P1P2: + case INVALID_DATA: + case CRYPTO_ILLEGAL_USE: + case CRYPTO_ILLEGAL_VALUE: + case CRYPTO_INVALID_INIT: + case CRYPTO_UNINITIALIZED_KEY: + case GENERIC_UNKNOWN_ERROR: + err = T::UNKNOWN_ERROR; + break; + case CRYPTO_NO_SUCH_ALGORITHM: + err = T::UNSUPPORTED_ALGORITHM; + break; + case UNSUPPORTED_INSTRUCTION: + case CMD_NOT_ALLOWED: + case SW_WRONG_LENGTH: + err = T::UNIMPLEMENTED; + break; + default: + err = static_cast(errorCode); + break; + } + return err; +} + +template +static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response) { + std::unique_ptr item(nullptr); + T errorCode = T::OK; + std::tie(item, errorCode) = cb.decodeData(response, true); + int32_t temp = static_cast(errorCode); + + bool canaryBitSet = (0 != (temp & CANARY_BIT_FLAG)); + + if (canaryBitSet) { + LOG(INFO) << "Secureelement reset happened"; + UNSET_CANARY_BIT(temp); + } + // SE sends errocode as unsigned value so convert the unsigned value + // into a signed value of same magnitude. + TWOS_COMPLIMENT(temp); + + //Write back temp to errorCode + errorCode = static_cast(temp); + + if (T::OK != errorCode) + errorCode = translateExtendedErrorsToHalErrors(errorCode); + return {std::move(item), errorCode}; +} static inline X509* parseDerCertificate(std::vector& certData) { X509 *x509 = NULL; @@ -173,8 +252,7 @@ static ErrorCode sendProvisionData(std::unique_ptr 2)) { //Skip last 2 bytes in cborData, it contains status. - std::tie(item, ret) = cborConverter.decodeData(std::vector(response.begin(), response.end()-2), - true); + std::tie(item, ret) = decodeData(cborConverter, std::vector(response.begin(), response.end()-2)); } else { ret = ErrorCode::UNKNOWN_ERROR; } @@ -366,8 +444,7 @@ ErrorCode Provision::getProvisionStatus(uint64_t& status) { return errorCode; } //Check if SE is provisioned. - std::tie(item, errorCode) = cborConverter.decodeData(std::vector(response.begin(), response.end()-2), - true); + std::tie(item, errorCode) = decodeData(cborConverter, std::vector(response.begin(), response.end()-2)); if(item != NULL) { if(!cborConverter.getUint64(item, 1, status)) { From 891bb863429f0696631f6d467a22720fd4889bb0 Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Thu, 22 Apr 2021 18:19:13 +0530 Subject: [PATCH 13/21] Corrected the testcase --- .../test/com/android/javacard/test/KMFunctionalTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index 83a37087..c77e83e0 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -2888,7 +2888,7 @@ public void testCardRest() { // Call abort operation and check if the canary bit is set or not. opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); ret = abort(opHandle, resetEvents[i][6]); - if (resetEvents[i][2] | resetEvents[i][3] | resetEvents[i][4] | resetEvents[i][5] | resetEvents[i][6]) { + if (resetEvents[i][1] || resetEvents[i][2] | resetEvents[i][3] | resetEvents[i][4] | resetEvents[i][5] | resetEvents[i][6]) { short err = KMInteger.cast(ret).getShort(); Assert.assertEquals(KMError.INVALID_OPERATION_HANDLE, err); } From fe547acf74544cb2d6711c47478a80e1a3216447 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Mon, 26 Apr 2021 15:44:44 +0100 Subject: [PATCH 14/21] Added logs for reset event --- HAL/keymaster/4.1/JavacardKeymaster4Device.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 728dcc14..e1f8bb89 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -233,9 +233,12 @@ static void deleteOprHandleEntry(uint64_t halGeneratedOperationHandle) { /* Clears all the strongbox operation handle entries from operation table */ static void clearStrongboxOprHandleEntries(const std::unique_ptr& oprCtx) { + LOG(WARNING) << "secure element reset occurred. All the below mentioned operation handles becomes invalid" + << " and the owners of these operations has to restart the operation again."; auto it = operationTable.begin(); while (it != operationTable.end()) { if (it->second.second == SB_KM_OPR) { //Strongbox operation + LOG(WARNING) << "operation handle: "(int32_t) it->first << " is invalid"; oprCtx->clearOperationData(it->second.first); it = operationTable.erase(it); } else { @@ -1038,6 +1041,9 @@ Return JavacardKeymaster4Device::update(uint64_t halGeneratedOprHandle, co uint64_t operationHandle; UpdateOperationResponse response; if (ErrorCode::OK != (errorCode = getOrigOperationHandle(halGeneratedOprHandle, operationHandle))) { + LOG(ERROR) << " Operation handle is invalid. This could happen if invalid operation handle is passed or if" + << " secure element reset occurred. In case if secure element reset occured owner" + << "has to restart this operation again."; _hidl_cb(errorCode, inputConsumed, outParams, output); return Void(); } @@ -1139,6 +1145,9 @@ Return JavacardKeymaster4Device::finish(uint64_t halGeneratedOprHandle, co FinishOperationResponse response; if (ErrorCode::OK != (errorCode = getOrigOperationHandle(halGeneratedOprHandle, operationHandle))) { + LOG(ERROR) << " Operation handle is invalid. This could happen if invalid operation handle is passed or if" + << " secure element reset occurred. In case if secure element reset occured owner" + << "has to restart this operation again."; _hidl_cb(errorCode, outParams, output); return Void(); } @@ -1258,6 +1267,8 @@ Return JavacardKeymaster4Device::abort(uint64_t halGeneratedOprHandle ErrorCode errorCode = ErrorCode::UNKNOWN_ERROR; uint64_t operationHandle; if (ErrorCode::OK != (errorCode = getOrigOperationHandle(halGeneratedOprHandle, operationHandle))) { + LOG(ERROR) << " Operation handle is invalid. This could happen if invalid operation handle is passed or if" + << " secure element reset occurred."; return errorCode; } AbortOperationRequest request; From ff365d2184a77bfe86b38fb1464e7bb564ce11f6 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Tue, 27 Apr 2021 22:35:51 +0100 Subject: [PATCH 15/21] Addressed review comments in the Applet code --- .../javacard/keymaster/KMKeymasterApplet.java | 65 ++++++++----------- .../javacard/keymaster/KMRepository.java | 35 ++++++---- 2 files changed, 47 insertions(+), 53 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 0fd6622a..aec806b5 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -41,7 +41,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe private static final short KM_HAL_VERSION = (short) 0x4000; private static final short MAX_AUTH_DATA_SIZE = (short) 512; private static final short DERIVE_KEY_INPUT_SIZE = (short) 256; - private static final short CANARY_BIT_FLAG = (short) 0x4000; + private static final short POWER_RESET_STATUS_FLAG = (short) 0x4000; // "Keymaster HMAC Verification" - used for HMAC key verification. public static final byte[] sharingCheck = { @@ -188,7 +188,6 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe protected static short[] tmpVariables; protected static short[] data; protected static byte provisionStatus = NOT_PROVISIONED; - protected static short cardResetStatus = 0x00; /** * Registers this applet. @@ -201,10 +200,6 @@ protected KMKeymasterApplet(KMSEProvider seImpl) { if (!isUpgrading) { keymasterState = KMKeymasterApplet.INIT_STATE; seProvider.createMasterKey((short) (KMRepository.MASTER_KEY_SIZE * 8)); - } else { - // This flag is explicitly set after upgrade, to make sure that - // Keymaster HAL releases its operation state table. - cardResetStatus = CANARY_BIT_FLAG; } KMType.initialize(); encoder = new KMEncoder(); @@ -306,17 +301,6 @@ protected void validateApduHeader(APDU apdu) { ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2); } } - - // Call this fucntion before processing the apdu. - private void updateCardResetFlag() { - if (repository.isResetEventOccurred()) { - // Release all the operation instances. - seProvider.releaseAllOperations(); - JCSystem.beginTransaction(); - cardResetStatus = CANARY_BIT_FLAG; - JCSystem.commitTransaction(); - } - } /** * Processes an incoming APDU and handles it using command objects. @@ -326,8 +310,11 @@ private void updateCardResetFlag() { @Override public void process(APDU apdu) { try { - // Get and update the card reset status before processing apdu. - updateCardResetFlag(); + // Handle the card reset status before processing apdu. + if (repository.isPowerResetEventOccurred()) { + // Release all the operation instances. + seProvider.releaseAllOperations(); + } repository.onProcess(); // Verify whether applet is in correct state. if ((keymasterState == KMKeymasterApplet.INIT_STATE) @@ -664,7 +651,7 @@ private void processAddRngEntropyCmd(APDU apdu) { private void processGetCertChainCmd(APDU apdu) { // Make the response tmpVariables[0] = seProvider.getCertificateChainLength(); - short int32Ptr = getErrorStatusWithCanaryBitSet(KMError.OK); + short int32Ptr = getErrorStatusWithPowerResetStatusSet(KMError.OK); //Total Extra length // Add arrayHeader and (CanaryBitSet + KMError.OK) tmpVariables[2] = (short) (1 + encoder.getEncodedIntegerLength(int32Ptr)); @@ -860,7 +847,7 @@ private void processProvisionSharedSecretCmd(APDU apdu) { private void processGetProvisionStatusCmd(APDU apdu) { tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, KMInteger.uint_16(provisionStatus)); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -935,7 +922,7 @@ private void processGetKeyCharacteristicsCmd(APDU apdu) { checkVersionAndPatchLevel(scratchPad); // make response. tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_CHARACTERISTICS]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -952,7 +939,7 @@ private void processGetHmacSharingParamCmd(APDU apdu) { KMHmacSharingParameters.cast(tmpVariables[2]).setSeed(KMByteBlob.instance((short) 0)); // prepare the response tmpVariables[3] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[3]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[3]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[3]).add((short) 1, tmpVariables[2]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1118,7 +1105,7 @@ private void processComputeSharedHmacCmd(APDU apdu) { tmpVariables[1] = KMByteBlob.instance(scratchPad, tmpVariables[6], tmpVariables[5]); // prepare the response tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, tmpVariables[1]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1196,7 +1183,7 @@ private void processUpgradeKeyCmd(APDU apdu) { } // prepare the response tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1474,7 +1461,7 @@ private void processAttestKeyCmd(APDU apdu) { cert.build(); bufferProp[BUF_START_OFFSET] = encoder.encodeCert((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], cert.getCertStart(), cert.getCertLength(), - getErrorStatusWithCanaryBitSet(KMError.OK)); + getErrorStatusWithPowerResetStatusSet(KMError.OK)); bufferProp[BUF_LEN_OFFSET] = (short) (cert.getCertLength() + (cert.getCertStart() - bufferProp[BUF_START_OFFSET])); sendOutgoing(apdu); } @@ -1639,7 +1626,7 @@ private void processFinishOperationCmd(APDU apdu) { if (data[OUTPUT_DATA] == KMType.INVALID_VALUE) { data[OUTPUT_DATA] = KMByteBlob.instance((short) 0); } - KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[2]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 2, data[OUTPUT_DATA]); @@ -2121,7 +2108,7 @@ private void processUpdateOperationCmd(APDU apdu) { if (data[OUTPUT_DATA] == KMType.INVALID_VALUE) { data[OUTPUT_DATA] = KMByteBlob.instance((short) 0); } - KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[2]).add((short) 1, KMInteger.uint_16(tmpVariables[3])); KMArray.cast(tmpVariables[2]).add((short) 2, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 3, data[OUTPUT_DATA]); @@ -2227,7 +2214,7 @@ private void processBeginOperationCmd(APDU apdu) { } tmpVariables[1] = KMKeyParameters.instance(tmpVariables[2]); tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[0]).add((short) 2, data[OP_HANDLE]); @@ -2837,7 +2824,7 @@ private void importKey(APDU apdu, byte[] scratchPad) { // prepare the response tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); KMArray.cast(tmpVariables[0]).add((short) 2, data[KEY_CHARACTERISTICS]); @@ -3339,7 +3326,7 @@ private static void processGenerateKey(APDU apdu) { // prepare the response tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithCanaryBitSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); KMArray.cast(tmpVariables[0]).add((short) 2, data[KEY_CHARACTERISTICS]); @@ -3847,28 +3834,28 @@ private static short deriveKey(byte[] scratchPad) { return tmpVariables[3]; } - private static short getErrorStatusWithCanaryBitSet(short err) { + private static short getErrorStatusWithPowerResetStatusSet(short err) { short int32Ptr = KMInteger.instance((short) 4); + short powerResetStatus = 0; + if (repository.isPowerResetEventOccurred()) { + powerResetStatus = POWER_RESET_STATUS_FLAG; + } Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), KMInteger.cast(int32Ptr).getStartOff(), - cardResetStatus); + powerResetStatus); Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), (short) (KMInteger.cast(int32Ptr).getStartOff() + 2), err); - if (cardResetStatus != 0x00) { - JCSystem.beginTransaction(); - cardResetStatus = 0x00; - JCSystem.commitTransaction(); - } + repository.restorePowerResetStatus(); return int32Ptr; } private static void sendError(APDU apdu, short err) { bufferProp[BUF_START_OFFSET] = repository.alloc((short) 5); - short int32Ptr = getErrorStatusWithCanaryBitSet(err); + short int32Ptr = getErrorStatusWithPowerResetStatusSet(err); bufferProp[BUF_LEN_OFFSET] = encoder.encodeError(int32Ptr, (byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], (short) 5); sendOutgoing(apdu); diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index d4a6278f..2409d9c1 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -42,6 +42,7 @@ public class KMRepository implements KMUpgradable { private static final short OPERATION_HANDLE_OFFSET = 1; private static final short OPERATION_HANDLE_ENTRY_SIZE = OPERATION_HANDLE_SIZE + OPERATION_HANDLE_STATUS_SIZE; + private static final byte POWER_RESET_STATUS_FLAG = (byte) 0xEF; // Data table offsets public static final byte COMPUTED_HMAC_KEY = 8; @@ -90,6 +91,8 @@ public class KMRepository implements KMUpgradable { private byte[] dataTable; private short dataIndex; private short[] reclaimIndex; + // This variable is used to check if power reset event occurred. + private byte[] powerResetStatus; // Operation table. private static final short OPER_TABLE_DATA_OFFSET = 0; @@ -109,37 +112,44 @@ public KMRepository(boolean isUpgrading) { heap = JCSystem.makeTransientByteArray(HEAP_SIZE, JCSystem.CLEAR_ON_RESET); heapIndex = JCSystem.makeTransientShortArray((short) 1, JCSystem.CLEAR_ON_RESET); reclaimIndex = JCSystem.makeTransientShortArray((short) 1, JCSystem.CLEAR_ON_RESET); + powerResetStatus = JCSystem.makeTransientByteArray((short) 1, JCSystem.CLEAR_ON_RESET); heapIndex[0] = (short) 0; + reclaimIndex[0] = HEAP_SIZE; + powerResetStatus[0] = POWER_RESET_STATUS_FLAG; newDataTable(isUpgrading); operationStateTable = new Object[2]; operationStateTable[0] = JCSystem.makeTransientByteArray(DATA_ARRAY_LENGTH, JCSystem.CLEAR_ON_RESET); operationStateTable[1] = JCSystem.makeTransientObjectArray(MAX_OPS, JCSystem.CLEAR_ON_RESET); - //Set the reset flags - resetHeapEndIndex(); //Initialize the device locked status if (!isUpgrading) { setDeviceLock(false); setDeviceLockPasswordOnly(false); + } else { + // In case of upgrade, the applet is deleted and installed again so all + // volatile memory is erased. so it is necessary to force the power reset flag + // to 0 so that the HAL can clear its operation state. + powerResetStatus[0] = (byte) 0; } repository = this; } - public void resetHeapEndIndex() { - reclaimIndex[0] = HEAP_SIZE; - } - // This function should only be called before processing any of the APUs. - // Once we start processing the APDU the reclainIndex[0] will change to - // a lesser value than HEAP_SIZE - public boolean isResetEventOccurred() { - if (reclaimIndex[0] == HEAP_SIZE) { + // Transient memory is cleared in two cases: + // 1. Card reset event + // 2. Applet upgrade. + public boolean isPowerResetEventOccurred() { + if (powerResetStatus[0] == POWER_RESET_STATUS_FLAG) { return false; } return true; } + public void restorePowerResetStatus() { + powerResetStatus[0] = POWER_RESET_STATUS_FLAG; + } + public void getOperationHandle(short oprHandle, byte[] buf, short off, short len) { if (KMInteger.cast(oprHandle).length() != OPERATION_HANDLE_SIZE) { KMException.throwIt(KMError.INVALID_OPERATION_HANDLE); @@ -319,15 +329,12 @@ public void onUninstall() { } public void onProcess() { - // When card reset happens reclaimIndex[0] will be equal to 0. - // So make sure the reclaimIndex[0] is always equal to HEAP_SIZE - resetHeapEndIndex(); } public void clean() { Util.arrayFillNonAtomic(heap, (short) 0, heapIndex[0], (byte) 0); heapIndex[0] = (short) 0; - resetHeapEndIndex(); + reclaimIndex[0] = HEAP_SIZE; } public void onDeselect() { From f57570b65ed5a9fd9077516820c0d40d10d85ce8 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 28 Apr 2021 10:04:14 +0100 Subject: [PATCH 16/21] renamed the function names appropriately --- .../4.1/JavacardKeymaster4Device.cpp | 52 +++++++++++++------ 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index e1f8bb89..2cabff46 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -51,9 +51,7 @@ #define INS_END_KM_CMD 0x7F #define SW_KM_OPR 0UL #define SB_KM_OPR 1UL -#define CANARY_BIT_FLAG ( 1 << 30) -#define UNSET_CANARY_BIT(a) (a &= ~CANARY_BIT_FLAG) -#define TWOS_COMPLIMENT(a) (a = ~a + 1) +#define SE_POWER_RESET_STATUS_FLAG ( 1 << 30) namespace keymaster { namespace V4_1 { @@ -247,28 +245,50 @@ static void clearStrongboxOprHandleEntries(const std::unique_ptr(~value+1); +} + +/** + * This function separates the original error code from the + * power reset flag and returns the original error code. + */ +static inline uint32_t extractActualErrorCode(uint32_t err) { + // Unset the power rest flag. + return (err & ~SE_POWER_RESET_STATUS_FLAG); +} + +/** + * Clears all the strongbox operation handle entries if secure element power reset happens. + * And also extracts the original error code value. + */ +static uint32_t handleSePowerResetAndExtractOriginalErrorCode(const std::unique_ptr& oprCtx, uint32_t errorCode) { + //Check if secure element is reset + bool isSeResetOccurred = (0 != (errorCode & SE_POWER_RESET_STATUS_FLAG)); + + if (isSeResetOccurred) { + //Clear the operation table for Strongbox operations entries. + clearStrongboxOprHandleEntries(oprCtx); + return extractActualErrorCode(errorCode); + } + return errorCode; +} + template static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response, bool hasErrorCode, const std::unique_ptr& oprCtx) { std::unique_ptr item(nullptr); T errorCode = T::OK; std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); - int32_t temp = static_cast(errorCode); - //Check if secure element is reset - bool canaryBitSet = (0 != (temp & CANARY_BIT_FLAG)); + uint32_t tempErrCode = handleSePowerResetAndExtractOriginalErrorCode(oprCtx, static_cast(errorCode)); - if (canaryBitSet) { - //Clear the operation table for Strongbox operations entries. - clearStrongboxOprHandleEntries(oprCtx); - UNSET_CANARY_BIT(temp); - } // SE sends errocode as unsigned value so convert the unsigned value - // into a signed value of same magnitude. - TWOS_COMPLIMENT(temp); - - //Write back temp to errorCode - errorCode = static_cast(temp); + // into a signed value of same magnitude and copy back to errorCode. + errorCode = static_cast(get2sCompliment(tempErrorCode)); if (T::OK != errorCode) errorCode = translateExtendedErrorsToHalErrors(errorCode); From 7867add099ebdc10a7088c362acdd18e23cfef84 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 28 Apr 2021 16:37:52 +0530 Subject: [PATCH 17/21] Updated the comment --- HAL/keymaster/4.1/JavacardKeymaster4Device.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 2cabff46..090ea6ba 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -231,12 +231,12 @@ static void deleteOprHandleEntry(uint64_t halGeneratedOperationHandle) { /* Clears all the strongbox operation handle entries from operation table */ static void clearStrongboxOprHandleEntries(const std::unique_ptr& oprCtx) { - LOG(WARNING) << "secure element reset occurred. All the below mentioned operation handles becomes invalid" - << " and the owners of these operations has to restart the operation again."; + LOG(WARNING) << "secure element reset occurred. All the below operation handles for private key operations" + << " becomes invalid and the owners of these operations has to restart the operation again."; auto it = operationTable.begin(); while (it != operationTable.end()) { if (it->second.second == SB_KM_OPR) { //Strongbox operation - LOG(WARNING) << "operation handle: "(int32_t) it->first << " is invalid"; + LOG(WARNING) << "operation handle: " << it->first << " is invalid"; oprCtx->clearOperationData(it->second.first); it = operationTable.erase(it); } else { @@ -288,7 +288,7 @@ static std::tuple, T> decodeData(CborConverter& cb, const // SE sends errocode as unsigned value so convert the unsigned value // into a signed value of same magnitude and copy back to errorCode. - errorCode = static_cast(get2sCompliment(tempErrorCode)); + errorCode = static_cast(get2sCompliment(tempErrCode)); if (T::OK != errorCode) errorCode = translateExtendedErrorsToHalErrors(errorCode); From febc0400794f3e4a6d876bf235cc56fa1f869513 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 28 Apr 2021 16:53:49 +0100 Subject: [PATCH 18/21] Renamed canary bit name to power reset status --- .../javacard/test/KMFunctionalTest.java | 32 +++++++-------- .../android/javacard/keymaster/KMEncoder.java | 4 +- .../javacard/keymaster/KMKeymasterApplet.java | 2 +- ProvisioningTool/Provision.cpp | 39 ++++++++++++------- 4 files changed, 45 insertions(+), 32 deletions(-) diff --git a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java index c77e83e0..41bc41a6 100644 --- a/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java +++ b/Applet/JCardSimProvider/test/com/android/javacard/test/KMFunctionalTest.java @@ -453,7 +453,7 @@ public class KMFunctionalTest { private static final short MAJOR_TYPE_MASK = 0xE0; private static final byte CBOR_ARRAY_MAJOR_TYPE = (byte) 0x80; private static final byte CBOR_UINT_MAJOR_TYPE = 0x00; - private static final short CANARY_BIT_FLAG = (short) 0x4000; + private static final short SE_POWER_RESET_FLAG = (short) 0x4000; private static final boolean RESET = true; private static final boolean NO_RESET = false; @@ -1969,7 +1969,7 @@ private short abort(short opHandle, boolean triggerReset) { short ret = decoder.decode(KMInteger.exp(), respBuf, (short) 0, (short) respBuf.length); if (triggerReset) { short error = KMInteger.cast(ret).getSignificantShort(); - Assert.assertEquals(error, CANARY_BIT_FLAG); + Assert.assertEquals(error, SE_POWER_RESET_FLAG); } return ret; } @@ -2837,7 +2837,7 @@ public void testCardRest() { //Call begin1 end---------------- //Call update operation---------------- - // Call update operation and check if the canary bit is set or not. + // Call update operation and check if the secure element power reset flag is set or not. short dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); opHandle = KMInteger.instance(opHandleBuf, (short) 0, (short) opHandleBuf.length); // update with trigger reset. @@ -2850,7 +2850,7 @@ public void testCardRest() { //Call update end---------------- //Call update1 operation---------------- - // Call update1 operation and check if the canary bit is set or not. + // Call update1 operation and check if the secure element power reset flag is set or not. dataPtr = KMByteBlob.instance(input, (short) 0, (short) input.length); opHandle1 = KMInteger.instance(opHandleBuf1, (short) 0, (short) opHandleBuf1.length); // update with trigger reset. @@ -2863,7 +2863,7 @@ public void testCardRest() { //Call update end---------------- //Call finish operation---------------- - // Call finish operation and check if the canary bit is set or not. + // Call finish operation and check if the secure element power reset flag is set or not. dataPtr = KMByteBlob.instance((short) 0); opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); short expectedErr = KMError.OK; @@ -2874,7 +2874,7 @@ public void testCardRest() { //Call finish end---------------- //Call finish1 operation---------------- - // Call finish1 operation and check if the canary bit is set or not. + // Call finish1 operation and check if the secure element power reset flag is set or not. dataPtr = KMByteBlob.instance((short) 0); opHandle1 = KMInteger.instance(opHandleBuf1, (short) 0, (short) opHandleBuf1.length); expectedErr = KMError.OK; @@ -2885,7 +2885,7 @@ public void testCardRest() { //Call finish end---------------- //Call abort operation---------------- - // Call abort operation and check if the canary bit is set or not. + // Call abort operation and check if the secure element power reset flag is set or not. opHandle = KMInteger.uint_64(opHandleBuf, (short) 0); ret = abort(opHandle, resetEvents[i][6]); if (resetEvents[i][1] || resetEvents[i][2] | resetEvents[i][3] | resetEvents[i][4] | resetEvents[i][5] | resetEvents[i][6]) { @@ -3379,14 +3379,14 @@ public short begin(byte keyPurpose, short keyBlob, short keyParmas, short hwToke Assert.assertEquals(error, KMError.OK); if (triggerReset) { error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); - Assert.assertEquals(error, CANARY_BIT_FLAG); + Assert.assertEquals(error, SE_POWER_RESET_FLAG); } return ret; } else {//Major type UINT. ret = decoder.decode(KMInteger.exp(), respBuf, (short) 0, len); if (triggerReset) { short error = KMInteger.cast(ret).getSignificantShort(); - Assert.assertEquals(error, CANARY_BIT_FLAG); + Assert.assertEquals(error, SE_POWER_RESET_FLAG); } return KMInteger.cast(ret).getShort(); /*if (len == 3) { @@ -3474,15 +3474,15 @@ public short finish(short operationHandle, short data, byte[] signature, short i if (expectedErr == KMError.OK) { error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getShort(); if (triggerReset) { - short canaryBit = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); - Assert.assertEquals(canaryBit, CANARY_BIT_FLAG); + short powerResetStatus = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); + Assert.assertEquals(powerResetStatus, SE_POWER_RESET_FLAG); } } else { error = KMInteger.cast(ret).getShort(); error = translateExtendedErrorCodes(error); if (triggerReset) { - short canaryBit = KMInteger.cast(ret).getSignificantShort(); - Assert.assertEquals(canaryBit, CANARY_BIT_FLAG); + short powerResetStatus = KMInteger.cast(ret).getSignificantShort(); + Assert.assertEquals(powerResetStatus, SE_POWER_RESET_FLAG); } } Assert.assertEquals(error, expectedErr); @@ -3528,13 +3528,13 @@ public short update(short operationHandle, short data, short inParams, short hwT Assert.assertEquals(error, KMError.OK); if (triggerReset) { error = KMInteger.cast(KMArray.cast(ret).get((short) 0)).getSignificantShort(); - Assert.assertEquals(error, CANARY_BIT_FLAG); + Assert.assertEquals(error, SE_POWER_RESET_FLAG); } } else { ret = decoder.decode(KMInteger.exp(), respBuf, (short)0, len); if (triggerReset) { - short canaryBit = KMInteger.cast(ret).getSignificantShort(); - Assert.assertEquals(canaryBit, CANARY_BIT_FLAG); + short powerResetStatus = KMInteger.cast(ret).getSignificantShort(); + Assert.assertEquals(powerResetStatus, SE_POWER_RESET_FLAG); } } return ret; diff --git a/Applet/src/com/android/javacard/keymaster/KMEncoder.java b/Applet/src/com/android/javacard/keymaster/KMEncoder.java index 0e4666f2..14d8ef4c 100644 --- a/Applet/src/com/android/javacard/keymaster/KMEncoder.java +++ b/Applet/src/com/android/javacard/keymaster/KMEncoder.java @@ -108,7 +108,7 @@ public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, s scratchBuf[LEN_OFFSET] = (short) (certStart + 1); //Array header - 2 elements i.e. 1 byte scratchBuf[START_OFFSET]--; - // errInt32Ptr - CanaryBit + ErrorCode - 4 bytes + // errInt32Ptr - PowerResetStatus + ErrorCode - 4 bytes // Integer header - 1 byte scratchBuf[START_OFFSET] -= getEncodedIntegerLength(errInt32Ptr); //Array header - 2 elements i.e. 1 byte @@ -123,7 +123,7 @@ public short encodeCert(byte[] certBuffer, short bufferStart, short certStart, s } bufferStart = scratchBuf[START_OFFSET]; writeMajorTypeWithLength(ARRAY_TYPE, (short) 2); // Array of 2 elements - encodeInteger(errInt32Ptr); //CanaryBit + ErrorCode + encodeInteger(errInt32Ptr); //PowerResetStatus + ErrorCode writeMajorTypeWithLength(ARRAY_TYPE, (short) 1); // Array of 1 element writeMajorTypeWithLength(BYTES_TYPE, certLength); // Cert Byte Blob of length return bufferStart; diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index aec806b5..5c452e56 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -653,7 +653,7 @@ private void processGetCertChainCmd(APDU apdu) { tmpVariables[0] = seProvider.getCertificateChainLength(); short int32Ptr = getErrorStatusWithPowerResetStatusSet(KMError.OK); //Total Extra length - // Add arrayHeader and (CanaryBitSet + KMError.OK) + // Add arrayHeader and (PowerResetStatus + KMError.OK) tmpVariables[2] = (short) (1 + encoder.getEncodedIntegerLength(int32Ptr)); tmpVariables[0] += tmpVariables[2]; tmpVariables[1] = KMByteBlob.instance(tmpVariables[0]); diff --git a/ProvisioningTool/Provision.cpp b/ProvisioningTool/Provision.cpp index f1762430..950b9203 100644 --- a/ProvisioningTool/Provision.cpp +++ b/ProvisioningTool/Provision.cpp @@ -35,9 +35,7 @@ #define APDU_P1 0x40 #define APDU_P2 0x00 #define APDU_RESP_STATUS_OK 0x9000 -#define CANARY_BIT_FLAG ( 1 << 30) -#define UNSET_CANARY_BIT(a) (a &= ~CANARY_BIT_FLAG) -#define TWOS_COMPLIMENT(a) (a = ~a + 1) +#define SE_POWER_RESET_STATUS_FLAG ( 1 << 30) namespace keymaster { namespace V4_1 { @@ -122,6 +120,28 @@ static T translateExtendedErrorsToHalErrors(T& errorCode) { return err; } +/** + * Returns the negative value of the same number. + */ +static inline int32_t get2sCompliment(uint32_t value) { + return static_cast(~value+1); +} + +/** + * This function separates the original error code from the + * power reset flag and returns the original error code. + */ +static uint32_t extractOriginalErrorCode(uint32_t errorCode) { + //Check if secure element is reset + bool isSeResetOccurred = (0 != (errorCode & SE_POWER_RESET_STATUS_FLAG)); + + if (isSeResetOccurred) { + LOG(ERROR) << "Secure element reset happened"; + return (errorCode & ~SE_POWER_RESET_STATUS_FLAG); + } + return errorCode; +} + template static std::tuple, T> decodeData(CborConverter& cb, const std::vector& response) { std::unique_ptr item(nullptr); @@ -129,18 +149,11 @@ static std::tuple, T> decodeData(CborConverter& cb, const std::tie(item, errorCode) = cb.decodeData(response, true); int32_t temp = static_cast(errorCode); - bool canaryBitSet = (0 != (temp & CANARY_BIT_FLAG)); + uint32_t tempErrCode = extractOriginalErrorCode(static_cast(errorCode)); - if (canaryBitSet) { - LOG(INFO) << "Secureelement reset happened"; - UNSET_CANARY_BIT(temp); - } // SE sends errocode as unsigned value so convert the unsigned value - // into a signed value of same magnitude. - TWOS_COMPLIMENT(temp); - - //Write back temp to errorCode - errorCode = static_cast(temp); + // into a signed value of same magnitude and copy back to errorCode. + errorCode = static_cast(get2sCompliment(tempErrCode)); if (T::OK != errorCode) errorCode = translateExtendedErrorsToHalErrors(errorCode); From c9656fdf2de23662fb6924ea403480f6f368f54d Mon Sep 17 00:00:00 2001 From: bvenkatswarlu Date: Thu, 29 Apr 2021 01:54:01 +0530 Subject: [PATCH 19/21] renamed function and variables names appropriately --- .../javacard/keymaster/KMKeymasterApplet.java | 37 +++++++++++-------- .../javacard/keymaster/KMRepository.java | 11 +++++- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java index 5c452e56..cb739745 100644 --- a/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java +++ b/Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java @@ -41,7 +41,7 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe private static final short KM_HAL_VERSION = (short) 0x4000; private static final short MAX_AUTH_DATA_SIZE = (short) 512; private static final short DERIVE_KEY_INPUT_SIZE = (short) 256; - private static final short POWER_RESET_STATUS_FLAG = (short) 0x4000; + private static final short POWER_RESET_MASK_FLAG = (short) 0x4000; // "Keymaster HMAC Verification" - used for HMAC key verification. public static final byte[] sharingCheck = { @@ -651,7 +651,7 @@ private void processAddRngEntropyCmd(APDU apdu) { private void processGetCertChainCmd(APDU apdu) { // Make the response tmpVariables[0] = seProvider.getCertificateChainLength(); - short int32Ptr = getErrorStatusWithPowerResetStatusSet(KMError.OK); + short int32Ptr = buildErrorStatus(KMError.OK); //Total Extra length // Add arrayHeader and (PowerResetStatus + KMError.OK) tmpVariables[2] = (short) (1 + encoder.getEncodedIntegerLength(int32Ptr)); @@ -847,7 +847,7 @@ private void processProvisionSharedSecretCmd(APDU apdu) { private void processGetProvisionStatusCmd(APDU apdu) { tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, KMInteger.uint_16(provisionStatus)); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -922,7 +922,7 @@ private void processGetKeyCharacteristicsCmd(APDU apdu) { checkVersionAndPatchLevel(scratchPad); // make response. tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_CHARACTERISTICS]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -939,7 +939,7 @@ private void processGetHmacSharingParamCmd(APDU apdu) { KMHmacSharingParameters.cast(tmpVariables[2]).setSeed(KMByteBlob.instance((short) 0)); // prepare the response tmpVariables[3] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[3]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[3]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[3]).add((short) 1, tmpVariables[2]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1105,7 +1105,7 @@ private void processComputeSharedHmacCmd(APDU apdu) { tmpVariables[1] = KMByteBlob.instance(scratchPad, tmpVariables[6], tmpVariables[5]); // prepare the response tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, tmpVariables[1]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1183,7 +1183,7 @@ private void processUpgradeKeyCmd(APDU apdu) { } // prepare the response tmpVariables[0] = KMArray.instance((short) 2); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); bufferProp[BUF_START_OFFSET] = repository.allocAvailableMemory(); @@ -1461,7 +1461,7 @@ private void processAttestKeyCmd(APDU apdu) { cert.build(); bufferProp[BUF_START_OFFSET] = encoder.encodeCert((byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], cert.getCertStart(), cert.getCertLength(), - getErrorStatusWithPowerResetStatusSet(KMError.OK)); + buildErrorStatus(KMError.OK)); bufferProp[BUF_LEN_OFFSET] = (short) (cert.getCertLength() + (cert.getCertStart() - bufferProp[BUF_START_OFFSET])); sendOutgoing(apdu); } @@ -1626,7 +1626,7 @@ private void processFinishOperationCmd(APDU apdu) { if (data[OUTPUT_DATA] == KMType.INVALID_VALUE) { data[OUTPUT_DATA] = KMByteBlob.instance((short) 0); } - KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[2]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[2]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 2, data[OUTPUT_DATA]); @@ -2108,7 +2108,7 @@ private void processUpdateOperationCmd(APDU apdu) { if (data[OUTPUT_DATA] == KMType.INVALID_VALUE) { data[OUTPUT_DATA] = KMByteBlob.instance((short) 0); } - KMArray.cast(tmpVariables[2]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[2]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[2]).add((short) 1, KMInteger.uint_16(tmpVariables[3])); KMArray.cast(tmpVariables[2]).add((short) 2, tmpVariables[1]); KMArray.cast(tmpVariables[2]).add((short) 3, data[OUTPUT_DATA]); @@ -2214,7 +2214,7 @@ private void processBeginOperationCmd(APDU apdu) { } tmpVariables[1] = KMKeyParameters.instance(tmpVariables[2]); tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, tmpVariables[1]); KMArray.cast(tmpVariables[0]).add((short) 2, data[OP_HANDLE]); @@ -2824,7 +2824,7 @@ private void importKey(APDU apdu, byte[] scratchPad) { // prepare the response tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); KMArray.cast(tmpVariables[0]).add((short) 2, data[KEY_CHARACTERISTICS]); @@ -3326,7 +3326,7 @@ private static void processGenerateKey(APDU apdu) { // prepare the response tmpVariables[0] = KMArray.instance((short) 3); - KMArray.cast(tmpVariables[0]).add((short) 0, getErrorStatusWithPowerResetStatusSet(KMError.OK)); + KMArray.cast(tmpVariables[0]).add((short) 0, buildErrorStatus(KMError.OK)); KMArray.cast(tmpVariables[0]).add((short) 1, data[KEY_BLOB]); KMArray.cast(tmpVariables[0]).add((short) 2, data[KEY_CHARACTERISTICS]); @@ -3834,11 +3834,15 @@ private static short deriveKey(byte[] scratchPad) { return tmpVariables[3]; } - private static short getErrorStatusWithPowerResetStatusSet(short err) { + // This function masks the error code with POWER_RESET_MASK_FLAG + // in case if card reset event occurred. The clients of the Applet + // has to extract the power reset status from the error code and + // process accordingly. + private static short buildErrorStatus(short err) { short int32Ptr = KMInteger.instance((short) 4); short powerResetStatus = 0; if (repository.isPowerResetEventOccurred()) { - powerResetStatus = POWER_RESET_STATUS_FLAG; + powerResetStatus = POWER_RESET_MASK_FLAG; } Util.setShort(KMInteger.cast(int32Ptr).getBuffer(), @@ -3849,13 +3853,14 @@ private static short getErrorStatusWithPowerResetStatusSet(short err) { (short) (KMInteger.cast(int32Ptr).getStartOff() + 2), err); + // reset power reset status flag to its default value. repository.restorePowerResetStatus(); return int32Ptr; } private static void sendError(APDU apdu, short err) { bufferProp[BUF_START_OFFSET] = repository.alloc((short) 5); - short int32Ptr = getErrorStatusWithPowerResetStatusSet(err); + short int32Ptr = buildErrorStatus(err); bufferProp[BUF_LEN_OFFSET] = encoder.encodeError(int32Ptr, (byte[]) bufferRef[0], bufferProp[BUF_START_OFFSET], (short) 5); sendOutgoing(apdu); diff --git a/Applet/src/com/android/javacard/keymaster/KMRepository.java b/Applet/src/com/android/javacard/keymaster/KMRepository.java index 2409d9c1..39127850 100644 --- a/Applet/src/com/android/javacard/keymaster/KMRepository.java +++ b/Applet/src/com/android/javacard/keymaster/KMRepository.java @@ -91,7 +91,9 @@ public class KMRepository implements KMUpgradable { private byte[] dataTable; private short dataIndex; private short[] reclaimIndex; - // This variable is used to check if power reset event occurred. + // This variable is used to monitor the power reset status as the Applet does not get + // any power reset event. Initially the value of this variable is set to POWER_RESET_STATUS_FLAG. + // If the power reset happens then this value becomes 0. private byte[] powerResetStatus; // Operation table. @@ -135,7 +137,8 @@ public KMRepository(boolean isUpgrading) { repository = this; } - // This function should only be called before processing any of the APUs. + // This function checks if card reset event occurred and this function + // should only be called before processing any of the APUs. // Transient memory is cleared in two cases: // 1. Card reset event // 2. Applet upgrade. @@ -146,6 +149,10 @@ public boolean isPowerResetEventOccurred() { return true; } + /** + * This function sets the power reset status flag to its + * default value. + */ public void restorePowerResetStatus() { powerResetStatus[0] = POWER_RESET_STATUS_FLAG; } From 5a10d4f85330fc56bf6d55f24103d5f12110aab3 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Wed, 28 Apr 2021 21:50:18 +0100 Subject: [PATCH 20/21] renamed function names and added comments. --- HAL/keymaster/4.1/JavacardKeymaster4Device.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp index 090ea6ba..76058f52 100644 --- a/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp +++ b/HAL/keymaster/4.1/JavacardKeymaster4Device.cpp @@ -252,27 +252,19 @@ static inline int32_t get2sCompliment(uint32_t value) { return static_cast(~value+1); } -/** - * This function separates the original error code from the - * power reset flag and returns the original error code. - */ -static inline uint32_t extractActualErrorCode(uint32_t err) { - // Unset the power rest flag. - return (err & ~SE_POWER_RESET_STATUS_FLAG); -} - /** * Clears all the strongbox operation handle entries if secure element power reset happens. - * And also extracts the original error code value. + * And also extracts the error code value after unmasking the power reset status flag. */ -static uint32_t handleSePowerResetAndExtractOriginalErrorCode(const std::unique_ptr& oprCtx, uint32_t errorCode) { +static uint32_t handleErrorCode(const std::unique_ptr& oprCtx, uint32_t errorCode) { //Check if secure element is reset bool isSeResetOccurred = (0 != (errorCode & SE_POWER_RESET_STATUS_FLAG)); if (isSeResetOccurred) { //Clear the operation table for Strongbox operations entries. clearStrongboxOprHandleEntries(oprCtx); - return extractActualErrorCode(errorCode); + // Unmask the power reset status flag. + errorCode &= ~SE_POWER_RESET_STATUS_FLAG; } return errorCode; } @@ -284,7 +276,7 @@ static std::tuple, T> decodeData(CborConverter& cb, const T errorCode = T::OK; std::tie(item, errorCode) = cb.decodeData(response, hasErrorCode); - uint32_t tempErrCode = handleSePowerResetAndExtractOriginalErrorCode(oprCtx, static_cast(errorCode)); + uint32_t tempErrCode = handleErrorCode(oprCtx, static_cast(errorCode)); // SE sends errocode as unsigned value so convert the unsigned value // into a signed value of same magnitude and copy back to errorCode. From 806bbc4977222208ce61b4686e7c7f47355ca124 Mon Sep 17 00:00:00 2001 From: BKSSM Venkateswarlu Date: Thu, 29 Apr 2021 02:59:06 +0530 Subject: [PATCH 21/21] variable names changed properly --- ProvisioningTool/Provision.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ProvisioningTool/Provision.cpp b/ProvisioningTool/Provision.cpp index 950b9203..0f31fb86 100644 --- a/ProvisioningTool/Provision.cpp +++ b/ProvisioningTool/Provision.cpp @@ -131,13 +131,13 @@ static inline int32_t get2sCompliment(uint32_t value) { * This function separates the original error code from the * power reset flag and returns the original error code. */ -static uint32_t extractOriginalErrorCode(uint32_t errorCode) { +static uint32_t extractErrorCode(uint32_t errorCode) { //Check if secure element is reset bool isSeResetOccurred = (0 != (errorCode & SE_POWER_RESET_STATUS_FLAG)); if (isSeResetOccurred) { LOG(ERROR) << "Secure element reset happened"; - return (errorCode & ~SE_POWER_RESET_STATUS_FLAG); + errorCode &= ~SE_POWER_RESET_STATUS_FLAG; } return errorCode; } @@ -147,9 +147,8 @@ static std::tuple, T> decodeData(CborConverter& cb, const std::unique_ptr item(nullptr); T errorCode = T::OK; std::tie(item, errorCode) = cb.decodeData(response, true); - int32_t temp = static_cast(errorCode); - uint32_t tempErrCode = extractOriginalErrorCode(static_cast(errorCode)); + uint32_t tempErrCode = extractErrorCode(static_cast(errorCode)); // SE sends errocode as unsigned value so convert the unsigned value // into a signed value of same magnitude and copy back to errorCode.