Skip to content
This repository has been archived by the owner on Apr 20, 2022. It is now read-only.

Make RFC6979 compliant. #115

Closed
wants to merge 1 commit into from
Closed

Make RFC6979 compliant. #115

wants to merge 1 commit into from

Conversation

bip32JP
Copy link

@bip32JP bip32JP commented Jan 2, 2015

I have made the same pull request to bitcore in regards to the missing HMAC.

Please check here for an explanation for the extra V = HMAC_K(V).
bitpay/bitcore#884

As for the bad r or s:
RFC specifies that if the k value is out of bounds OR if r or s is 0 you must loop through Step h until a proper value is found.

The way bitcore implements this is by including an integer called "badrs" that will force entering the re-hashing loop of Step h. The incrementing of badrs is done in a do while loop around k generation and r and s calculation.

This is not a large issue, as the only possible vector of attack for it as is... is if someone had a transaction + privkey pair that just happened to produce an out of bounds k... AND they decided to sign that transaction using both blockchain.info AND Electrum (uses python-ecdsa, 100% RFC6979 compliant) and then for some reason posted those two signed transactions in the public space somehow.

Either way, this needs to be fixed.

I will make a pull request to bitcoinjs-lib later.

@dabura667
Copy link

bitcoinjs-lib just merged their own implementation of this.

It is slightly more elegant than the bitcore solution (what I am proposing here) but also is harder to see how it will work in action.

https://github.com/bitcoinjs/bitcoinjs-lib/pull/337/files

@kitsuphat2528
Copy link

Hard to do

@bip32JP
Copy link
Author

bip32JP commented May 22, 2017

Looks like this is already dealt with.

@bip32JP bip32JP closed this May 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants