Skip to content

Commit 55689bf

Browse files
committed
Allow reusing PIN tokens for getAssertion with UP=false
1 parent 2e53f10 commit 55689bf

File tree

2 files changed

+68
-19
lines changed

2 files changed

+68
-19
lines changed

python_tests/ctap/test_ctap_basics.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,51 @@ def test_basic_assertion(self):
356356
assert_res.auth_data + assert_client_data, assert_res.signature
357357
)
358358

359+
def test_pin_uv_reuse(self):
360+
cred_res = self.ctap2.make_credential(**self.basic_makecred_params)
361+
362+
pin = secrets.token_hex(10)
363+
cp = ClientPin(self.ctap2)
364+
cp.set_pin(pin)
365+
uv = cp.get_pin_token(pin)
366+
assert_client_data = self.get_random_client_data()
367+
pin_uv_protocol = cp.protocol.VERSION
368+
pin_uv_param = cp.protocol.authenticate(uv, assert_client_data)
369+
370+
assert_res1 = self.get_assertion_from_cred(cred_res, client_data=assert_client_data,
371+
pin_uv_protocol=pin_uv_protocol,
372+
pin_uv_param=pin_uv_param,
373+
options={
374+
'up': False
375+
})
376+
assert_res2 = self.get_assertion_from_cred(cred_res, client_data=assert_client_data,
377+
pin_uv_protocol=pin_uv_protocol,
378+
pin_uv_param=pin_uv_param,
379+
options={
380+
'up': False
381+
})
382+
assert_res3 = self.get_assertion_from_cred(cred_res, client_data=assert_client_data,
383+
pin_uv_protocol=pin_uv_protocol,
384+
pin_uv_param=pin_uv_param,
385+
options={
386+
'up': True
387+
})
388+
error_raised = False
389+
try:
390+
self.get_assertion_from_cred(cred_res, client_data=assert_client_data,
391+
pin_uv_protocol=pin_uv_protocol,
392+
pin_uv_param=pin_uv_param,
393+
options={
394+
'up': False
395+
})
396+
except CtapError as e:
397+
self.assertEqual(CtapError.ERR.PIN_AUTH_INVALID, e.code)
398+
error_raised = True
399+
self.assertTrue(error_raised)
400+
for assert_res in [assert_res1, assert_res2, assert_res3]:
401+
self.assertEqual(cred_res.auth_data.credential_data.credential_id,
402+
assert_res.credential['id'])
403+
359404
def test_creds_are_tamper_resistant(self):
360405
cred_res = self.ctap2.make_credential(**self.basic_makecred_params)
361406

