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

scrypt: Provide read access to the parameter values #123

Closed
demurgos opened this issue Feb 2, 2021 · 4 comments
Closed

scrypt: Provide read access to the parameter values #123

demurgos opened this issue Feb 2, 2021 · 4 comments

Comments

@demurgos
Copy link

demurgos commented Feb 2, 2021

Hi,
I am currently migrating a Node application to Rust. This application uses scrypt-kdf which is itself based on node-scrypt. The kdf function in these libraries uses a different format than the one provided by this lib. I plan to eventually move to the PHC string format, but my first step is to perform a simple port.

When porting the Node algorithm, I need to write the current parameters into the output buffer. I planned to store my parameters directly as a scrypt::Param instance but this type does not allow to read the current values, preventing me from using it to write the Node format. My current solution is to store these parameters in my own struct, but it creates duplication. This is especially bad when using Params::recommended as I now have to copy the recommended values to my own crate.

Would it be possible to provide read access to the current values of the parameters?

These values are pub(crate) currently. I understand that making them public might not be desirable as it completely breaks encapsulation (a mut scrypt::Params would allow anyone to change its values at any time). Would it then be possible to add getter methods? Something like:

impl Params {
  pub fn r(&self) -> u32 {
    self.r
  }
  // ...
}

pub fn main() {
   let params = Params::recommended();
   let r = params.r();
   // ...
}
@tarcieri
Copy link
Member

tarcieri commented Feb 2, 2021

Sounds good to me.

@newpavlov you okay with the getter method approach?

@newpavlov
Copy link
Member

newpavlov commented Feb 6, 2021

Yes, sounds good.

@tarcieri
Copy link
Member

tarcieri commented Feb 6, 2021

I added accessors in 1bb21b1, but accidentally pushed master instead of opening a PR. Whoops.

Let me know if that looks good.

@tarcieri tarcieri closed this as completed Feb 6, 2021
@tarcieri tarcieri mentioned this issue Feb 6, 2021
@demurgos
Copy link
Author

demurgos commented Feb 7, 2021

Thank you very much 🙂

dns2utf8 pushed a commit to dns2utf8/password-hashes that referenced this issue Jan 24, 2023
…nsts

Re-export `typenum::consts` as `consts`
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

No branches or pull requests

3 participants