Skip to content

Add more explicit constant time warnings#638

Merged
bwesterb merged 1 commit into
mainfrom
bas/gw10
Jul 15, 2026
Merged

Add more explicit constant time warnings#638
bwesterb merged 1 commit into
mainfrom
bas/gw10

Conversation

@bwesterb

@bwesterb bwesterb commented Jul 14, 2026

Copy link
Copy Markdown
Member

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

// Package partiallyblindrsa implements a partially blind RSA protocol.
//
// Warning: The per-request derivation of the metadata-specific key uses
// math/big and is not constant time in the private key.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// math/big and is not constant time in the private key.
// math/big and may therefore leak the private key via a timing side channel.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think it's better to write a bit about this in README.md (todo). Better to mark where things are not constant time for now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread tss/rsa/rsa_threshold.go
// by Victor Shoup [1].
//
// Warning: Sign operations rely on math/big and are not constant time in the
// secret key share. Pass a non-nil random source to Sign to enable blinding.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you mean "blinding" as in "masking side channels" or "blinding factor in a cryptographic operation"? If the former, mention how the random source is used for masking.

It also seems a bit weird that randomness source controls this behavior. What else is the randomness source used for in this function?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Blinding is the standard way to do this for RSA: it's rare for RSA to be implemented in constant time. I'd rather mention the fact then give a lecture in the comments.

@lukevalenta lukevalenta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved pending addressing Chris' comments

@bwesterb bwesterb merged commit 0c7e54b into main Jul 15, 2026
14 checks passed
@bwesterb bwesterb deleted the bas/gw10 branch July 15, 2026 14:49
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.

3 participants