fido-alliance / conformance-test-tools-resources

Certification Test Tools Resources. For security and privacy related issues email tools@certification.fidoalliance.org
https://fidoalliance.org/certification/functional-certification/conformance/
40 stars 14 forks source link

Error : "Signature without R or S" even though x and y are extracted properly #741

Closed ghislaindemael closed 7 months ago

ghislaindemael commented 7 months ago

By submitting this issue you are acknowledging that any information regarding this issue will be publicly available.

If you have privacy concerns, please email conformance-tools@fidoalliance.org

FIRST PRE CHECK

What protocol are you implementing?

NOTE: UAF 1.0 certification have been officially sunset. U2F 1.2 only supported version of U2F.

What is your implementation class?

If you are platform authenticator vendor, please email conformance-tools@fidoalliance.org

What is the version of the tool are you using?

v1.7.17

What is the OS and the version are you running?

For desktop tools

Issue description

Hi !

I am currently working on a MakeCredential implementation, with a simili-Arduino.

Launching the MakeCredential Response tests, they are succesful for the first 3, n° 4 and 5 are skipped, however the last test, P-06 fails.

P1P3 P06Error

The error stack is the following :

Error: Signature without r or s at assert (...\resources\app.asar\node_modules\minimalistic-assert\index.js:5:11) at new Signature (...\resources\app.asar\node_modules\elliptic\lib\elliptic\ec\signature.js:15:3) at EC.verify (...\resources\app.asar\node_modules\elliptic\lib\elliptic\ec\index.js:159:15) at KeyPair.verify (...\resources\app.asar\node_modules\elliptic\lib\elliptic\ec\key.js:115:18) at Object.verifySignatureCOSE (...\resources\app.asar\dependencies\cryptodep.js:165:26) at n.eval (eval at compileCode (js/sandbox.js:25:26), :5748:68)

Incriminating lines seem to be :
\elliptic\lib\elliptic\ec\index.js:159 signature = new Signature(signature, 'hex'); as the "signature" that is passed around is let signature = response.attStmtStruct.sig;, which corresponds to a 132-byte array , visible in "Memory" :

image

The authenticator generates "sig" with the following lines.

memcpy(&makeCredSigPreHash[0], &makeCredAuthData[0], 233); memcpy(&makeCredSigPreHash[233], &clientDataHash[0], 32); P521::sign(makeCredSig, privateKey, makeCredSigPreHash, 265);

How could I be able to fix this ? Does the error originate from the P521::sign() function ?

P.S. After numerous checks, the sig is not altered through the BLE frames. It corresponds byte-by-byte to the result of P521::sign.

P.P.S. In my auth_metadata.json, loaded in the tools, I have set the following array :

"authenticationAlgorithms": [ "secp521r1_ecdsa_sha512_raw" ],

iirachek commented 7 months ago

This is likely caused by the sig not being correctly encoded in ASN1. DER format and is why the elliptic errors out with this message.

ghislaindemael commented 7 months ago

Hi !

Switched a little my code to send this as the attStmt :

A2 # Map with "alg" and "sig" 63 61 6C 67 # "alg" 38 23 # -36 (alg code) 63 73 69 67 # "sig" 58 8A # 138-byte byte string (the signature in ASN1.DER format) 30 88 # 136 byte sequence 02 42 # 66-byte integer 00 74 F2 FC CC EF 98 C7 31 43 87 B4 22 A2 44 71 FC AA 84 D6 04 57 40 C6 9D 95 D7 E6 68 BB 8F 76 9F 21 68 97 17 75 CF 14 D4 E9 77 6D 18 DA 65 DF F1 85 30 7F 92 A5 05 AF 3C 82 8D 9B B8 0C 75 88 B0 37 # 66 first bytes of "sig" 02 42 # 66-byte integer 01 D6 46 27 93 5C B6 33 6A 02 20 07 4F B9 99 CB 9C A8 8A B1 D4 83 50 AD CD 4B 74 B0 96 D0 D3 E9 9A EC 34 67 D3 36 8D DE 28 2C 54 8A AC C7 87 9D AE 62 66 A3 78 8C 3A 89 0B E1 B5 8C CE BC ED BC DA 81 # 66 last bytes of "sig"

However, this stills returns me the "Error: Signature without r or s" error.

iirachek commented 7 months ago

Ok, this took a bit longer to figure out than it should, but maybe it'll be a useful reference in the future. And I hope that no mistakes were made during explanation.


Taking the CBOR map listed above as an example, let's start with the value of sig key (line 5) and go from there.

58 8A # 138-byte byte string (the signature in ASN1.DER format)

