Add support for PBES2 Key algorithms #190
Conversation
Adds functionality to support the optional key algorithms: * PBES2-HS256+A128KW * PBES2-HS384+A192KW * PBES2-HS512+A256KW
|
|
|
Hi @maraino! Thanks for your pull request! I finally have time to review this, will take a look now. |
crypter.go
Outdated
| @@ -108,6 +108,8 @@ type Recipient struct { | |||
| Algorithm KeyAlgorithm | |||
| Key interface{} | |||
| KeyID string | |||
| P2C int | |||
| P2S []byte | |||
There was a problem hiding this comment.
Can we change the names of these fields to make it clear what they do? Or, alternatively, add doc comments.
There was a problem hiding this comment.
Alternative names could be PBES2Count and PBES2Salt, or just Count and Salt. But I rather add docs and have the field names match better the data written in the header.
There was a problem hiding this comment.
I like PBES2Count and PBES2Salt. Since this struct doesn't directly represent a header I think it's ok to to have names not match the header fields.
| func (parsed rawHeader) getP2C() (int, error) { | ||
| v := parsed[headerP2C] | ||
| if v == nil { | ||
| return 0, nil |
There was a problem hiding this comment.
I think this should probably return an error here.
There was a problem hiding this comment.
I was following the behavior of similar methods on the rawHeader type, getString(), getByteBuffer(), getJWK(), ...
|
This looks pretty good! It looks like they key is still passed in as |
@csstaub: that makes sense. I can accept both string and []byte |
|
@csstaub: just pushed the changes requested. For consistency with other methods, I haven't change the |
|
@csstaub: any update on this? |
|
@maraino LGTM, thanks for your contribution! If you would like for me to tag/release a new version with this included let me know. |
@csstaub: I leave this up to you, I have no preferences. |
|
@csstaub: go ahead and add a new tag, it will be cleaner and easier to manage. |
|
Tagged as v2.1.8: https://github.com/square/go-jose/releases/tag/v2.1.8 |
Add functionality to support for password-based cryptography with the optional key algorithms:
PBES2 algorithms require two new parameters in the header p2c (pbkdf2 number of iterations) and p2s (salt). I added those parameters in the recipient and they are optional. Safe defaults are used if they are left empty, 100,000 for the count and a 128 bits salt. NIST recommendation is at least 10,000, but some applications like 1Password are already using 100k.
I'm open to suggestions, do a PR for the master branch, change the defaults, or use EncrypterOptions to add the extra parameters instead of the recipient.