Keymaster CTS Fixes #69
Conversation
2. Able to generate key when both FINGERPRINT and BIOMETRIC were passed as USER_AUTH types. 3. USER_SECURE_ID and NO_AUTH_REQUIRED Mutual exclusive.
2. Validate Tokens in littleEndian and BigEndian formats.
2. Combined cert chain and cert params command in a single command
…_to_static_final Applet version upgrade move to static final
| if (magicNumber != KMKeymasterApplet.KM_MAGIC_NUMBER) { | ||
| // Previous version of the applet does not have versioning support. | ||
| // In this case the first byte is the provision status. | ||
| provisionStatus = magicNumber; |
There was a problem hiding this comment.
Maybe a good idea to add validation here to ensure provisionStatus is from the enums itself and not something out of bounds etc.
There was a problem hiding this comment.
As discussed in the meeting no validation is required.
| // Previous version of the applet does not have versioning support. | ||
| // In this case the first byte is the provision status. | ||
| provisionStatus = magicNumber; | ||
| keymasterState = element.readByte(); |
There was a problem hiding this comment.
As discussed in the meeting no validation is required.
| ISOException.throwIt(ISO7816.SW_CONDITIONS_NOT_SATISFIED); | ||
| } | ||
| packageVersion = version; | ||
| provisionStatus = element.readByte(); |
There was a problem hiding this comment.
line #68 - 71 seems to be duplicate in if/else. Please optimize.
There was a problem hiding this comment.
Optimized the code.
| // In this case the first byte is the provision status. | ||
| provisionStatus = magicNumber; | ||
| keymasterState = element.readByte(); | ||
| repository.onRestore(element, packageVersion, CURRENT_PACKAGE_VERSION); |
There was a problem hiding this comment.
what is the value of packageVersion here ?
There was a problem hiding this comment.
Updated this code. In this scenario the packageVersion is null.
When applet is installed for the first time the package version is set to zeros and when applet is getting upgraded this packageVersion holds the previous package version. And for the old applets where no versioning information this value will be null.
| // provisionStatus + keymasterState | ||
| return (short) 4; | ||
| // provisionStatus + keymasterState + magic byte | ||
| return (short) 3; |
There was a problem hiding this comment.
Was it a mistake to have "4" before ? As we are reducing the size now while adding more data ?
There was a problem hiding this comment.
The previous value used to be 2. I even verified the divegeek:Javacard_KM_41_AOSP_UPMerge branch the value is 2 only. May be in this pull request some how it is showing the diff between the previous commits and latest.
| short oldMinorVersion = Util.getShort(version, (short) 2); | ||
| short currentMajorVersion = Util.getShort(CURRENT_PACKAGE_VERSION, (short) 0); | ||
| short currentMinorVersion = Util.getShort(CURRENT_PACKAGE_VERSION, (short) 2); | ||
| // Downgrade of the Applet is not allowed. |
There was a problem hiding this comment.
Check, if we can simplify the logic like this ?
Status = false;
if(currentMajorVersion - oldMajorVersion == 1) {
Status = true;
} else if (currentMajorVersion - oldMajorVersion == 0) {
if(currentMinorVersoin - oldMinorVersion == 1) {
Status = true;
}
return status;
There was a problem hiding this comment.
Yes I used this logic and simplified the code.
| computedHmacKey = KMHmacKey.onRestore(element); | ||
| short oldMajorVersion = Util.getShort(oldVersion, (short) 0); | ||
| short oldMinorVersion = Util.getShort(oldVersion, (short) 2); | ||
| if (oldMajorVersion == 0 && oldMinorVersion == 0) { |
There was a problem hiding this comment.
how do we know that these values will be 0 in olderVersion ?
There was a problem hiding this comment.
As discussed in this link #69 (comment). The value of the old applets package version will be null. so updated the code accordingly.
2. Removed transactions in onRestore
Javacard km 41 aosp upmerge 0630
No description provided.