Kitura / BlueECC

Elliptic-curve cryptography for Swift
Apache License 2.0
92 stars 35 forks source link

secp521r1 signatures based on r|s are sometimes invalid #21

Open ChaosCoder opened 4 years ago

ChaosCoder commented 4 years ago

We saw some issues when using Swift-JWT with the ES512 signer, which uses the BlueECC crypto underneath. The issue was, that the generated signature is sometimes invalid, producing r and s values with 65 Bytes instead of 66 Bytes in ~0,1% of the cases.

By adding the following test, you see that the verification fails sometimes:

func test_SignatureGeneration() throws {
    let secp521r1Key = try ECPrivateKey.make(for: .secp521r1)
    let secp521r1PubKey = try secp521r1Key.extractPublicKey()
    do {
        for i in 1 ..< 10000 {
            let signature521 = try "Hello world".sign(with: secp521r1Key)
            XCTAssertEqual(signature521.r.count, 66) // those two assertions
            XCTAssertEqual(signature521.s.count, 66) // will sometimes fail
            let copiedSignature = try ECSignature(r: signature521.r, s: signature521.s)
            let verified521 = copiedSignature.verify(plaintext: "Hello world", using: secp521r1PubKey)
            XCTAssertTrue(verified521)
            print("\(i) done")
        }
    } catch {
        return XCTFail("test_SignatureGeneration failed: \(error)")
    }
}

During debugging, I found that in the asn1ToRSSig method, the returned trimmed r and s values sometimes have 65 Bytes instead of 66. However, I was unsuccessful just padding them with null bytes.

The error must be somewhere in the transformation from the original asn1 to the rs format, because when using asn1 in the test above directly:

let copiedSignature = try ECSignature(asn1: signature521.asn1)

the error does not occur.

Note: I was unable to reproduce this on a different curve (e.g. es384). I guess this is due to the nature of the 521 bit curve.