Skip to content

Commit

Permalink
Merge pull request #112 from subrahmanyaman/hmac_sign_len_validation
Browse files Browse the repository at this point in the history 
Hmac sign len validation
  • Loading branch information
@mdwivedi
mdwivedi authored Apr 19, 2022
2 parents b0061fc + 21449c0 commit 0d842b6
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 18 deletions.
1 change: 1 addition & 0 deletions Applet/src/com/android/javacard/keymaster/KMKeyParameters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ public static short makeHidden(short appIdBlob, short appDataBlob, short rootOfT
index += 2;
}
if (appDataBlob != KMTag.INVALID_VALUE) {
KMByteBlob.cast(appDataBlob);
Util.setShort(scratchPad, index, appDataBlob);
index += 2;
}
Expand Down
49 changes: 32 additions & 17 deletions Applet/src/com/android/javacard/keymaster/KMKeymasterApplet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.android.javacard.seprovider.KMAttestationCert;
import com.android.javacard.seprovider.KMDeviceUniqueKeyPair;
import com.android.javacard.seprovider.KMException;
import com.android.javacard.seprovider.KMHmacKey;
import com.android.javacard.seprovider.KMSEProvider;
import javacard.framework.APDU;
import javacard.framework.Applet;
Expand All @@ -29,8 +28,6 @@
import javacard.framework.JCSystem;
import javacard.framework.Util;
import javacard.security.CryptoException;
import javacard.security.HMACKey;
import javacard.security.KeyBuilder;
import javacardx.apdu.ExtendedLength;

