Recovery of pubic key when manually building signature

[mikekelly]

Hey @csjones, I'm encountering this error when trying to recover a public key:

Fatal error: 'try!' expression unexpectedly raised an error: secp256k1.secp256k1Error.underlyingCryptoError

Steps to reproduce:

        let sigHex = "283f5723bc367993c7492f8d79087b499e776012e0d744426e1be8e12d57264ab765f52dad6dac7df61d2209e875b037390c181b8c205ad47443c5bfea2f08c149"
        let dataHex = "03a69666f5863ecc3b35ac143ef843f2a07ef98a76c09fba6bbd23ea36c7839602"
        
        let sigData = Data(try! sigHex.bytes)
        let data = Data(try! dataHex.bytes)
        
        let sig = try! secp256k1.Recovery.ECDSASignature(dataRepresentation: sigData)
        
        let publicKey = try! secp256k1.Recovery.PublicKey(data, signature: sig)

I've dug around the library's source but I can't seem to resolve or figure out what I'm doing wrong. Is this a bug?

[csjones]

Hey @mikekelly 👋

Something seems off with the sigHex, while the number of bytes is correct, the recID returns 73 which is expected to be invalid.

Here is the code I used to check:

let sigHex = "283f5723bc367993c7492f8d79087b499e776012e0d744426e1be8e12d57264ab765f52dad6dac7df61d2209e875b037390c181b8c205ad47443c5bfea2f08c149"
let dataHex = "03a69666f5863ecc3b35ac143ef843f2a07ef98a76c09fba6bbd23ea36c7839602"
        
let sigData = Data(try! sigHex.bytes)
let data = Data(try! dataHex.bytes)

let sig = try! secp256k1.Recovery.ECDSASignature(dataRepresentation: sigData)

print(sig.dataRepresentation.base64EncodedString())
print(try! sig.compactRepresentation.recoveryId)

let expectedSignature = "rPnhleCU8vQOthm5h4gX/5UbmxH6w3zw1ykAmLvvtXT4YGKBoiMaP8eBBF8upN8IaTYmO7+o0Vyhf+cODD1uVgE="
let expectedDerSignature = Data(base64Encoded: expectedSignature, options: .ignoreUnknownCharacters)!
let expectedSig = try! secp256k1.Recovery.ECDSASignature(expectedDerSignature)

print(expectedSig.dataRepresentation.base64EncodedString())
print(try! expectedSig.compactRepresentation.recoveryId)

And here is the output:

KD9XI7w2eZPHSS+NeQh7SZ53YBLg10RCbhvo4S1XJkq3ZfUtrW2sffYdIgnodbA3OQwYG4wgWtR0Q8W/6i8IwUk=
73
rPnhleCU8vQOthm5h4gX/5UbmxH6w3zw1ykAmLvvtXT4YGKBoiMaP8eBBF8upN8IaTYmO7+o0Vyhf+cODD1uVgE=
1

[mikekelly]

Hi @csjones thanks for having a look. To me, isn't the the rec id of sigHex in range?

        let sigHex = "283f5723bc367993c7492f8d79087b499e776012e0d744426e1be8e12d57264ab765f52dad6dac7df61d2209e875b037390c181b8c205ad47443c5bfea2f08c149"
        let sigData = Data(try! sigHex.bytes)
        print(String(sigData.map { String($0) }.joined(separator: " ")))

outputs

40 63 87 35 188 54 121 147 199 73 47 141 121 8 123 73 158 119 96 18 224 215 68 66 110 27 232 225 45 87 38 74 183 101 245 45 173 109 172 125 246 29 34 9 232 117 176 55 57 12 24 27 140 32 90 212 116 67 197 191 234 47 8 193 73

ie. the rec id is 40 according to the format?

[mikekelly]

sorry, I meant the header is 40 - the rec id is 1

[csjones]

sorry, I meant the header is 40 - the rec id is 1

I'm not sure how you're getting a recID of 1. Given a header byte value of 40 (in decimal format which corresponds to the hexadecimal value 28), the least significant 2 bits are "00" meaning the recID is 0.

How is the signature being created?

[mikekelly]

The signature comes from a closed source hardware wallet, doesn't 40 resolve to 1 according to this BIP?

https://github.com/bitcoin/bips/blob/master/bip-0137.mediawiki#procedure-for-signingverifying-a-signature

[mikekelly]

I can get around all of it by manually picking off the header, deriving the rec id, and then constructing the sig:

            let header = UInt8(sigData.first!)
            let recId = recIdFromHeader(header)
            let compact = sigData.dropFirst()
            let sig = try! secp256k1.Recovery.ECDSASignature(compactRepresentation: compact, recoveryId: recId)

where recIdFromHeader looks like this (implementation lifted from the BIP referenced above) :

func recIdFromHeader(_ header: UInt8) -> Int32 {
        var adjustedValue = header
        
        if(adjustedValue >= 39) // this is a bech32 signature
        {
            adjustedValue -= 12;
        } // this is a segwit p2sh signature
        else if(adjustedValue >= 35)
        {
            adjustedValue -= 8;
        } // this is a compressed key signature
        else if (adjustedValue >= 31) {
            adjustedValue -= 4;
        }
        
        return Int32(adjustedValue - 27);
    }

[csjones]

Thanks for the information. Yes, I see how the recID is 1 because of BIP137 (i.e. 40-39=1); I'll need to investigate more though. Stepping through, secp256k1_ecdsa_recover( returns 0 which raises the error, secp256k1.secp256k1Error.underlyingCryptoError.

Might be helpful to use the bindings directly to recreate the issue. Additionally, I think this comment might be related: bitcoin-core/secp256k1#1024 (comment)

[csjones]

Hey @mikekelly,

Investigating this more, it seems we're mixing types,ECDSASignature and ECDSACompactSignature, which are not equivalent.

This line, secp256k1.Recovery.ECDSASignature(dataRepresentation:), is using libsecp256k1's internal signature representation which includes the note "The exact representation of data inside is implementation defined and not guaranteed to be portable between different platforms or versions."

The line that worked for you after dropping the header, secp256k1.Recovery.ECDSASignature(compactRepresentation:recoveryId:) is using the compact signature representation which is expected to be used with BIP-0137.

[mikekelly]

Right, that makes sense. At some point it may be worth creating something like Bip137Signature(dataRepresentation:) just to make this easier for users so they don't have to manually take the header apart like I'm doing. Either way, I've got a working solution so all good my end. Thanks for your help it's much appreciated 🙏

[csjones]

Right, that makes sense. At some point it may be worth creating something like Bip137Signature(dataRepresentation:) just to make this easier for users so they don't have to manually take the header apart like I'm doing. Either way, I've got a working solution so all good my end. Thanks for your help it's much appreciated 🙏

I like the idea and if you're interested in contributing, I'd gladly review the changes for acceptance. 😁

[mikekelly]

Right, that makes sense. At some point it may be worth creating something like Bip137Signature(dataRepresentation:) just to make this easier for users so they don't have to manually take the header apart like I'm doing. Either way, I've got a working solution so all good my end. Thanks for your help it's much appreciated 🙏

I like the idea and if you're interested in contributing, I'd gladly review the changes for acceptance. 😁

Yep for sure man! If/when I get the time to go back and refactor this into a class, I'll create a PR :)