src/main/java/us/q3q/fido2/FIDO2Applet.java

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ private void makeCredential(APDU apdu, short lc, byte[] buffer) {
822822
// Come back and verify PIN auth
823823
byte pinPermissions = transientStorage.getPinPermissions();
824824

825-
verifyPinAuth(apdu, buffer, pinAuthIdx, buffer, clientDataHashIdx, pinProtocol);
825+
verifyPinAuth(apdu, buffer, pinAuthIdx, buffer, clientDataHashIdx, pinProtocol, true);
826826

827827
if ((pinPermissions & FIDOConstants.PERM_MAKE_CREDENTIAL) == 0) {
828828
// PIN token doesn't have permission for the MC operation
@@ -1397,16 +1397,18 @@ private void hmacSha256(APDU apdu, byte[] keyBuff, short keyOff,
13971397
/**
13981398
* Uses the currently-set pinToken to hash some data and compare against a verification value
13991399
*
1400-
* @param apdu Request/response object
1401-
* @param content Buffer containing content to HMAC using the pinToken
1402-
* @param contentIdx Index of content in given buffer
1403-
* @param contentLen Length of content
1404-
* @param checkAgainst Buffer containing "correct" hash we're looking for
1405-
* @param checkIdx Index of correct hash in corresponding buffer
1406-
* @param pinProtocol Integer PIN protocol version number
1400+
* @param apdu Request/response object
1401+
* @param content Buffer containing content to HMAC using the pinToken
1402+
* @param contentIdx Index of content in given buffer
1403+
* @param contentLen Length of content
1404+
* @param checkAgainst Buffer containing "correct" hash we're looking for
1405+
* @param checkIdx Index of correct hash in corresponding buffer
1406+
* @param pinProtocol Integer PIN protocol version number
1407+
* @param invalidateToken If true, consider invalidating the PIN token
14071408
*/
14081409
private void checkPinToken(APDU apdu, byte[] content, short contentIdx, short contentLen,
1409-
byte[] checkAgainst, short checkIdx, byte pinProtocol) {
1410+
byte[] checkAgainst, short checkIdx, byte pinProtocol,
1411+
boolean invalidateToken) {
14101412
if (pinProtocol != transientStorage.getPinProtocolInUse()) {
14111413
// Can't use PIN protocol 1 with tokens created with v2 or vice versa
14121414
sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
@@ -1435,7 +1437,7 @@ private void checkPinToken(APDU apdu, byte[] content, short contentIdx, short co
14351437

14361438
bufferManager.release(apdu, scratchHandle, (short) 32);
14371439

1438-
if (ONE_USE_PER_PIN_TOKEN) {
1440+
if (invalidateToken && ONE_USE_PER_PIN_TOKEN) {
14391441
transientStorage.setPinProtocolInUse((byte) 3, (byte) 0);
14401442
}
14411443
}
@@ -1449,10 +1451,11 @@ private void checkPinToken(APDU apdu, byte[] content, short contentIdx, short co
14491451
* @param clientDataHashBuffer Buffer containing client data hash
14501452
* @param clientDataHashIdx Index of the hash of the clientData object, as given by the platform
14511453
* @param pinProtocol Integer PIN protocol number
1454+
* @param invalidateToken If true, consider invalidating the PIN token
14521455
*/
14531456
private void verifyPinAuth(APDU apdu, byte[] buffer, short readIdx,
14541457
byte[] clientDataHashBuffer, short clientDataHashIdx,
1455-
byte pinProtocol) {
1458+
byte pinProtocol, boolean invalidateToken) {
14561459
byte desiredLength = 16;
14571460
if (pinProtocol == 2) {
14581461
desiredLength = 32;
@@ -1490,7 +1493,7 @@ private void verifyPinAuth(APDU apdu, byte[] buffer, short readIdx,
14901493
}
14911494

14921495
checkPinToken(apdu, clientDataHashBuffer, clientDataHashIdx, CLIENT_DATA_HASH_LEN,
1493-
buffer, readIdx, pinProtocol);
1496+
buffer, readIdx, pinProtocol, invalidateToken);
14941497
}
14951498

14961499
/**
@@ -2147,17 +2150,18 @@ private void getAssertion(final APDU apdu, final short lc, final byte[] buffer,
21472150
}
21482151

21492152
// check the provided PIN
2150-
byte pinPermissions = transientStorage.getPinPermissions();
2153+
final byte pinPermissions = transientStorage.getPinPermissions();
2154+
final boolean shouldInvalidatePinToken = transientStorage.hasUPOption();
21512155

21522156
verifyPinAuth(apdu, buffer, pinAuthIdx, clientDataHashBuffer, clientDataHashIdx,
2153-
stateKeepingBuffer[stateKeepingIdx]);
2157+
stateKeepingBuffer[stateKeepingIdx], shouldInvalidatePinToken);
21542158

21552159
if ((pinPermissions & FIDOConstants.PERM_GET_ASSERTION) == 0) {
21562160
// PIN token doesn't have permission for the GA operation
21572161
sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
21582162
}
21592163

2160-
if (WRITES_INVALIDATE_PINS) {
2164+
if (WRITES_INVALIDATE_PINS && shouldInvalidatePinToken) {
21612165
transientStorage.setPinProtocolInUse((byte) 3, (byte) 0);
21622166
}
21632167

@@ -3904,7 +3908,7 @@ private void handleLargeBlobs(APDU apdu, byte[] reqBuffer, short lc) {
39043908
byte pinPerms = transientStorage.getPinPermissions();
39053909

39063910
checkPinToken(apdu, scratch, scratchOffset, (short) 70,
3907-
reqBuffer, pinUvAuthIdx, pinProtocol);
3911+
reqBuffer, pinUvAuthIdx, pinProtocol, true);
39083912

39093913
if ((pinPerms & FIDOConstants.PERM_LARGE_BLOB_WRITE) == 0x00) {
39103914
sendErrorByte(apdu, FIDOConstants.CTAP2_ERR_PIN_AUTH_INVALID);
@@ -4458,7 +4462,7 @@ private void authenticatorConfigSubcommand(APDU apdu, byte[] reqBuffer, short lc
44584462
}
44594463

44604464
checkPinToken(apdu, scratchBuffer, scratchOffset, bufferSize,
4461-
reqBuffer, authIndex, pinProtocol);
4465+
reqBuffer, authIndex, pinProtocol, true);
44624466

44634467
if (WRITES_INVALIDATE_PINS) {
44644468
transientStorage.setPinProtocolInUse((byte) 3, (byte) 0);
@@ -4727,10 +4731,10 @@ private void credManagementSubcommand(APDU apdu, byte[] buffer, short lc) {
47274731
// Straight-up mangle the input buffer to put the command byte immediately before the subcommand params
47284732
buffer[(short)(subCommandParamsIdx - 1)] = buffer[subCommandIdx];
47294733
checkPinToken(apdu, buffer, (short) (subCommandParamsIdx - 1), (short) (subCommandParamsLen + 1),
4730-
buffer, readIdx, pinProtocol);
4734+
buffer, readIdx, pinProtocol, true);
47314735
} else {
47324736
checkPinToken(apdu, buffer, subCommandIdx, (short) 1,
4733-
buffer, readIdx, pinProtocol);
4737+
buffer, readIdx, pinProtocol, true);
47344738
}
47354739
}
47364740

0 commit comments

Comments
 (0)