/**
Expand Down Expand Up @@ -227,6 +224,10 @@ public class KMKeymasterApplet extends Applet implements AppletEvent, ExtendedLe
// Key type constants
public static final byte SYM_KEY_TYPE = 0;
public static final byte ASYM_KEY_TYPE = 1;
// SHA-256 Digest length in bits
public static final short SHA256_DIGEST_LEN_BITS = 256;
// Minimum HMAC length in bits
public static final short MIN_HMAC_LENGTH_BITS = 64;

protected static RemotelyProvisionedComponentDevice rkp;
protected static KMEncoder encoder;
Expand Down Expand Up @@ -983,7 +984,7 @@ private boolean isKeyUpgradeRequired(short keyBlob, short appId, short appData,
KMException.throwIt(KMError.INVALID_ARGUMENT);
}
} else {
KMException.throwIt(KMError.UNKNOWN_ERROR);
KMException.throwIt(KMError.UNKNOWN_ERROR);
}
index += 4;
}
Expand Down Expand Up @@ -1728,8 +1729,7 @@ private void finishSigningVerifyingOperation(KMOperationState op, byte[] scratch
// the truncated signature back to the caller. At the time of verfication
// we again compute the signature of the plain text input, truncate it to
// TAG_MAC_LENGTH and compare it with the input signature for
// verification. So this is the reason we are using KMType.SIGN directly
// instead of using op.getPurpose().
// verification.
op.getOperation()
.sign(
KMByteBlob.cast(data[INPUT_DATA]).getBuffer(),
Expand All @@ -1741,10 +1741,18 @@ private void finishSigningVerifyingOperation(KMOperationState op, byte[] scratch
// Copy only signature of mac length size.
data[OUTPUT_DATA] =
KMByteBlob.instance(scratchPad, (short) 0, (short) (op.getMacLength() / 8));
}else if (op.getPurpose() == KMType.VERIFY) {
} else if (op.getPurpose() == KMType.VERIFY) {
if ((KMByteBlob.cast(data[SIGNATURE]).length() < (MIN_HMAC_LENGTH_BITS / 8)) ||
KMByteBlob.cast(data[SIGNATURE]).length() > (SHA256_DIGEST_LEN_BITS / 8)) {
KMException.throwIt(KMError.UNSUPPORTED_MAC_LENGTH);
}
if ((KMByteBlob.cast(data[SIGNATURE]).length() < (short)(op.getMinMacLength() / 8))) {
KMException.throwIt(KMError.INVALID_MAC_LENGTH);
}

if (0
!= Util.arrayCompare(
scratchPad, (short)0,
scratchPad, (short) 0,
KMByteBlob.cast(data[SIGNATURE]).getBuffer(),
KMByteBlob.cast(data[SIGNATURE]).getStartOff(),
KMByteBlob.cast(data[SIGNATURE]).length())) {
Expand Down Expand Up @@ -2167,14 +2175,17 @@ private void processBeginOperationCmd(APDU apdu) {
} else {
iv = KMArray.instance((short) 0);
}

short macLen = 0;
if(op.getMacLength() != KMType.INVALID_VALUE) {
macLen = (short) (op.getMacLength()/8) ;
}
short params = KMKeyParameters.instance(iv);
short resp = KMArray.instance((short) 5);
KMArray.cast(resp).add((short) 0, KMInteger.uint_16(KMError.OK));
KMArray.cast(resp).add((short) 1, params);
KMArray.cast(resp).add((short) 2, data[OP_HANDLE]);
KMArray.cast(resp).add((short) 3, KMInteger.uint_8(op.getBufferingMode()));
KMArray.cast(resp).add((short) 4, KMInteger.uint_16((short) (op.getMacLength() / 8)));
KMArray.cast(resp).add((short) 4, KMInteger.uint_16(macLen));
sendOutgoing(apdu, resp);
}

Expand Down Expand Up @@ -2404,6 +2415,9 @@ private void authorizeBlockModeAndMacLength(KMOperationState op) {
}
break;
case KMType.HMAC:
short minMacLen = KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.MIN_MAC_LENGTH,
data[HW_PARAMETERS]);
op.setMinMacLength(minMacLen);
if (macLen == KMType.INVALID_VALUE) {
if (op.getPurpose() == KMType.SIGN) {
KMException.throwIt(KMError.MISSING_MAC_LENGTH);
Expand All @@ -2413,12 +2427,13 @@ private void authorizeBlockModeAndMacLength(KMOperationState op) {
if (op.getPurpose() == KMType.VERIFY) {
KMException.throwIt(KMError.INVALID_ARGUMENT);
}
if (macLen
< KMIntegerTag.getShortValue(
KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, data[HW_PARAMETERS])) {
if (macLen % 8 != 0 ||
macLen > SHA256_DIGEST_LEN_BITS ||
macLen < MIN_HMAC_LENGTH_BITS) {
KMException.throwIt(KMError.UNSUPPORTED_MAC_LENGTH);
}
if (macLen < minMacLen) {
KMException.throwIt(KMError.INVALID_MAC_LENGTH);
} else if (macLen % 8 != 0 || macLen > 256) {
KMException.throwIt(KMError.UNSUPPORTED_MAC_LENGTH);
}
op.setMacLength(macLen);
}
Expand Down Expand Up @@ -3720,8 +3735,8 @@ private static void validateHmacKey() {
KMIntegerTag.getShortValue(KMType.UINT_TAG, KMType.MIN_MAC_LENGTH, data[KEY_PARAMETERS]);

if (((short) (minMacLength % 8) != 0)
|| minMacLength < (short) 64
|| minMacLength > (short) 256) {
|| minMacLength < MIN_HMAC_LENGTH_BITS
|| minMacLength > SHA256_DIGEST_LEN_BITS) {
KMException.throwIt(KMError.UNSUPPORTED_MIN_MAC_LENGTH);
}
// Read Keysize
Expand Down
11 changes: 10 additions & 1 deletion Applet/src/com/android/javacard/keymaster/KMOperationState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ public class KMOperationState {
private static final byte MAC_LENGTH = 7;
private static final byte MGF_DIGEST = 8;
private static final byte AUTH_TYPE = 9;
private static final byte MIN_MAC_LENGTH = 10;
// sizes
public static final byte OPERATION_HANDLE_SIZE = 8;
public static final byte DATA_SIZE = 10;
public static final byte DATA_SIZE = 11;
public static final byte AUTH_TIME_SIZE = 8;
// Secure user ids 5 * 8 = 40 bytes ( Considering Maximum 5 SECURE USER IDs)
// First two bytes are reserved to store number of secure ids. So total 42 bytes.
Expand Down Expand Up @@ -278,6 +279,14 @@ public void setMacLength(short length) {
data[MAC_LENGTH] = length;
}

public void setMinMacLength(short length) {
data[MIN_MAC_LENGTH] = length;
}

public short getMinMacLength() {
return data[MIN_MAC_LENGTH];
}

public short getMacLength() {
return data[MAC_LENGTH];
}
Expand Down

0 comments on commit 0d842b6

Please sign in to comment.