RFC: Create SecretBuffer and use it to help keep LibGit2's secrets

base/secretbuffer.jl

@@ -0,0 +1,186 @@
+"""
+    Base.SecretBuffer()
+
+An IOBuffer-like object where the contents will be securely wiped when garbage collected.
+
+It is considered best practice to wipe the buffer using `Base.shred!(::SecretBuffer)` as
+soon as the secure data are no longer required. When initializing with existing data, the
+`SecretBuffer!` method is highly recommended to securely zero the passed argument. Avoid
+initializing with and converting to `String`s as they are unable to be securely zeroed.
+
+# Examples
+```jldoctest
+julia> s = Base.SecretBuffer()
+SecretBuffer("*******")
+
+julia> write(s, 's', 'e', 'c', 'r', 'e', 't')
+6
+
+julia> seek(s, 0); Char(read(s, UInt8))
+'s': ASCII/Unicode U+0073 (category Ll: Letter, lowercase)
+
+julia> Base.shred!(s)
+SecretBuffer("*******")
+
+julia> eof(s)
+true
+```
+"""
+mutable struct SecretBuffer <: IO
+    data::Vector{UInt8}
+    size::Int
+    ptr::Int
+
+    function SecretBuffer(; sizehint=128)
+        s = new(Vector{UInt8}(undef, sizehint), 0, 1)
+        finalizer(final_shred!, s)
+        return s
+    end
+end
+
+"""
+    SecretBuffer(str::AbstractString)
+
+A convenience constructor to initialize a `SecretBuffer` from a non-secret string.
+
+Strings are bad at keeping secrets because they are unable to be securely
+zeroed or destroyed. Therefore, avoid using this constructor with secret data.
+Instead of starting with a string, either construct the `SecretBuffer`
+incrementally with `SecretBuffer()` and `write`, or use a `Vector{UInt8}` with
+the `Base.SecretBuffer!(::Vector{UInt8})` constructor.
+"""
+SecretBuffer(str::AbstractString) = SecretBuffer(String(str))
+function SecretBuffer(str::String)
+    buf = codeunits(str)
+    s = SecretBuffer(sizehint=length(buf))
+    for c in buf
+        write(s, c)
+    end
+    seek(s, 0)
+    s
+end
+convert(::Type{SecretBuffer}, s::AbstractString) = SecretBuffer(String(s))
+
+"""
+    SecretBuffer!(data::Vector{UInt8})
+
+Initialize a new `SecretBuffer` from `data`, securely zeroing `data` afterwards.
+"""
+function SecretBuffer!(d::Vector{UInt8})
+    len = length(d)
+    s = SecretBuffer(sizehint=len)
+    for i in 1:len
+        write(s, d[i])
+    end
+    seek(s, 0)
+    securezero!(d)
+    s
+end
+
+unsafe_SecretBuffer!(s::Cstring) = unsafe_SecretBuffer!(convert(Ptr{UInt8}, s), ccall(:strlen, Cint, (Cstring,), s))
+function unsafe_SecretBuffer!(p::Ptr{UInt8}, len=1)
+    s = SecretBuffer(sizehint=len)
+    for i in 1:len
+        write(s, unsafe_load(p, i))
+    end
+    seek(s, 0)
+    unsafe_securezero!(p, len)
+    s
+end
+
+
+show(io::IO, s::SecretBuffer) = print(io, "SecretBuffer(\"*******\")")
+
+# Unlike other IO objects, equality is computed by value for convenience
+==(s1::SecretBuffer, s2::SecretBuffer) = (s1.ptr == s2.ptr) && (s1.size == s2.size) && (UInt8(0) == _bufcmp(s1.data, s2.data, min(s1.size, s2.size)))
+# Also attempt a constant time buffer comparison algorithm — the length of the secret might be
+# inferred by a timing attack, but not its values.
+@noinline function _bufcmp(data1::Vector{UInt8}, data2::Vector{UInt8}, sz::Int)
+    res = UInt8(0)
+    for i = 1:sz
+        res |= xor(data1[i], data2[i])
+    end
+    return res
+end
+# All SecretBuffers hash the same to avoid leaking information or breaking consistency with ==
+const _sb_hash = UInt === UInt32 ? 0x111c0925 : 0xb06061e370557428
+hash(s::SecretBuffer, h::UInt) = hash(_sb_hash, h)
+
+
+function write(io::SecretBuffer, b::UInt8)
+    if io.ptr > length(io.data)
+        # We need to resize! the array: do this manually to ensure no copies are left behind
+        newdata = Vector{UInt8}(undef, (io.size+16)*2)
+        copyto!(newdata, io.data)
+        securezero!(io.data)
+        io.data = newdata
+    end
+    io.size == io.ptr-1 && (io.size += 1)
+    io.data[io.ptr] = b
+    io.ptr += 1
+    return 1
+end
+
+function write(io::IO, s::SecretBuffer)
+    nb = 0
+    for i in 1:s.size
+        nb += write(io, s.data[i])
+    end
+    return nb
+end
+
+cconvert(::Type{Cstring}, s::SecretBuffer) = unsafe_convert(Cstring, s)
+function unsafe_convert(::Type{Cstring}, s::SecretBuffer)
+    # Ensure that no nuls appear in the valid region
+    if any(==(0x00), s.data[i] for i in 1:s.size)
+        throw(ArgumentError("`SecretBuffers` containing nul bytes cannot be converted to C strings"))
+    end
+    # Add a hidden nul byte just past the end of the valid region
+    p = s.ptr
+    s.ptr = s.size + 1
+    write(s, '\0')
+    s.ptr = p
+    s.size -= 1
+    return Cstring(unsafe_convert(Ptr{Cchar}, s.data))
+end
+
+seek(io::SecretBuffer, n::Integer) = (io.ptr = max(min(n+1, io.size+1), 1); io)
+seekend(io::SecretBuffer) = seek(io, io.size+1)
+skip(io::SecretBuffer, n::Integer) = seek(io, position(io) + n)
+
+bytesavailable(io::SecretBuffer) = io.size - io.ptr + 1
+position(io::SecretBuffer) = io.ptr-1
+eof(io::SecretBuffer) = io.ptr > io.size
+isempty(io::SecretBuffer) = io.size == 0
+function peek(io::SecretBuffer)
+    eof(io) && throw(EOFError())
+    return io.data[io.ptr]
+end
+function read(io::SecretBuffer, ::Type{UInt8})
+    eof(io) && throw(EOFError())
+    byte = io.data[io.ptr]
+    io.ptr += 1
+    return byte
+end
+
+function final_shred!(s::SecretBuffer)
+    !isshredded(s) && @warn("a SecretBuffer was `shred!`ed by the GC; use `shred!` manually after use to minimize exposure.")
+    shred!(s)
+end
+
+function shred!(s::SecretBuffer)
+    securezero!(s.data)
+    s.ptr = 1
+    s.size = 0
+    return s
+end
+
+isshredded(s::SecretBuffer) = all(iszero, s.data)
+
+function shred!(f::Function, x)
+    try
+        f(x)
+    finally
+        shred!(x)
+    end
+end

