Shred CredentialPayload upon approval/rejection #24884
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously running securezero! on credentials in the reject(::CredentialCache, ...) results in the credentials being wiped before reject(::GitConfig, ...) was run.
Note I switched the credential_loop to have a shred keyword as it seems best to default have the function use the same default as the standard LibGit2 credential loop.
omus
added
libgit2
|
cc: @stevengj |
stevengj
approved these changes
Dec 5, 2017
|
Given approval from @stevengj, please merge as appropriate! |
evetion
pushed a commit
to evetion/julia
that referenced
this pull request
Dec 12, 2017
* Zero LibGit2 credential payload in approve/reject Previously running securezero! on credentials in the reject(::CredentialCache, ...) results in the credentials being wiped before reject(::GitConfig, ...) was run. * Always return payload from credential_loop * Add additional shred tests Note I switched the credential_loop to have a shred keyword as it seems best to default have the function use the same default as the standard LibGit2 credential loop. * challenge_prompt shows process output on ProcessError * Display message about expected warning * Code formatting fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As I mentioned in #24731 there were some security issues with
CredentialPayloadwhere credentials were not being zeroed after they were done being used.The changes in this PR ensure that credential information is wiped out during normal use and fixes some minor issues where credentials were being wiped too early when they were rejected. Note that I am specifically not wiping the
explicitandcachefields fromCredentialPayloadas this is credential information being provided from the caller and should be wiped by the caller.Additionally, I addressed some minor details including:
challenge_prompt(assists in debugging)