Skip to content
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

WIP: RFC: Create type SecureString #24738

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export
RoundNearestTiesUp,
RoundToZero,
RoundUp,
SecureString,
Set,
Some,
StepRange,
Expand Down Expand Up @@ -592,6 +593,7 @@ export
rpad,
rsplit,
rstrip,
shred!,
split,
string,
strip,
Expand Down
82 changes: 82 additions & 0 deletions base/strings/secure.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
"""
SecureString(string::AbstractString)

A string where the contents will be securely wiped when garbage collected. However, it is
considered best practise to wipe the string using `shred!(::SecureString)` as soon as the
secure data is no longer required. Note that when the parameter is of type `Vector{UInt8}`
then the memory of the passed in parameter will also be securely wiped.

# Examples
```jldoctest
julia> str = "abc"::String
"abc"

julia> s = SecureString(str)
"abc"

julia> shred!(s)
"\0\0\0"

julia> str
"abc"
```
"""
mutable struct SecureString <: AbstractString
data::Vector{UInt8}

function SecureString(data::Vector{UInt8})
s = new(data)
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 print(io::IO, s::SecureString)
write(io, s.data)
nothing
end

String(s::SecureString) = String(copy(s.data))

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]

isvalid(s::SecureString, i::Int) = isvalid(String(s), i)

==(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 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)
return s
end

isshredded(s::SecureString) = sum(s.data) == 0
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want all(s.data .== 0); as there's a (minute) possibility that this can overflow to precisely zero.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or rather all(iszero, s.data).


function shred!(f::Function, x)
try
f(x)
finally
shred!(x)
end
x
end
1 change: 1 addition & 0 deletions base/strings/strings.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ import .Unicode: textwidth, islowercase, isuppercase, isletter, isdigit, isnumer

include("strings/util.jl")
include("strings/io.jl")
include("strings/secure.jl")
10 changes: 6 additions & 4 deletions base/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ securezero!(s::String) = unsafe_securezero!(pointer(s), sizeof(s))
unsafe_securezero!(p::Ptr{Cvoid}, len::Integer=1) = Ptr{Cvoid}(unsafe_securezero!(Ptr{UInt8}(p), len))

if Sys.iswindows()
function getpass(prompt::AbstractString)
function getpass(prompt::AbstractString)::SecureString
print(prompt)
flush(stdout)
p = Vector{UInt8}(undef, 128) # mimic Unix getpass in ignoring more than 128-char passwords
Expand Down Expand Up @@ -475,11 +475,13 @@ function getpass(prompt::AbstractString)
return ""
end
else
getpass(prompt::AbstractString) = unsafe_string(ccall(:getpass, Cstring, (Cstring,), prompt))
function getpass(prompt::AbstractString)::SecureString
unsafe_string(ccall(:getpass, Cstring, (Cstring,), prompt))
end
end

