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

Reading secret from a file #557

Open
cecton opened this issue Nov 20, 2020 · 8 comments
Open

Reading secret from a file #557

cecton opened this issue Nov 20, 2020 · 8 comments

Comments

@cecton
Copy link

cecton commented Nov 20, 2020

I was wondering if this code could potentially leave a copy of a secret in memory:

let password = SecretString::new(fs::read_to_string(file_path)?);

Because the underlying implementation of read_to_string might re-allocate the String and not zeroes the partially-read secret.

Do you have any suggestion?

I would be happy to provide a PR that convert an impl Read to a SecretString that would zeroes things between reads if you think that is a good implementation.

@tony-iqlusion
Copy link
Member

@cecton being able to read from a file directly into a SecretString would definitely be interesting!

@c-git
Copy link
Contributor

c-git commented Aug 15, 2022

@cecton did you ever implement the reading in a secret from a file?

@cecton
Copy link
Author

cecton commented Aug 15, 2022

@c-git sorry I totally forgot about this ticket, Feel free to go ahead with the implementation if you would likd

@c-git
Copy link
Contributor

c-git commented Aug 15, 2022

Ok thanks. If I do I'll submit it but I'm not quite sure how to securely accomplish that task.

@cecton
Copy link
Author

cecton commented Aug 15, 2022

tbh I'm not sure either, My colleagues back in the days mentioned something special to do but I forgot what exactly... @tomaka maybe you can help with some advice here? (I think it was you)

@cecton
Copy link
Author

cecton commented Aug 15, 2022

I would be happy to provide a PR that convert an impl Read to a SecretString that would zeroes things between reads if you think that is a good implementation.

oh wait, probably that was a not so innocent suggestion, I think the idea was to copy the code of fs::read_to_string from std and do something special like zeroing the buffer before the exit.

@c-git
Copy link
Contributor

c-git commented Aug 15, 2022

Thanks that looks like something I might be able to follow up on.

@c-git
Copy link
Contributor

c-git commented Aug 15, 2022

Disclaimer I'm new to Rust (but not programing). So if someone can look over what I did that would be great. (I used PyCharm to follow the code from one point to the next in the standard library and it looks like it was navigating correctly to me. Was able to match it in the online github repo)

Didn't seem possible for a full copy to be left in a buffer somewhere in memory

That said I think no separate buffers are used other than the one created to store the string in the first call. And this owned string is moved into the Secret wrapper. There was one point in the code here where it looked like another buffer was allocated.

code screen shot

However when I checked what a ReadBuf was it turns out it's just a pointer to part of an already existing buffer (the one created earlier). That is based on the comments that I found here.

Screen shot of supporting comment

Partial copies seemed possible (but under what seemed like unlikely situations)

I did see two ways that part of the file could be loaded into memory and then not cleared. I'm not sure of the viability of either approach but from reading the comments and following the code logic I was lead to believe that it might be possible.

  1. If when allocating the reserved space in the string here if the size of the file was not able to be determined so that the required space was not allocated then it is possible that the Vec will need to grow during the reading process and the old layouts could contain partial amounts of the characters from the file.
  2. If an error occurs during the file read then the string loaded into memory thus far might not be zeroed out when the function returns.

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