UPD: Everything is ok here. The 58 8A (byte-string) prefix is required as the attStmt is CBOR Map (see table at end of 6.1.2).


30 88 # 136 byte sequence

The ASN1 actually has three encoding forms for length. The short form uses a single byte to represent values from 0 to 127;
and the long form that uses multiple bytes for values bigger than 127; there is also an indefinite form, but we aren't interested in that one. See point 8.1.3.5 for how the long form is encoded.

Since in our case the length value is above the 127 limit, we must use the long form. Thus the TL will look like this: 30 81 88 # with 81 being an octet that specifies length of length and 88 being actual length of the sequence.


02 42 # 66-byte integer 00 74 F2 FC CC EF 98 C7 31 43 87 B4 22 A2 44 71 FC AA 84 D6 04 57 40 C6 9D 95 D7 E6 68 BB 8F 76 9F 21 68 97 17 75 CF 14 D4 E9 77 6D 18 DA 65 DF F1 85 30 7F 92 A5 05 AF 3C 82 8D 9B B8 0C 75 88 B0 37 # 66 first bytes of "sig"

There is a small issue with padding in this one that could've been missed in other case. See section 8.3 for integer encoding rules.

Fortunately, since R and S are always positive values, the 00 padding is only added if the next octet has a value of 0x80 or more (leading bit is 1). Otherwise no padding is applied in our case. Note: Depending on the decoder implementation this may not be mandatory (it is in case of the conformance tools).


In conclusion, the resulting value of sig from above should look like this. 58 8A # 138-byte byte string (containing the signature in ASN1.DER format) 30 81 87 # 135 byte sequence 02 41 # 65-byte integer 74 F2 FC CC EF 98 C7 31 43 87 B4 22 A2 44 71 FC AA 84 D6 04 57 40 C6 9D 95 D7 E6 68 BB 8F 76 9F 21 68 97 17 75 CF 14 D4 E9 77 6D 18 DA 65 DF F1 85 30 7F 92 A5 05 AF 3C 82 8D 9B B8 0C 75 88 B0 37 # 65 first bytes of "sig" 02 42 # 66-byte integer 01 D6 46 27 93 5C B6 33 6A 02 20 07 4F B9 99 CB 9C A8 8A B1 D4 83 50 AD CD 4B 74 B0 96 D0 D3 E9 9A EC 34 67 D3 36 8D DE 28 2C 54 8A AC C7 87 9D AE 62 66 A3 78 8C 3A 89 0B E1 B5 8C CE BC ED BC DA 81

When processed by the tools, it no longer errors out with Signature without r or s and instead should result in a proper signature verification.

ghislaindemael commented 7 months ago

Hi again !

The 58 8A (byte-string) prefix is required as the attStmt is CBOR Map (see table at end of 6.1.2). Omitting these two bytes and starting with 30 81 (aka 0b001_10000 0b100_00001) is treated as a negative integer, followed by a 1-item array, etc. This is invalid CBOR (cbor.me returns the error : Out of bytes to decode (need at least 24170197596917893478 bytes more)).

However, when my sig byte-string by 58 8A 30 81 87 02 41 ... 02 42 ..., the Signature without r or s error disappears.

I now have the AssertionError: The assertion signature can not be verified!: expected false to be true. Debug time for this.

My first idea would be to debug the Signature object created, however there is something I don't fully understand. Indeed, when taking memory snapshots (both v1.7.17, one when sending straight the 132-byte sig, the other with the 138-byte sig in ASN.DER format) Heapsnap1.txt Heapsnap2.txt (converted to .txt as GitHub doesn't allow .heapsnaphot uploads), I have a Signature object in the 132-byte one, but none is created in the "valid" one.

What would be the reason for the Signature object (...\resources\app.asar\node_modules\elliptic\lib\elliptic\ec\index.js:159) to not be recorded in the memory ?

EDIT : This is the case for multiple tests, when error Signature without r or s occurs, Signature is in the snapshot, but not when sig is properly configured.

ghislaindemael commented 7 months ago

After reviewing my code, the source of the error may be that the library that I use, more specifically the P521 signing function truncates the message to 64 bytes if it is longer.

In my case, the concatenation of authData and cientDataHash sum to 265 bytes (233 + 32), which exceeds the 64-byte limit.

Since I can't seem to find a similar truncation in the Tools verification procedure, would this be why the signatures differ ?

iirachek commented 7 months ago

The 58 8A (byte-string) prefix is required as the attStmt is CBOR Map

Good point, I'll amend that part in my message.


After reviewing my code, the source of the error may be that the library that I use, more specifically the P521 signing function truncates the message to 64 bytes if it is longer.

