Skip to content

Conversation

tcharding
Copy link
Member

Follow up of #844

Fix rerandomization stuff.

  • Use keypair secret data to rerandomize in musig when doing partial siging.
  • Remove all the zero seed arrays and use None to disable rerandomization when no secret data used.

In `Session::partial_sign` rerandomize using the secret bytes from the
keypair.
Turns out I didn't (and maybe still don't) understand why we do
re-randomization. I looked online and came up with

> *  The purpose of context structures is to cache large precomputed data tables
> *  that are expensive to construct, and also to maintain the randomization data
> *  for blinding.

So we only re-randomize after signing? Or from what I gleaned from
Andrew's comments on the original PR, only in functions that contain
secret data (which I _think_ implies signing).

All instances remove the seed and use `None` to disable re-randomization.
@tcharding tcharding changed the title Push vrqyzmmttmlm Fix rerandomization seed usage Sep 17, 2025
This was referenced Sep 17, 2025
@apoelstra
Copy link
Member

Looks great!

Arguably in the signing methods we should xor in the message, and in ElligatorSwift::from_seckey we should xor in the auxliary randomness if it's available. The idea here is that if the user signs with multiple messages, we won't rerandomize with the same seed. This might provide additional protection in some multi-threaded scenarios. (But it would still not help in the case of a user who is doing many signatures and spawning a new thread for each one. I guess we ought to discourage that in the docs somewhere.)

But I'm kinda tired of thinking about this, and the marginal benefit is really small, so let's go with this for now.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 99171f8; successfully ran local tests

@apoelstra apoelstra merged commit 649ea8b into rust-bitcoin:master Sep 17, 2025
28 checks passed
@tcharding tcharding mentioned this pull request Sep 18, 2025
chain-forgexcr45 added a commit to chain-forgexcr45/rust-secp256k1 that referenced this pull request Sep 28, 2025
99171f82a9b7c590e63cb7b9472a0f0cf553b9a2 Don't re-randomize unnecessarily (Tobin C. Harding)
0e0eb60fc06bffcb764f01dee7c3aea7177ac294 musig: Use secret bytes from keypair to rerandomize (Tobin C. Harding)

Pull request description:

  Follow up of #844
  
  Fix rerandomization stuff.
  
  - Use keypair secret data to rerandomize in musig when doing partial siging. 
  - Remove all the zero seed arrays and use `None` to disable rerandomization when no secret data used.


ACKs for top commit:
  apoelstra:
    ACK 99171f82a9b7c590e63cb7b9472a0f0cf553b9a2; successfully ran local tests


Tree-SHA512: 4bc68d9e819b2a0fb94ade67aa29c25c2454f94dcdd623f4fb67f9aa74caac75146bccb8683ed1ed636591d8e1dfb181c76cc16d33b0b57885af9b2b94c8b931
william2332-limf added a commit to william2332-limf/rust-secp256k1 that referenced this pull request Oct 2, 2025
99171f82a9b7c590e63cb7b9472a0f0cf553b9a2 Don't re-randomize unnecessarily (Tobin C. Harding)
0e0eb60fc06bffcb764f01dee7c3aea7177ac294 musig: Use secret bytes from keypair to rerandomize (Tobin C. Harding)

Pull request description:

  Follow up of #844
  
  Fix rerandomization stuff.
  
  - Use keypair secret data to rerandomize in musig when doing partial siging. 
  - Remove all the zero seed arrays and use `None` to disable rerandomization when no secret data used.


ACKs for top commit:
  apoelstra:
    ACK 99171f82a9b7c590e63cb7b9472a0f0cf553b9a2; successfully ran local tests


Tree-SHA512: 4bc68d9e819b2a0fb94ade67aa29c25c2454f94dcdd623f4fb67f9aa74caac75146bccb8683ed1ed636591d8e1dfb181c76cc16d33b0b57885af9b2b94c8b931
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants