Added a definitive check for possible password errors. #121
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
Codecov Report
@@ Coverage Diff @@
## master #121 +/- ##
==========================================
- Coverage 64.61% 64.58% -0.03%
==========================================
Files 24 24
Lines 2063 2067 +4
==========================================
+ Hits 1333 1335 +2
- Misses 580 581 +1
- Partials 150 151 +1
Continue to review full report at Codecov.
|
michelvocks
previously requested changes
Oct 6, 2018
security/vault.go
Outdated
| @@ -190,6 +190,7 @@ func (v *Vault) encrypt(data []byte) (string, error) { | |||
| // User has deleted all the secrets. the file will be empty. | |||
| return "", nil | |||
| } | |||
| data = append(data, []byte("\nGAIA_CHECK=!CHECK_ME!")...) | |||
There was a problem hiding this comment.
That looks a bit magic to me. What do you think about moving this to constant variable?
security/vault.go
Outdated
| @@ -241,6 +242,10 @@ func (v *Vault) decrypt(data []byte) ([]byte, error) { | |||
| if err != nil { | |||
| return []byte{}, err | |||
| } | |||
|
|
|||
| if !bytes.Contains(unpadMsg, []byte("!CHECK_ME!")) { | |||
There was a problem hiding this comment.
Same here. A bit magic. Maybe move that to a separate constant with a tiny description? 😄
security/vault.go
Outdated
| // by the padding process. Here it will be caught because we can't | ||
| // marshal the data into proper k/v pairs. | ||
| return errors.New("possible mistyped password") | ||
| if bytes.Equal(d[0], []byte("GAIA_CHECK")) { |
Skarlso
dismissed
michelvocks’s stale review
Addressed comments, please re-review. :)
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.
This deals with #120.
Instead of doing this bit:
gaia/security/vault.go
Lines 256 to 261 in 413494f
Now there is a definitive way of checking if the decryption was successful or not.