From the reading of it, truncating to 64 bytes by P521::sign() is only done to the pre-hashed messages. Which makes sense, since the output of SHA512 will be 64 bytes and so is the expected length of hashed message.

P521::sign(makeCredSig, privateKey, makeCredSigPreHash, 265); in this line the fifth hash parameter is missing. If I'm reading the documentation correctly, unspecified hash defaults to a null-pointer. In such cases message is treated as one that has been hashed beforehand and thus function only truncates it to the expected size. (line 286)

I suppose, signature verification in the tools fails because the message isn't hashed before signing. Supplying hash parameter to the sign() should produce a correct output.

ghislaindemael commented 7 months ago

Hello again @iirachek

ASN encoding works well, I remove the 0-padding for both R and S if it exists, and I now only have the "Assertion Error", even though I properly hash my message with SHA512 (results correspond to the one with online implementations such as this one.

Assuming the P51::sign() implementation is correct, I chose to look deeper at the files and I found the following case. I'm continuing on this thread, but if you desire, I can create a new issue.

According to the IANA COSE Algorihtms, my implementation relies on ECDSA w/ SHA-512, with code -36.

This is the code I send in alg in my attStmt. 38 23 in uint8_t attStmtHeader[13] {0xa2, 0x63, 0x61, 0x6c, 0x67, 0x38, 0x23, 0x63, 0x73, 0x69, 0x67, 0x58, sigLen};

However, in the Tools, the verification function : (...\resources\app.asar\dependencies\cryptodep.js:139)

verifySignatureCOSE: (cosekey, messageBuffer, signatureBuffer) => {
        let coseKey = cbor.CBORBufferToNATIVESTRUCT(cosekey)[0];
        let hashAlg = COSE_ALG_HASH[coseKey.get(COSE_KEYS.alg)];
        let result = false;
        if(coseKey.get(COSE_KEYS.kty) === COSE_KTY.RSA) {
            //SKIP
        } else if(coseKey.get(COSE_KEYS.kty) === COSE_KTY.EC2) {
            let xCoefficient = coseKey.get(COSE_KEYS.x);
            let yCoefficient = coseKey.get(COSE_KEYS.y);

            let keyBuffer   = Buffer.concat([Buffer.from([0x04]), xCoefficient, yCoefficient]);
            let messageHash = hash(hashAlg, messageBuffer);
            let curve       = COSE_CRV_CURVE[coseKey.get(COSE_KEYS.crv)];

            let ec  = new elliptic.ec(curve);
            let key = ec.keyFromPublic(keyBuffer);

            result = key.verify(messageHash, signatureBuffer);
        } else if(coseKey.get(COSE_KEYS.kty) === COSE_KTY.OKP) {
           //SKIP
        }
        return result
    },

uses the COSE_ALG_HASH dictionary (...\resources\app.asar\dependencies\cryptodep.js:93)

let COSE_ALG_HASH = {
    '-257'  : 'SHA-256', // RSASSA-PKCS1-v1_5 w/ SHA-256 Section 8.2 of [RFC8017]
    '-258'  : 'SHA-384', // RSASSA-PKCS1-v1_5 w/ SHA-384 Section 8.2 of [RFC8017]
    '-259'  : 'SHA-512', // RSASSA-PKCS1-v1_5 w/ SHA-512 Section 8.2 of [RFC8017]
    '-65535': 'SHA-1',   // RSASSA-PKCS1-v1_5 w/ SHA-1 Section 8.2 of [RFC8017]
    '-39'   : 'SHA-512',  // RSASSA-PSS w/ SHA-512  [RFC8230]
    '-38'   : 'SHA-384',  // RSASSA-PSS w/ SHA-384 [RFC8230]
    '-37'   : 'SHA-256',   // RSASSA-PSS w/ SHA-256 [RFC8230]
    '-260'  : 'SHA-256', // TPM_ECC_BN_P256 curve w/ SHA-256
    '-261'  : 'SHA-512', // ECC_BN_ISOP512 curve w/ SHA-512
    '-7'    : 'SHA-256',   // ECDSA w/ SHA-256 
    '-36'   : 'SHA-384',  // ECDSA w/ SHA-384 
    '-37'   : 'SHA-512'  // ECDSA w/ SHA-512
}

where -36 does not point to the proper algorithm (SHA-384 instead of SHA-512). Hence I believe the source of the AssertionError is here.

I could theoretically cheat out in the code to pass the test, but that is not proper coding practice, therefore I'll wait for your communication on this issue.

Sincirely, Ghislain

EDIT : Attempting to cheat this error out results in another error as -37 is not a valid algorithm for ECDSA. image

iirachek commented 7 months ago

To keep everything organized, it's better to open another issue for the mismatching algorithm identifiers.