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

pss: make *_with_salt methods the default #291

Closed
wants to merge 1 commit into from

Conversation

tarcieri
Copy link
Member

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.

cc @lumag

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.
@lumag
Copy link
Contributor

lumag commented Apr 14, 2023

Generic comment: do we need the without salt functions at all? Let me suggest another approach. Basically even if salt_length is None the code will use some default for it (I can not trace an origin of the default).

@tarcieri
Copy link
Member Author

I don't actually know if anyone uses unsalted PSS.

Indeed It seems weird to have a separate API for it when someone who really wants it can specify a salt length of 0.

Perhaps we should just get rid of the "unsalted" functions to simplify the API?

@lumag
Copy link
Contributor

lumag commented Apr 15, 2023

Yes, that's my idea. Will post it shortly.

@tarcieri
Copy link
Member Author

I can just remove the _unsalted methods from this PR?

@lumag
Copy link
Contributor

lumag commented Apr 15, 2023

@tarcieri slightly more than that (especially on the verification side).

@lumag
Copy link
Contributor

lumag commented Apr 15, 2023

@tarcieri done, #294

@tarcieri
Copy link
Member Author

I like #294 as an alternative to this, but I'll leave this open for now until people have a chance to discuss

@tarcieri
Copy link
Member Author

Closed in favor of #294

@tarcieri tarcieri closed this Apr 17, 2023
@tarcieri tarcieri deleted the pss/salt-by-default branch April 17, 2023 00:05
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