base/sysimg.jl

@@ -333,6 +333,7 @@ using .Filesystem
 include("process.jl")
 include("grisu/grisu.jl")
 include("methodshow.jl")
+include("secretbuffer.jl")
 
 # core math functions
 include("floatfuncs.jl")

base/util.jl

@@ -441,69 +441,68 @@ will always be called.
 """
 function securezero! end
 @noinline securezero!(a::AbstractArray{<:Number}) = fill!(a, 0)
-securezero!(s::String) = unsafe_securezero!(pointer(s), sizeof(s))
 @noinline unsafe_securezero!(p::Ptr{T}, len::Integer=1) where {T} =
     ccall(:memset, Ptr{T}, (Ptr{T}, Cint, Csize_t), p, 0, len*sizeof(T))
 unsafe_securezero!(p::Ptr{Cvoid}, len::Integer=1) = Ptr{Cvoid}(unsafe_securezero!(Ptr{UInt8}(p), len))
 
+"""
+    Base.getpass(message::AbstractString) -> `Base.SecretBuffer`
+
+Display a message and wait for the user to input a secret, returning an `IO`
+object containing the secret.
+
+Note that on Windows, the secret might be displayed as it is typed; see
+`Base.winprompt` for securely retrieving username/password pairs from a
+graphical interface.
+"""
+function getpass end
+
 if Sys.iswindows()
 function getpass(prompt::AbstractString)
-    print(prompt)
+    print(prompt, ": ")
     flush(stdout)
-    p = Vector{UInt8}(undef, 128) # mimic Unix getpass in ignoring more than 128-char passwords
-                          # (also avoids any potential memory copies arising from push!)
-    try
-        plen = 0
-        while true
-            c = ccall(:_getch, UInt8, ())
-            if c == 0xff || c == UInt8('\n') || c == UInt8('\r')
-                break # EOF or return
-            elseif c == 0x00 || c == 0xe0
-                ccall(:_getch, UInt8, ()) # ignore function/arrow keys
-            elseif c == UInt8('\b') && plen > 0
-                plen -= 1 # delete last character on backspace
-            elseif !iscntrl(Char(c)) && plen < 128
-                p[plen += 1] = c
-            end
+    s = SecretBuffer()
+    plen = 0
+    while true
+        c = UInt8(ccall(:_getch, Cint, ()))
+        if c == 0xff || c == UInt8('\n') || c == UInt8('\r')
+            break # EOF or return
+        elseif c == 0x00 || c == 0xe0
+            ccall(:_getch, Cint, ()) # ignore function/arrow keys
+        elseif c == UInt8('\b') && plen > 0
+            plen -= 1 # delete last character on backspace
+        elseif !iscntrl(Char(c)) && plen < 128
+            write(s, c)
         end
-        return unsafe_string(pointer(p), plen) # use unsafe_string rather than String(p[1:plen])
-                                               # to be absolutely certain we never make an extra copy
-    finally
-        securezero!(p)
     end
-
-    return ""
+    return  s
 end
 else
-getpass(prompt::AbstractString) = unsafe_string(ccall(:getpass, Cstring, (Cstring,), prompt))
+function getpass(prompt::AbstractString)
+    msg = string(prompt, ": ")
+    unsafe_SecretBuffer!(ccall(:getpass, Cstring, (Cstring,), msg))
+end
 end
 
 """
