Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkcs1v15: make *_with_prefix methods the default #290

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

tarcieri
Copy link
Member

Renames the following:

  • SigningKey::new => SigningKey::new_unprefixed
  • SigningKey::new_with_prefix => SigningKey::new
  • VerifyingKey::new => VerifyingKey::new_unprefixed
  • VerifyingKey::new_with_prefix => VerifyingKey::new

The *_with_prefix methods are preserved with a deprecation warning, which should help people migrate to the new versions.

Closes #238

Renames the following:

- `SigningKey::new` => `SigningKey::new_unprefixed`
- `SigningKey::new_with_prefix` => `SigningKey::new`
- `VerifyingKey::new` => `VerifyingKey::new_unprefixed`
- `VerifyingKey::new_with_prefix` => `VerifyingKey::new`

The `*_with_prefix` methods are preserved with a deprecation warning,
which should help people migrate to the new versions.

Closes #238
@tarcieri tarcieri requested a review from dignifiedquire April 10, 2023 04:01
@tarcieri
Copy link
Member Author

Per #238 having an empty prefix seems to be quite rare.

People are reaching for new expecting it to work, because in most other environments prefixes seem to "just work" by default.

This change might be confusing for anyone who happens to actually be using empty prefixes, though I haven't encountered anyone who is personally.

@tarcieri
Copy link
Member Author

cc @lumag

@lumag
Copy link
Contributor

lumag commented Apr 10, 2023

Ack, LGTM.

Copy link
Member

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

👍

@tarcieri tarcieri merged commit bf1defd into master Apr 11, 2023
@tarcieri tarcieri deleted the pkcs1v15/prefixed-signatures-by-default branch April 11, 2023 12:37
tarcieri added a commit that referenced this pull request Apr 14, 2023
Similar to #290, this renames the following:

- `Pss::new` => `Pss::new_unsalted`
- `Pss::new_with_salt` => `Pss:new`
- `Pss::new_blinded` => `Pss::new_blinded_unsalted`
- `Pss::new_blinded_with_salt` => `Pss::new_blinded`
- `SigningKey::new` => `SigningKey::new_unsalted`
- `SigningKey::new_with_salt_len` => `SigningKey::new`
- `SigningKey::random` => `SigningKey::random_unsalted`
- `SigningKey::random_with_salt_len` => `SigningKey::random`

The `*_with_salt` methods are preserved with a deprecation warning,
which should help people migrate to the new versions.

This also removes the `From<RsaPrivateKey>` impl for `SigningKey`, since
users should consider up front whether or not they need a salt rather
than defaulting to unsalted.
tarcieri added a commit that referenced this pull request Apr 15, 2023
Following #290, which amended `pkcs1v15::SigningKey`, this commit makes
a corresponding change to `Pkcs1v15Sign` so the method name is
consistent with `SigningKey::new_unprefixed`
tarcieri added a commit that referenced this pull request Apr 17, 2023
Following #290, which amended `pkcs1v15::SigningKey`, this commit makes
a corresponding change to `Pkcs1v15Sign` so the method name is
consistent with `SigningKey::new_unprefixed`
@tarcieri tarcieri mentioned this pull request Apr 27, 2023
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.

Confusion over pkcs1v15::VerifyingKey::new vs ::new_with_prefix
3 participants