-
-
Notifications
You must be signed in to change notification settings - Fork 751
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
Password callbacks #410
Password callbacks #410
Conversation
/// The callback will be passed the password buffer and should return the number of characters | ||
/// placed into the buffer. | ||
pub fn private_key_from_pem_cb<R, F>(reader: &mut R, pass_cb: F) -> Result<PKey, SslError> | ||
where R: Read, F: FnMut(&mut [i8]) -> usize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c_char isn't always signed - maybe just use a &mut [c_char]
instead here?
This looks pretty nice! catch_unwind and resume_unwind weren't stabilized until Rust 1.9 so I think we might want to put these behind a Cargo features for now - I like lagging behind by one release to give people time to upgrade. |
Okay, I think everything is fine now. |
@@ -277,4 +303,22 @@ mod test { | |||
|
|||
assert!(result); | |||
} | |||
|
|||
#[test] | |||
pub fn test_password() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be flagged under the catch_unwind feature - I fixed the travis builds so if you rebase onto master it should be ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
And fix an unused variable warning
/// is encrypted. | ||
/// | ||
/// The callback will be passed the password buffer and should return the number of characters | ||
/// placed into the buffer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs for these methods should mention that they require the catch_unwind
feature.
Made two more comments, and I think we're good then. |
Great! Done. |
Ping @sfackler |
Oops, thanks! |
Tests are missing (both manual and automated), but I wanted to get this ready for an initial review, since I'm really not sure how to implement these callbacks safely (but I think I like this pattern).