feat(key): pad keys up to the nearest 16/24/32 byte#9
feat(key): pad keys up to the nearest 16/24/32 byte#9Inphi wants to merge 3 commits intoblaskovicz:masterfrom
Conversation
Pull Request Test Coverage Report for Build 30
💛 - Coveralls |
| } | ||
| key, err := pkcs7Unpad(cryptKeeperKey) | ||
| if err != nil { | ||
| panic("bug") |
There was a problem hiding this comment.
can you put a more descriptive message here like
| panic("bug") | |
| panic(fmt.Errorf("unpad of key failed, this should not happen: %s", err)) |
|
|
||
| // pkcs7pad pads b to the nearest 16, 24 or 32 byte | ||
| func pkcs7Pad(b []byte) ([]byte, error) { | ||
| if len(b) == 0 { |
There was a problem hiding this comment.
can you assign len(b) to a variable for usage in this function
| } | ||
|
|
||
| func pkcs7Unpad(b []byte) ([]byte, error) { | ||
| if len(b) == 0 { |
There was a problem hiding this comment.
same here, can you assign len(b) to a variable
|
@Inphi how about we add another variable with getters/setters (similar to |
| } | ||
|
|
||
| func getBlockSize(n int) int { | ||
| if n <= 16 { |
There was a problem hiding this comment.
is a switch / case statement more concise here?
I considered this but was hesitant to do this because it'll require users to also keep track of the padding algorithm. I presume, a goal of this library, based on its design, is simplicity: You don't need to specify the blocksize or encryption algorithm; only the secret key is required. This enables the sender and receiver to easily pass opaque data around using simple API calls; |
|
@Inphi let's skip the extra complexity for now. i thought more about it and can't see us changing padding algorithm. let's just clean up the other things and then we can get this merged. Also, can you make sure the test coverage doesn't go down? Thanks! |
|
@blaskovicz Any idea what's up with coveralls? |
|
@Inphi it's free so it probably gets overloaded once and a while. I can try to re-trigger it. |
| } | ||
| } else { | ||
| t.Errorf("pkcs7pad failed. Error: %v\n", err) | ||
| } |
There was a problem hiding this comment.
@Inphi can you add an unpad check here as well, then it's good to 🚢
3 participants
via PKCS#7 padding.
I guess the user can do this themselves. So this is more of a convenience.
Caveat: if this is merged, we'll be locked into a given padding algorithm. Any change in padding would require a new incompatible API.