Switch GitCredential equality to use ==

Originally used `isequal` to deal with `Nullable`

base/strings/secure.jl

@@ -21,56 +21,108 @@ julia> str
 "abc"
 ```
 """
-mutable struct SecureString <: AbstractString
+mutable struct SecureString <: IO
     data::Vector{UInt8}
+    size::Int
+    ptr::Int
 
-    function SecureString(data::Vector{UInt8})
-        s = new(data)
+    function SecureString(; sizehint=128)
+        s = new(Vector{UInt8}(undef, sizehint), 0, 1)
         finalizer(final_shred!, s)
         return s
     end
 end
 
-SecureString(str::String) = SecureString(copy(unsafe_wrap(Vector{UInt8}, str)))
-SecureString(str::Union{AbstractString,CodeUnits}) = SecureString(String(str))
-
-show(io::IO, s::SecureString) = print(io, "SecureString(\"*****\")")
-write(io::IO, s::SecureString) = write(io, s.data)
+function SecureString!(p::Ptr{UInt8})
+    # Copy into our own Vector and zero the source
+    len = ccall(:strlen, Cint, (Ptr{UInt8},), p)
+    s = SecureString(sizehint=len)
+    for i in 1:len
+        write(s, unsafe_load(p, i))
+    end
+    seek(s, 0)
+    s
+end
 
-function print(io::IO, s::SecureString)
-    write(io, s.data)
-    nothing
+convert(::Type{SecureString}, s::AbstractString) = SecureString(String(s))
+SecureString(str::AbstractString) = SecureString(String(str))
+function SecureString(str::String)
+    buf = unsafe_wrap(Vector{UInt8}, str)
+    s = SecureString(sizehint=length(buf))
+    for c in buf
+        write(s, c)
+    end
+    seek(s, 0)
+    s
 end
 
-String(s::SecureString) = String(copy(s.data))
+show(io::IO, s::SecureString) = print(io, "SecureString(\"*******\")")
 
-iterate(s::SecureString, i::Integer=firstindex(s)) = iterate(String(s), i)
-ncodeunits(s::SecureString) = Core.sizeof(s.data)
-codeunit(s::SecureString) = UInt8
-codeunit(s::SecureString, i::Integer) = s.data[i]
+hash(s::SecureString, h::UInt) = hash(SecureString, hash((s.data, s.size, s.ptr), h))
+==(s1::SecureString, s2::SecureString) = (view(s1.data, 1:s1.size), s1.ptr) == (view(s2.data, 1:s2.size), s2.ptr)
 
-isvalid(s::SecureString, i::Int) = isvalid(String(s), i)
+function write(io::SecureString, 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
 
-==(a::SecureString, b::SecureString) = a.data == b.data
-==(a::String, b::SecureString) = unsafe_wrap(Vector{UInt8}, a) == b.data
-==(a::AbstractString, b::SecureString) = String(a) == b
-==(a::SecureString, b::AbstractString) = b == a
+function write(io::IO, s::SecureString)
+    nb = 0
+    for i in 1:s.size
+        nb += write(io, s.data[i])
+    end
+    return nb
+end
+
+function unsafe_convert(::Type{Ptr{UInt8}}, s::SecureString)
+    # 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 unsafe_convert(Ptr{UInt8}, s.data)
+end
+
+seek(io::SecureString, n::Integer) = (io.ptr = max(min(n+1, io.size+1), 1))
+bytesavailable(io::SecureString) = io.size - io.ptr + 1
+position(io::SecureString) = io.ptr-1
+eof(io::SecureString) = io.ptr > io.size
+isempty(io::SecureString) = io.size == 0
+function peek(io::SecureString)
+    eof(io) && throw(EOFError())
+    return io.data[io.ptr]
+end
+function read(io::SecureString, ::Type{UInt8})
+    eof(io) && throw(EOFError())
+    byte = io.data[io.ptr]
+    io.ptr += 1
+    return byte
+end
 
 function final_shred!(s::SecureString)
     if !isshredded(s)
-        # TODO: Printing SecureString data is temporary while I locate issues in tests
-        # Note: `@warn` won't work here
-        println("SecureString \"$s\" not explicitly shredded")
         shred!(s)
     end
 end
 
 function shred!(s::SecureString)
     securezero!(s.data)
+    s.ptr = 1
+    s.size = 0
     return s
 end
 
-isshredded(s::SecureString) = sum(s.data) == 0
+isshredded(s::SecureString) = all(iszero, s.data)
 
 function shred!(f::Function, x)
     try

base/util.jl

@@ -441,42 +441,34 @@ will always be called.
 """
 function securezero! end
 @noinline securezero!(a::AbstractArray{<:Number}) = fill!(a, 0)
-securezero!(s::String) = unsafe_securezero!(pointer(s), sizeof(s))
+securezero!(s::Union{String,Cstring}) = 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))
 
 if Sys.iswindows()
-function getpass(prompt::AbstractString)::SecureString
+function getpass(prompt::AbstractString)
     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 = SecureString()
+    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
+            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
-function getpass(prompt::AbstractString)::SecureString
-    unsafe_string(ccall(:getpass, Cstring, (Cstring,), prompt))
+function getpass(prompt::AbstractString)
+    SecureString(ccall(:getpass, Cstring, (Cstring,), prompt))
 end
 end
 

stdlib/LibGit2/src/callbacks.jl

@@ -169,7 +169,7 @@ function authenticate_ssh(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPayload,
     end
 
     return ccall((:git_cred_ssh_key_new, :libgit2), Cint,
-                 (Ptr{Ptr{Cvoid}}, Cstring, Cstring, Cstring, Cstring),
+                 (Ptr{Ptr{Cvoid}}, Cstring, Cstring, Cstring, Ptr{UInt8}),
                  libgit2credptr, cred.user, cred.pubkey, cred.prvkey, cred.pass)
 end
 
@@ -187,9 +187,8 @@ 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, ""))
+        cred.user = something(git_cred.username, "")
+        cred.pass = something(git_cred.password, "")
         shred!(git_cred)
         revised = true
 
@@ -228,7 +227,7 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPay
     end
 
     return ccall((:git_cred_userpass_plaintext_new, :libgit2), Cint,
-                 (Ptr{Ptr{Cvoid}}, Cstring, Cstring),
+                 (Ptr{Ptr{Cvoid}}, Cstring, Ptr{UInt8}),
                  libgit2credptr, cred.user, cred.pass)
 end
 

stdlib/LibGit2/src/gitcredential.jl

@@ -7,10 +7,10 @@ Git credential information used in communication with git credential helpers. Th
 named using the [input/output key specification](https://git-scm.com/docs/git-credential#IOFMT).
 """
 mutable struct GitCredential
-    protocol::Union{SecureString, Nothing}
-    host::Union{SecureString, Nothing}
-    path::Union{SecureString, Nothing}
-    username::Union{SecureString, Nothing}
+    protocol::Union{AbstractString, Nothing}
+    host::Union{AbstractString, Nothing}
+    path::Union{AbstractString, Nothing}
+    username::Union{AbstractString, Nothing}
     password::Union{SecureString, Nothing}
     use_http_path::Bool
 
@@ -28,30 +28,21 @@ function GitCredential(cfg::GitConfig, url::AbstractString)
     fill!(cfg, parse(GitCredential, url))
 end
 
-function GitCredential(cred::UserPasswordCredential, url::AbstractString)
-    git_cred = parse(GitCredential, url)
-    git_cred.username = SecureString(cred.user)
-    git_cred.password = SecureString(cred.pass)
-    return git_cred
-end
+GitCredential(cred::UserPasswordCredential, url::AbstractString) = parse(GitCredential, url)
+
+Base.:(==)(c1::GitCredential, c2::GitCredential) = (c1.protocol, c1.host, c1.path, c1.username, c1.password, c1.use_http_path) ==
+                                                   (c2.protocol, c2.host, c2.path, c2.username, c2.password, c2.use_http_path)
+Base.hash(cred::GitCredential, h::UInt) = hash(GitCredential, hash((cred.protocol, cred.host, cred.path, cred.username, cred.password, cred.use_http_path), h))
 
 function shred!(cred::GitCredential)
-    cred.protocol !== nothing && shred!(cred.protocol)
-    cred.host !== nothing && shred!(cred.host)
-    cred.path !== nothing && shred!(cred.path)
-    cred.username !== nothing && shred!(cred.username)
+    cred.protocol !== nothing && (cred.protocol = nothing)
+    cred.host !== nothing && (cred.host = nothing)
+    cred.path !== nothing && (cred.path = nothing)
+    cred.username !== nothing && (cred.username = nothing)
     cred.password !== nothing && shred!(cred.password)
     return cred
 end
 
-function Base.:(==)(a::GitCredential, b::GitCredential)
-    isequal(a.protocol, b.protocol) &&
-    isequal(a.host, b.host) &&
-    isequal(a.path, b.path) &&
-    isequal(a.username, b.username) &&
-    isequal(a.password, b.password) &&
-    a.use_http_path == b.use_http_path
-end
 
 """
     ismatch(url, git_cred) -> Bool
@@ -97,28 +88,36 @@ function Base.copy!(a::GitCredential, b::GitCredential)
 end
 
 function Base.write(io::IO, cred::GitCredential)
-    cred.protocol !== nothing && println(io, "protocol=", cred.protocol)
-    cred.host !== nothing && println(io, "host=", cred.host)
-    cred.path !== nothing && cred.use_http_path && println(io, "path=", cred.path)
-    cred.username !== nothing && println(io, "username=", cred.username)
-    cred.password !== nothing && println(io, "password=", cred.password)
+    cred.protocol !== nothing && write(io, "protocol=", cred.protocol, '\n')
+    cred.host !== nothing && write(io, "host=", cred.host, '\n')
+    cred.path !== nothing && cred.use_http_path && write(io, "path=", cred.path, '\n')
+    cred.username !== nothing && write(io, "username=", cred.username, '\n')
+    cred.password !== nothing && write(io, "password=", cred.password, '\n')
     nothing
 end
 
 function Base.read!(io::IO, cred::GitCredential)
     # https://git-scm.com/docs/git-credential#IOFMT
     while !eof(io)
-        key, value = split(readline(io), '=')
-
+        key = readuntil(io, '=')
+        if key == "password"
+            value = SecureString()
+            while !eof(io) && (c = read(io, UInt8)) != UInt8('\n')
+                write(value, c)
+            end
+            seek(value, 0)
+        else
+            value = readuntil(io, '\n')
+        end
         if key == "url"
             # Any components which are missing from the URL will be set to empty
             # https://git-scm.com/docs/git-credential#git-credential-codeurlcode
             shred!(cred)
             copy!(cred, parse(GitCredential, value))
         else
             field = getproperty(cred, Symbol(key))
-            field !== nothing && shred!(field)
-            setproperty!(cred, Symbol(key), String(value))
+            field !== nothing && Symbol(key) == :password && shred!(field)
+            setproperty!(cred, Symbol(key), value)
         end
     end
 

stdlib/LibGit2/src/types.jl

@@ -1193,9 +1193,9 @@ isfilled(::AbstractCredential)
 
 "Credential that support only `user` and `password` parameters"
 mutable struct UserPasswordCredential <: AbstractCredential
-    user::SecureString
+    user::AbstractString
     pass::SecureString
-    function UserPasswordCredential(user::AbstractString="", pass::AbstractString="")
+    function UserPasswordCredential(user::AbstractString="", pass::Union{AbstractString, SecureString}="")
         new(user, pass)
     end
 
@@ -1211,7 +1211,6 @@ mutable struct UserPasswordCredential <: AbstractCredential
 end
 
 function shred!(cred::UserPasswordCredential)
-    shred!(cred.user)
     shred!(cred.pass)
     return cred
 end
@@ -1226,12 +1225,13 @@ end
 
 "SSH credential type"
 mutable struct SSHCredential <: AbstractCredential
-    user::SecureString
+    user::AbstractString
     pass::SecureString
-    prvkey::SecureString
-    pubkey::SecureString
-    function SSHCredential(user::AbstractString="", pass::AbstractString="",
-                           prvkey::AbstractString="", pubkey::AbstractString="")
+    # Paths to private keys
+    prvkey::AbstractString
+    pubkey::AbstractString
+    function SSHCredential(user="", pass="",
+                           prvkey="", pubkey="")
         new(user, pass, prvkey, pubkey)
     end
 
@@ -1248,10 +1248,7 @@ mutable struct SSHCredential <: AbstractCredential
 end
 
 function shred!(cred::SSHCredential)
-    shred!(cred.user)
     shred!(cred.pass)
-    shred!(cred.prvkey)
-    shred!(cred.pubkey)
     return cred
 end