search

google / OpenSK

OpenSK is an open-source implementation for security keys written in Rust that supports both FIDO U2F and FIDO2 standards.
Apache License 2.0
2.98k stars 289 forks source link

MakeCredentialMissingParameterTest fails #584

Closed bunnie closed 1 year ago

bunnie commented 1 year ago

Expected Behavior

This test:

https://github.com/google/CTAP2-test-tool/blob/1afd50bd1b700dc86d6dffeca70a331771d24e45/src/tests/make_credential.cc#L593-L609

Should report a passing result, by denying the transaction.

Actual Behavior

The transaction is allowed to proceed and return a credential response, as seen below:

Failed test: Tests if client PIN fails with missing parameters in MakeCredential. - Missing PIN parameters were not rejected when PIN is set.
A prompt was sent unexpectedly.
Expected error code `CTAP2_ERR_PIN_REQUIRED`, got `CTAP2_OK`.
The failing error code is `CTAP1_ERR_OTHER`.

Steps to Reproduce the Problem

  1. Run OpenSK develop branch against the CTAP2-test-tool
  2. Search for the error message in the log

Specifications

bunnie commented 1 year ago

Below is a transaction log of the full failure:

DBG :vault::ctap: Received command: Ok(AuthenticatorClientPin(AuthenticatorClientPinParameters { pin_uv_auth_protocol: V1, sub_command: GetPinToken, key_agreement: Some(CoseKey { x_bytes: [14, 12, 64, 73, 139, 19, 24, 87, 63, 158, 188, 210, 203, 141, 249, 87, 136, 103, 115, 133, 20, 104, 171, 142, 107, 80, 175, 219, 74, 218, 207, 48], y_bytes: [249, 117, 124, 104, 51, 38, 110, 64, 18, 35, 8, 15, 156, 246, 249, 159, 164, 217, 90, 185, 137, 172, 171, 250, 210, 55, 219, 70, 222, 244, 129, 245], algorithm: -25, key_type: 2, curve: 1 }), pin_uv_auth_param: None, new_pin_enc: None, pin_hash_enc: Some([30, 53, 216, 15, 149, 131, 153, 158, 226, 3, 223, 161, 78, 66, 198, 134]), permissions: None, permissions_rp_id: None })) (apps\vault\src\ctap\mod.rs:576)
DBG :persistent_store::store: find key: 2044 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :persistent_store::store: find key: 2045 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :persistent_store::store: find key: 2044 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :persistent_store::store: pre-delete key: opensk:2044 (apps\vault\libraries\persistent_store\src\store.rs:281)
DBG :persistent_store::store: write key: opensk:2044 (apps\vault\libraries\persistent_store\src\store.rs:287)
DBG :persistent_store::store: remove key: opensk:2044 (apps\vault\libraries\persistent_store\src\store.rs:311)
DBG :persistent_store::store: find key: 2040 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :vault::ctap: Sending response: Ok(AuthenticatorClientPin(Some(AuthenticatorClientPinResponse { key_agreement: None, pin_uv_auth_token: Some([209, 71, 104, 188, 16, 106, 123, 212, 182, 221, 216, 19, 23, 123, 152, 59, 82, 115, 112, 206, 221, 156, 206, 150, 243, 194, 55, 91, 127, 226, 186, 151]), retries: None, power_cycle_state: None }))) (apps\vault\src\ctap\mod.rs:579)
DBG :vault::ctap: Received command: Ok(AuthenticatorMakeCredential(AuthenticatorMakeCredentialParameters { client_data_hash: [205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205, 205], rp: PublicKeyCredentialRpEntity { rp_id: "make_credential_pin_auth_missing_parameter.example.com", rp_name: None, rp_icon: None }, user: PublicKeyCredentialUserEntity { user_id: [29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29, 29], user_name: Some("Adam"), user_display_name: None, user_icon: None }, pub_key_cred_params: [PublicKeyCredentialParameter { cred_type: PublicKey, alg: Es256 }], exclude_list: None, extensions: MakeCredentialExtensions { hmac_secret: false, cred_protect: None, min_pin_length: false, cred_blob: None, large_blob_key: None }, options: MakeCredentialOptions { rk: false, uv: false }, pin_uv_auth_param: None, pin_uv_auth_protocol: None, enterprise_attestation: None })) (apps\vault\src\ctap\mod.rs:576)
DBG :persistent_store::store: find key: 2038 (apps\vault\libraries\persistent_store\src\store.rs:385)

Please touch your security key!
Sending 'y'

DBG :persistent_store::store: find key: 2046 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :persistent_store::store: find key: 2046 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :persistent_store::store: find key: 2047 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :persistent_store::store: find key: 3 (apps\vault\libraries\persistent_store\src\store.rs:385)
DBG :vault::ctap: Sending response: Ok(AuthenticatorMakeCredential(AuthenticatorMakeCredentialResponse { fmt: "packed", auth_data: [90, 232, 154, 231, 189, 177, 122, 64, 199, 192, 147, 33, 98, 3, 200, 74, 238, 108, 110, 107, 191, 180, 94, 104, 234, 138, 178, 141, 251, 70, 118, 108, 65, 0, 0, 0, 1, 72, 224, 55, 28, 50, 82, 87, 247, 210, 240, 0, 15, 189, 122, 137, 25, 0, 241, 1, 5, 1, 128, 50, 159, 52, 101, 188, 144, 171, 231, 118, 113, 89, 8, 242, 134, 193, 80, 215, 175, 100, 41, 196, 52, 158, 100, 11, 159, 215, 36, 212, 94, 76, 98, 93, 224, 162, 66, 130, 184, 28, 102, 140, 187, 65, 94, 117, 26, 90, 184, 115, 226, 156, 218, 211, 120, 146, 235, 80, 168, 10, 41, 113, 9, 112, 201, 67, 32, 189, 113, 31, 14, 95, 157, 174, 210, 7, 206, 244, 244, 249, 200, 14, 113, 18, 72, 255, 248, 119, 16, 56, 56, 25, 64, 94, 19, 62, 11, 29, 85, 24, 124, 86, 52, 165, 105, 59, 234, 61, 200, 95, 199, 154, 63, 126, 160, 212, 186, 128, 167, 123, 20, 43, 41, 143, 5, 34, 194, 206, 173, 189, 16, 165, 119, 203, 58, 27, 14, 239, 22, 224, 72, 2, 207, 66, 88, 232, 199, 0, 253, 84, 216, 157, 44, 124, 242, 92, 202, 107, 211, 255, 202, 65, 254, 69, 225, 1, 84, 96, 236, 198, 225, 67, 147, 188, 201, 53, 230, 18, 158, 169, 246, 123, 220, 49, 138, 235, 221, 80, 216, 102, 236, 68, 44, 170, 152, 9, 147, 238, 44, 44, 19, 112, 100, 19, 86, 85, 248, 156, 187, 55, 51, 62, 171, 55, 202, 217, 223, 183, 0, 168, 247, 55, 147, 92, 110, 92, 230, 240, 163, 57, 86, 20, 183, 64, 76, 243, 69, 129, 165, 1, 2, 3, 38, 32, 1, 33, 88, 32, 182, 132, 253, 20, 175, 230, 41, 128, 103, 113, 138, 211, 38, 251, 218, 188, 119, 65, 217, 80, 66, 197, 45, 150, 151, 222, 14, 244, 222, 127, 78, 0, 34, 88, 32, 24, 20, 225, 179, 31, 158, 129, 195, 88, 112, 34, 118, 44, 46, 183, 152, 236, 21, 149, 106, 198, 187, 12, 165, 132, 246, 209, 137, 134, 49, 143, 185], att_stmt: PackedAttestationStatement { alg: -7, sig: [48, 69, 2, 33, 0, 187, 210, 213, 198, 38, 202, 53, 3, 117, 225, 166, 32, 137, 125, 94, 219, 18, 152, 63, 70, 41, 198, 174, 163, 214, 200, 67, 37, 119, 139, 197, 55, 2, 32, 30, 186, 106, 21, 33, 246, 178, 16, 83, 119, 13, 124, 239, 143, 102, 202, 103, 205, 23, 168, 201, 49, 252, 225, 166, 4, 175, 10, 196, 191, 158, 28], x5c: None, ecdaa_key_id: None }, ep_att: None, large_blob_key: None })) (apps\vault\src\ctap\mod.rs:579)

--> reset due to PIN fail <--

We did not expect an Ok() result in the end; the result should have been an error code.

kaczmarczyck commented 1 year ago

I read this note as a confirmation for what OpenSK is doing: uv_required

Regarding the transaction log, can you explain why rejection is the correct behavior for OpenSK?

bunnie commented 1 year ago

Ah, the reason I think it's the correct behavior is because that's what the testbench was looking for.

Looking at your other responses, it seems more likely that the testbench is just out of date, and not a problem with your code, then.

bunnie commented 1 year ago

I'll close this out, chalking this up to the test program being out of date...thanks!

kaczmarczyck commented 1 year ago

Sorry for the confusion, and thanks for contributing!