Skip to content

Conversation

@aidantwoods
Copy link

As alluded to in #32, keys should to be strongly bound to their parameter choices to prevent algorithm confusion attacks (so byte arrays or similar shouldn't be accepted). From the PASETO spec:

PASETO Cryptography Key Requirements

Cryptography keys in PASETO are defined as both the raw key material and its parameter choices, not just the raw key material.

PASETO implementations MUST enforce some logical separation between different key types; especially when the raw key material is the same (i.e. a 256-bit opaque blob).

Arbitrary strings (or byte arrays, or equivalent language constructs) MUST NOT be accepted as a key in any PASETO library, [...]

I've opted to refactor the core PASETO operations into methods associated with each specific key (e.g. V2SymmetricKey has implementations for encrypt, decrypt involving its raw material). This means that the version level methods just need to do a type assertion checking that the given key matches the version, before deferring down to the key specific implementation.

Fixes #32

Rather than accepting bytes we accept a generalised key type associated
with its function.
Keys are a natural place for encrypt implementations associated with
that particular key to live. This also allows us to gate the
unitTestNonce behind a non-exported method.
Key splitting should be a method of the key in particular. This
particularly relevant for PASETO v3 and v4 implementations, which have
different key splitting logic to v1.
Implement v1 in terms of specific key types
Implement v2 in terms of versions specific keys
// slice of 32 bytes, regardless of whether PASETO v1 or v2 is being used.
Encrypt(key []byte, payload interface{}, footer interface{}) (string, error)
// Encrypt encrypts a token with a symmetric key
Encrypt(key SymmetricKey, payload interface{}, footer interface{}) (string, error)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something perhaps to consider here is how to handle the implicit param added the encrypt, decrypt, sign, and verify in v3 and v4. It probably isn't ideal to add an implicit arg to the existing methods in v1 and v2, as it may falsely give the impression that the implicit param is being validated (when it would be ignored for those versions). Perhaps a reasonable middle ground would be to add the param everywhere, but error if implicit is non-empty in v1 and v2. An alternative would be to specify separate methods altogether e.g. EncryptImplicit(key SymmetricKey, payload interface{}, footer interface{}, implicit interface{}) (string, error), where these implicit methods always throw for v1 and v2.

Copy link

@Gnarnar Gnarnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workflows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bind Keys to Version and Purpose

2 participants