"""
prompt(message; default="", password=false) -> Union{String, Nothing}
prompt(message; default="", password=false) -> Union{AbstractString, 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
Expand Down Expand Up @@ -583,7 +585,7 @@ if Sys.iswindows()
# Done.
passbuf_ = passbuf[1:passlen[]-1]
result = (String(transcode(UInt8, usernamebuf[1:usernamelen[]-1])),
String(transcode(UInt8, passbuf_)))
SecureString(transcode(UInt8, passbuf_)))
securezero!(passbuf_)
securezero!(passbuf)

Expand Down
2 changes: 1 addition & 1 deletion stdlib/LibGit2/src/callbacks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ function authenticate_userpass(libgit2credptr::Ptr{Ptr{Cvoid}}, p::CredentialPay
# 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)
shred!(git_cred)
revised = true

p.use_git_helpers = false
Expand Down
60 changes: 30 additions & 30 deletions stdlib/LibGit2/src/gitcredential.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ 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{String, Nothing}
host::Union{String, Nothing}
path::Union{String, Nothing}
username::Union{String, Nothing}
password::Union{String, Nothing}
protocol::Union{SecureString, Nothing}
host::Union{SecureString, Nothing}
path::Union{SecureString, Nothing}
username::Union{SecureString, Nothing}
password::Union{SecureString, Nothing}
use_http_path::Bool

function GitCredential(
Expand All @@ -20,9 +20,7 @@ mutable struct GitCredential
path::Union{AbstractString, Nothing}=nothing,
username::Union{AbstractString, Nothing}=nothing,
password::Union{AbstractString, Nothing}=nothing)
c = new(protocol, host, path, username, password, true)
finalizer(securezero!, c)
return c
new(protocol, host, path, username, password, true)
end
end

Expand All @@ -32,26 +30,26 @@ end

function GitCredential(cred::UserPasswordCredential, url::AbstractString)
git_cred = parse(GitCredential, url)
git_cred.username = deepcopy(cred.user)
git_cred.password = deepcopy(cred.pass)
git_cred.username = SecureString(cred.user)
git_cred.password = SecureString(cred.pass)
return git_cred
end

function securezero!(cred::GitCredential)
cred.protocol !== nothing && securezero!(cred.protocol)
cred.host !== nothing && securezero!(cred.host)
cred.path !== nothing && securezero!(cred.path)
cred.username !== nothing && securezero!(cred.username)
cred.password !== nothing && securezero!(cred.password)
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.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.protocol == b.protocol &&
a.host == b.host &&
a.path == b.path &&
a.username == b.username &&
a.password == b.password &&
a.use_http_path == b.use_http_path
end

Expand Down Expand Up @@ -90,12 +88,11 @@ function Base.parse(::Type{GitCredential}, url::AbstractString)
end

function Base.copy!(a::GitCredential, b::GitCredential)
# Note: deepcopy calls avoid issues with securezero!
a.protocol = deepcopy(b.protocol)
a.host = deepcopy(b.host)
a.path = deepcopy(b.path)
a.username = deepcopy(b.username)
a.password = deepcopy(b.password)
a.protocol = b.protocol
a.host = b.host
a.path = b.path
a.username = b.username
a.password = b.password
return a
end

Expand All @@ -116,9 +113,12 @@ function Base.read!(io::IO, cred::GitCredential)
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
Base.setproperty!(cred, Symbol(key), String(value))
field = getproperty(cred, Symbol(key))
field !== nothing && shred!(field)
setproperty!(cred, Symbol(key), String(value))
end
end

Expand Down Expand Up @@ -283,7 +283,7 @@ function approve(cfg::GitConfig, cred::UserPasswordCredential, url::AbstractStri
approve(helper, git_cred)
end

securezero!(git_cred)
shred!(git_cred)
nothing
end

Expand All @@ -295,6 +295,6 @@ function reject(cfg::GitConfig, cred::UserPasswordCredential, url::AbstractStrin
reject(helper, git_cred)
end

securezero!(git_cred)
shred!(git_cred)
nothing
end
48 changes: 22 additions & 26 deletions stdlib/LibGit2/src/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,7 @@ function objtype(obj_type::Consts.OBJECT)
end
end

import Base.securezero!
import Base.shred!

abstract type AbstractCredential end

Expand All @@ -1185,12 +1185,10 @@ isfilled(::AbstractCredential)

"Credential that support only `user` and `password` parameters"
mutable struct UserPasswordCredential <: AbstractCredential
user::String
pass::String
user::SecureString
pass::SecureString
function UserPasswordCredential(user::AbstractString="", pass::AbstractString="")
c = new(user, pass)
finalizer(securezero!, c)
return c
new(user, pass)
end

# Deprecated constructors
Expand All @@ -1204,9 +1202,9 @@ mutable struct UserPasswordCredential <: AbstractCredential
UserPasswordCredential(prompt_if_incorrect::Bool) = UserPasswordCredential("","",prompt_if_incorrect)
end

function securezero!(cred::UserPasswordCredential)
securezero!(cred.user)
securezero!(cred.pass)
function shred!(cred::UserPasswordCredential)
shred!(cred.user)
shred!(cred.pass)
return cred
end

Expand All @@ -1220,15 +1218,13 @@ end

"SSH credential type"
mutable struct SSHCredential <: AbstractCredential
user::String
pass::String
prvkey::String
pubkey::String
user::SecureString
pass::SecureString
prvkey::SecureString
pubkey::SecureString
function SSHCredential(user::AbstractString="", pass::AbstractString="",
prvkey::AbstractString="", pubkey::AbstractString="")
c = new(user, pass, prvkey, pubkey)
finalizer(securezero!, c)
return c
prvkey::AbstractString="", pubkey::AbstractString="")
new(user, pass, prvkey, pubkey)
end

# Deprecated constructors
Expand All @@ -1243,11 +1239,11 @@ mutable struct SSHCredential <: AbstractCredential
SSHCredential(prompt_if_incorrect::Bool) = SSHCredential("","","","",prompt_if_incorrect)
end

function securezero!(cred::SSHCredential)
securezero!(cred.user)
securezero!(cred.pass)
securezero!(cred.prvkey)
securezero!(cred.pubkey)
function shred!(cred::SSHCredential)
shred!(cred.user)
shred!(cred.pass)
shred!(cred.prvkey)
shred!(cred.pubkey)
return cred
end

Expand All @@ -1270,8 +1266,8 @@ Base.haskey(cache::CachedCredentials, cred_id) = Base.haskey(cache.cred, cred_id
Base.getindex(cache::CachedCredentials, cred_id) = Base.getindex(cache.cred, cred_id)
Base.get!(cache::CachedCredentials, cred_id, default) = Base.get!(cache.cred, cred_id, default)

function securezero!(p::CachedCredentials)
foreach(securezero!, values(p.cred))
function shred!(p::CachedCredentials)
foreach(shred!, values(p.cred))
return p
end

Expand Down Expand Up @@ -1385,7 +1381,7 @@ function approve(p::CredentialPayload; shred::Bool=true)
approve(p.config, cred, p.url)
end

shred && securezero!(cred)
shred && shred!(cred)
nothing
end

Expand All @@ -1410,7 +1406,7 @@ function reject(p::CredentialPayload; shred::Bool=true)
reject(p.config, cred, p.url)
end

shred && securezero!(cred)
shred && shred!(cred)
nothing
end

Expand Down
Loading