libgit2 credentials objects are dangerous

[Keno]

When they finalize themselves, they zero their contents. In particular, they also zero the underlying memory of any string they contains, so for example, in the following:

function authenticate(pass)
    UserPasswordCredentials("anonymous", pass)
end

after the first such object is finalized, that literal gets zero'd and subsequent calls to the same function are constructed with a zero string. I suggest that instead, we pass Vector{UInt8}s to these objects, which are semantically mutable and people won't be as surprised to see them zerod.

[simonbyrne]

Perhaps a better approach would be to have a SecureString object, which is guaranteed to be zeroed when gc'ed?

[StefanKarpinski]

Yes, that seems like a good approach. It might have other safety properties, like not being so easily copied around or converted to a normal string, to avoid inadvertent copies leaking into memory.

[simonbyrne]

is there prior art for this in other languages? seems like something that should be fairly common

[stevengj]

See also the discussion in #17560, where a SensitiveData type was proposed.

One objection to SecureString is the same as it was there: in applications where a string contains sensitive data, you want it to be zeroed immediately after use rather than waiting for gc.

Another objection is that it's not clear to me what SecureString would provide to the user over a byte array. It would be dangerous to print it (which might leave it somewhere else in memory) and you wouldn't want to construct it with SecureString("foo") or really do any string-like operations on it at all.

My preference would be to just change getpass and similar usages of password strings to Vector{UInt8}, and continue to call Base.securezero! immediately when we are done with the password.

[stevengj]

Another perspective is that the problem @Keno identified may be a feature, not a bug: you shouldn't be putting sensitive password/credential data into program string literals, should you?

[Keno]

The username may or may not be a sensitive credential. In any case zeroing out semantically immutable memory is a bad idea.

[stevengj]

In any case, changing it to Vector{UInt8} seems reasonable to me.

[omus]

I created a PR which implements SecureString. At the moment it still zeros out immutable memory.