blaskovicz
feat(key): pad keys up to the nearest 16/24/32 byte
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.
Pull Request Test Coverage Report for Build 30
- 45 of 54 (83.33%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage decreased (-2.4%) to 88.398%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| cryptkeeper.go | 45 | 54 | 83.33% |
| <!-- | Total: | 45 | 54 |
| Totals | |
|---|---|
| Change from base Build 28: | -2.4% |
| Covered Lines: | 160 |
| Relevant Lines: | 181 |
💛 - Coveralls
@Inphi how about we add another variable with getters/setters (similar to cryptKeeperKey) called keyPaddingMode; that way, we can say keyPaddingMode = PaddingModePKS7, GetKeyPaddingMode, etc which will allow us to add / support other modes in the future? Thoughts?
@Inphi how about we add another variable with getters/setters (similar to
cryptKeeperKey) calledkeyPaddingMode; that way, we can saykeyPaddingMode = PaddingModePKS7,GetKeyPaddingMode, etc which will allow us to add / support other modes in the future? Thoughts?
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; Encrypt(stuff) and Decrypt(cipherStuff).
Requiring the user to also specify the padding mode (in the case where the non-default padding mode is required) seems contra to simplicity.
But perhaps the complexity is worth it. If you think it's a good idea then I can add that in too.
@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!
@Inphi it's free so it probably gets overloaded once and a while. I can try to re-trigger it.