Store password in secure type like secstr::SecUtf8

[mverleg]

Would it be a good idea to return the password wrapped in data type meant for that purpose? Possibly as an optional feature.

Advantages include being unprintable, zero-ing when dropped, and other things mentioned at secstr.

(It could be any type, I'm not affiliated with secstr, it's just the only one I know.)

[conradkleinespel]

@mverleg Thanks for the suggestion! Here's what I think of secstr strong points:

• constant time comparison: not needed, there is no comparison in rpassword
• automatically zeroing out in the destructor: @tov just added that, so that's fixed :tada:
• mlock protection if possible: it's sadly not cross platform but would be great to have on unix
• stripping the password from logs to prevent leaks: great idea
• using libsodium: no, I don't want to force everyone to have a dependency on libsodium
• de/serializable into anything Serde supports as a byte string: sure, why not !

The code from secstr for things that I'm open to (mlock, no log leaks, serde) is just a few lines, so it would great if we can copy paste them into the struct that @tov created.

I don't want to add secstr as a dependency, because as a dev, I don't want a password reading library to have dependencies to unknown packages. Currently, rpassword only uses libc and winapi which are both very well known. So yeah, adding secstr is just never gonna happen.

But copying/pasting some of its code would be useful so I'm open to that :+1:

[mverleg]

Yeah I think those are more most useful features.

I guess there's always a balance between having dependencies that could be dangerous, and duplicating code that could get outdated - I'll trust your judgement.

One thing to note though, maybe less related to security: if each crate creates their own type, there needs to be a conversion each time, e.g. getting a password using rpassword and the hashing it and storing it somewhere... Though likely, security trumps convenience.

[mverleg]

Oh I perhaps should have been more clear - my hope was that the library would return the wrapper type so the password wouldn't have to go back to a string. I see ZeroOnDrop is used internally only.

[conradkleinespel]

@mverleg Returning ZeroOnDrop would break all existing code, it's a breaking change. And if the user uses something other than ZeroOnDrop for whatever reason, than it's of little use. But with the From trait, it seems to me like going from String to whatever type you want can be trivial throughout a codebase. Here's an example:

struct Test {
    inner: String
}

impl From<String> for Test {
    fn from(t: String) -> Test {
        Test { inner: t }
    }
}

fn test_fn_with_arg(arg: Test) {
    println!("{}", arg.inner);
}

fn main() {
    let str_a: &'static str = "hello";
    let test_val_a: Test = String::from(str_a).into();

    let str_b: &'static str = "world";
    let test_val_b = Test::from(String::from(str_b));

    println!(
        "{} {}",
        test_val_a.inner,
        test_val_b.inner
        );

    test_fn_with_arg(Test { inner: str_a.to_owned() });
    test_fn_with_arg(str_b.to_owned().into());
}

[DDoSolitary]

mlock protection if possible: it's sadly not cross platform but would be great to have on unix

Windows has a similar function called VirtualLock which is already available in the winapi crate

[conradkleinespel]

This issue seems stale. If at any point anyone wants to create a pull request for that, feel free to do so.