-    prompt(message; default="", password=false) -> Union{String, Nothing}
+    prompt(message; default="") -> Union{String, Nothing}
 
 Displays the `message` then waits for user input. Input is terminated when a newline (\\n)
 is encountered or EOF (^D) character is entered on a blank line. If a `default` is provided
-then the user can enter just a newline character to select the `default`. Alternatively,
-when the `password` keyword is `true` the characters entered by the user will not be
-displayed.
+then the user can enter just a newline character to select the `default`.
+
+See also `Base.getpass` and `Base.winprompt` for secure entry of passwords.
 """
-function prompt(message::AbstractString; default::AbstractString="", password::Bool=false)
-    if Sys.iswindows() && password
-        error("Command line prompt not supported for password entry on windows. Use `Base.winprompt` instead")
-    end
-    msg = !isempty(default) ? "$message [$default]:" : "$message:"
-    if password
-        # `getpass` automatically chomps. We cannot tell an EOF from a '\n'.
-        uinput = getpass(msg)
-    else
-        print(msg)
-        uinput = readline(keep=true)
-        isempty(uinput) && return nothing  # Encountered an EOF
-        uinput = chomp(uinput)
-    end
+function prompt(message::AbstractString; default::AbstractString="")
+    msg = !isempty(default) ? "$message [$default]: " : "$message: "
+    print(msg)
+    uinput = readline(keep=true)
+    isempty(uinput) && return nothing  # Encountered an EOF
+    uinput = chomp(uinput)
     isempty(uinput) ? default : uinput
 end
 
+
 # Windows authentication prompt
 if Sys.iswindows()
     struct CREDUI_INFO
@@ -583,7 +582,7 @@ if Sys.iswindows()
         # Done.
         passbuf_ = passbuf[1:passlen[]-1]
         result = (String(transcode(UInt8, usernamebuf[1:usernamelen[]-1])),
-                  String(transcode(UInt8, passbuf_)))
+                  SecretBuffer!(transcode(UInt8, passbuf_)))
         securezero!(passbuf_)
         securezero!(passbuf)
 

stdlib/LibGit2/src/callbacks.jl

@@ -151,7 +151,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPayload,
                 response === nothing && return user_abort()
                 cred.pass = response[2]
             else
-                response = Base.prompt("Passphrase for $(cred.prvkey)", password=true)
+                response = Base.getpass("Passphrase for $(cred.prvkey)")
                 response === nothing && return user_abort()
                 cred.pass = response
                 isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline
@@ -167,7 +167,6 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPayload,
     if !revised
         return exhausted_abort()
     end
-
     return ccall((:git_cred_ssh_key_new, :libgit2), Cint,
                  (Ptr{Ptr{Cvoid}}, Cstring, Cstring, Cstring, Cstring),
                  libgit2credptr, cred.user, cred.pubkey, cred.prvkey, cred.pass)
@@ -187,10 +186,9 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPay
     if p.use_git_helpers && (!revised || !isfilled(cred))
         git_cred = GitCredential(p.config, p.url)
 
-        # Use `deepcopy` to ensure zeroing the `git_cred` doesn't also zero the `cred`s copy
-        cred.user = deepcopy(something(git_cred.username, ""))
-        cred.pass = deepcopy(something(git_cred.password, ""))
-        securezero!(git_cred)
+        cred.user = something(git_cred.username, "")
+        cred.pass = something(git_cred.password, "")
+        Base.shred!(git_cred)
         revised = true
 
         p.use_git_helpers = false
@@ -211,7 +209,7 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPay
             cred.user = response
 
             url = git_url(scheme=p.scheme, host=p.host, username=cred.user)
-            response = Base.prompt("Password for '$url'", password=true)
+            response = Base.getpass("Password for '$url'")
             response === nothing && return user_abort()
             cred.pass = response
             isempty(cred.pass) && return user_abort()  # Ambiguous if EOF or newline