-
-
Notifications
You must be signed in to change notification settings - Fork 22
Export V4AsymmetricPublicKey to paserk k4.public #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
21b6849
5935d37
6d343b5
e7fab38
66e376b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| package paseto | ||
|
|
||
| // keyPurpose indicates if key is symmetric, private or public | ||
| type keyPurpose int | ||
|
|
||
| // keyVersion indicates the token version the key may be used for | ||
| type KeyVersion int | ||
|
|
||
| const ( | ||
| // Invalid key version | ||
| KeyVersionInvalid KeyVersion = 0 | ||
| // Key used for V1 tokens | ||
| KeyVersionV1 KeyVersion = 1 | ||
| // Key used for V2 tokens | ||
| KeyVersionV2 KeyVersion = 2 | ||
| // Key used for V3 tokens | ||
| KeyVersionV3 KeyVersion = 3 | ||
| // Key used for V4 tokens | ||
| KeyVersionV4 KeyVersion = 4 | ||
|
|
||
| // Symmetric key used for local tokens | ||
| keyPurposeLocal keyPurpose = 1 | ||
| // Asymmetric secret key used for public tokens | ||
| keyPurposeSecret keyPurpose = 2 | ||
| // Asymmetric public key used for public tokens | ||
| keyPurposePublic keyPurpose = 3 | ||
| ) | ||
|
|
||
| type Key interface { | ||
| // Export raw key data as hex string | ||
| ExportHex() string | ||
| // Export raw key data as byte array | ||
| ExportBytes() []byte | ||
| // Returns purpose of key | ||
| getPurpose() keyPurpose | ||
| // Returns the version of the paseto tokens the key may be used for | ||
| getVersion() KeyVersion | ||
| } | ||
|
|
||
| // Convert key version to PASERK header string | ||
| func KeyVersionToString(version KeyVersion) string { | ||
| switch version { | ||
| case KeyVersionV1: | ||
| return "k1" | ||
| case KeyVersionV2: | ||
| return "k2" | ||
| case KeyVersionV3: | ||
| return "k3" | ||
| case KeyVersionV4: | ||
| return "k4" | ||
| default: | ||
| return "" | ||
| } | ||
| } | ||
|
|
||
| // Parse key version from PASERK header string (eg. "k2") | ||
| func KeyVersionFromString(versionStr string) KeyVersion { | ||
| switch versionStr { | ||
| case "k1": | ||
| return KeyVersionV1 | ||
| case "k2": | ||
| return KeyVersionV2 | ||
| case "k3": | ||
| return KeyVersionV3 | ||
| case "k4": | ||
| return KeyVersionV4 | ||
| default: | ||
| return KeyVersionInvalid | ||
| } | ||
| } | ||
|
Comment on lines
+57
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My preference for this would be to return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, it is quite simple to add a second return value with error type, but even that case something must be returned as KeyVersion as well. The following statement would be invalid: The return value must be an integer (which is the underlying type of KeyVersion). Default value of int is 0, which is the preferred return value on error. or equivalently (as So if you think, it is cleaner to return an error object as well, I could simply add it, just please let me know. I'm not very familiar with coding conventions in go. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we return a pointer to the key version, and an error In Go, usually we return an explicit error for failure (which then reminds the caller to check for an error, handling where the returned There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the signature to return a pointer would cause an error on valid cases. KeyVersion instances are currently constants, not variables, so the code below would be invalid: Should each There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah you're right, sorry I overlooked that. Do we actually need this pattern at all if we set: const KeyVersionV1 = "k1"etc...? If we do, I'd still prefer we return an error than rely on callers checking for the invalid value, as the error is more explicit from a function signature standpoint, so harder to forget. Either we can assign the const to a locally scoped var before returning (if using pointers), or return the invalid zero value (though maybe we don't need to actually export that invalid value const as part of the API). |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| package paseto | ||
|
|
||
| import ( | ||
| "encoding/base64" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| // type of PASERK token | ||
| type PaserkType int | ||
|
|
||
| // Error indicating the given paserk import/export method is not yet implemented on the key type. | ||
| type NotImplementedError struct { | ||
| keyTypeStr string | ||
| paserkTypeStr string | ||
| } | ||
|
|
||
| // Error indicating the given paserk type is not valid for the key type. | ||
| type InvalidPaserkTypeError struct { | ||
| keyTypeStr string | ||
| paserkTypeStr string | ||
| } | ||
|
|
||
| const ( | ||
| // Invalid Paserk token type | ||
| PaserkTypeInvalid PaserkType = 0 | ||
| // Unique Identifier for a separate PASERK for local PASETOs | ||
| PaserkTypeLid PaserkType = 1 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason not to use the strings corresponding to each variant here for simplicity? |
||
| // Symmetric key for local tokens | ||
| PaserkTypeLocal PaserkType = 2 | ||
| // Symmetric key wrapped using asymmetric encryption | ||
| PaserkTypeSeal PaserkType = 3 | ||
| // Symmetric key wrapped by another symmetric key | ||
| PaserkTypeLocalWrap PaserkType = 4 | ||
| // Symmetric key wrapped using password-based encryption | ||
| PaserkTypeLocalPw PaserkType = 5 | ||
| // Unique Identifier for a separate PASERK for public PASETOs. (Secret Key) | ||
| PaserkTypeSid PaserkType = 6 | ||
| // Public key for verifying public tokens | ||
| PaserkTypePublic PaserkType = 7 | ||
| // Unique Identifier for a separate PASERK for public PASETOs. (Public Key) | ||
| PaserkTypePid PaserkType = 8 | ||
| // Secret key for signing public tokens | ||
| PaserkTypeSecret PaserkType = 9 | ||
| // Asymmetric secret key wrapped by another symmetric key | ||
| PaserkTypeSecretWrap PaserkType = 10 | ||
| // Asymmetric secret key wrapped using password-based encryption | ||
| PaserkTypeSecretPw PaserkType = 11 | ||
| ) | ||
|
|
||
| // Convert token type to PASERK header string `type` | ||
| func PaserkTypeToString(paserkType PaserkType) string { | ||
| switch paserkType { | ||
| case PaserkTypeLid: | ||
| return "lid" | ||
| case PaserkTypeLocal: | ||
| return "local" | ||
| case PaserkTypeSeal: | ||
| return "seal" | ||
| case PaserkTypeLocalWrap: | ||
| return "local-wrap" | ||
| case PaserkTypeLocalPw: | ||
| return "local-pw" | ||
| case PaserkTypeSid: | ||
| return "sid" | ||
| case PaserkTypePublic: | ||
| return "public" | ||
| case PaserkTypePid: | ||
| return "pid" | ||
| case PaserkTypeSecret: | ||
| return "secret" | ||
| case PaserkTypeSecretWrap: | ||
| return "secret-wrap" | ||
| case PaserkTypeSecretPw: | ||
| return "secret-pw" | ||
| default: | ||
| return "" | ||
| } | ||
| } | ||
|
|
||
| // Parse token type from string value of `type` field of PASERK token | ||
| func PaserkTypeFromString(typeStr string) PaserkType { | ||
| switch typeStr { | ||
| case "lid": | ||
| return PaserkTypeLid | ||
| case "local": | ||
| return PaserkTypeLocal | ||
| case "seal": | ||
| return PaserkTypeSeal | ||
| case "local-wrap": | ||
| return PaserkTypeLocalWrap | ||
| case "local-pw": | ||
| return PaserkTypeLocalPw | ||
| case "sid": | ||
| return PaserkTypeSid | ||
| case "public": | ||
| return PaserkTypePublic | ||
| case "pid": | ||
| return PaserkTypePid | ||
| case "secret": | ||
| return PaserkTypeSecret | ||
| case "secret-wrap": | ||
| return PaserkTypeSecretWrap | ||
| case "secret-pw": | ||
| return PaserkTypeSecretPw | ||
| default: | ||
| return PaserkTypeInvalid | ||
| } | ||
| } | ||
|
|
||
| // Checks if the representation (paserk token type) is available for the key | ||
| func (paserkType PaserkType) isAvailableForKey(key Key) bool { | ||
| switch key.getPurpose() { | ||
| case keyPurposeLocal: | ||
| switch paserkType { | ||
| case PaserkTypeLid, | ||
| PaserkTypeLocal, | ||
| PaserkTypeSeal, | ||
| PaserkTypeLocalWrap, | ||
| PaserkTypeLocalPw: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| case keyPurposePublic: | ||
| switch paserkType { | ||
| case PaserkTypePid, | ||
| PaserkTypePublic: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| case keyPurposeSecret: | ||
| switch paserkType { | ||
| case PaserkTypeSid, | ||
| PaserkTypeSecret, | ||
| PaserkTypeSecretWrap, | ||
| PaserkTypeSecretPw: | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| func (e NotImplementedError) Error() string { | ||
| return fmt.Sprintf("PASERK type %s is not yet implemented on key type %s", e.paserkTypeStr, e.keyTypeStr) | ||
| } | ||
|
|
||
| func (e InvalidPaserkTypeError) Error() string { | ||
| return fmt.Sprintf("PASERK type %s is invalid for key type %s", e.paserkTypeStr, e.keyTypeStr) | ||
| } | ||
|
|
||
| // ExportPaserk export a V4AsymmetricPublicKey to a paserk token of type paserkType | ||
| func ExportPaserkRaw(k Key) (string, error) { | ||
| var paserkType PaserkType | ||
| switch k.getPurpose() { | ||
| case keyPurposeLocal: | ||
| paserkType = PaserkTypeLocal | ||
| case keyPurposeSecret: | ||
| paserkType = PaserkTypeSecret | ||
| case keyPurposePublic: | ||
| paserkType = PaserkTypePublic | ||
| default: | ||
| return "", errors.New("invalid key purpose") | ||
| } | ||
| if !paserkType.isAvailableForKey(k) { | ||
| return "", InvalidPaserkTypeError{fmt.Sprintf("%T", k), PaserkTypeToString(paserkType)} | ||
| } | ||
| if paserkType != PaserkTypePublic { | ||
| return "", NotImplementedError{fmt.Sprintf("%T", k), PaserkTypeToString(paserkType)} | ||
| } | ||
| header := KeyVersionToString(k.getVersion()) + "." + PaserkTypeToString(paserkType) + "." | ||
| data := base64.RawURLEncoding.EncodeToString(k.ExportBytes()) | ||
| return header + data, nil | ||
| } | ||
|
|
||
| func ParsePaserkRaw(paserkStr string) (Key, error) { | ||
| frags := strings.Split(paserkStr, ".") | ||
| if len(frags) != 3 { | ||
| return nil, fmt.Errorf("Invalid PASERK token: %s", paserkStr) | ||
| } | ||
| tokenVersion := KeyVersionFromString(frags[0]) | ||
| typ := PaserkTypeFromString(frags[1]) | ||
| data, err := base64.RawURLEncoding.DecodeString(frags[2]) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "can't decode data part of pasrk key") | ||
| } | ||
|
|
||
| switch tokenVersion { | ||
| case KeyVersionV4: | ||
| switch typ { | ||
| case PaserkTypePublic: | ||
| key, err := NewV4AsymmetricPublicKeyFromBytes(data) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "can't construct key from data part of paserk key") | ||
| } | ||
| return &key, nil | ||
| default: | ||
| return nil, NotImplementedError{frags[0], frags[1]} | ||
| } | ||
| default: | ||
| return nil, NotImplementedError{frags[0], frags[1]} | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| { | ||
| "name": "PASERK k4.public Test Vectors", | ||
| "tests": [ | ||
| { | ||
| "name": "k4.public-1", | ||
| "expect-fail": false, | ||
| "key": "0000000000000000000000000000000000000000000000000000000000000000", | ||
| "paserk": "k4.public.AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" | ||
| }, | ||
| { | ||
| "name": "k4.public-2", | ||
| "expect-fail": false, | ||
| "key": "707172737475767778797a7b7c7d7e7f808182838485868788898a8b8c8d8e8f", | ||
| "paserk": "k4.public.cHFyc3R1dnd4eXp7fH1-f4CBgoOEhYaHiImKi4yNjo8" | ||
| }, | ||
| { | ||
| "name": "k4.public-3", | ||
| "expect-fail": false, | ||
| "key": "707172737475767778797a7b7c7d7e7f808182838485868788898a8b8c8d8e90", | ||
| "paserk": "k4.public.cHFyc3R1dnd4eXp7fH1-f4CBgoOEhYaHiImKi4yNjpA" | ||
| }, | ||
| { | ||
| "name": "k4.public-fail-1", | ||
| "expect-fail": true, | ||
| "key": "02707172737475767778797a7b7c7d7e7f808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9f", | ||
| "paserk": null, | ||
| "comment": "Implementations MUST NOT accept a PASERK of the wrong version." | ||
| } | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,8 @@ type TestVector struct { | |
| Footer string | ||
| ExpectFail bool `json:"expect-fail"` | ||
| ImplicitAssertation string `json:"implicit-assertion"` | ||
| Paserk string | ||
| Comment string | ||
|
Comment on lines
+30
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than adding onto the PASETO vectors test here, I think probably we can just create a separate set of tests for PASREK (since most of the PASETO fields are going to be missing from parsed PASREK data). |
||
| } | ||
|
|
||
| func TestV2(t *testing.T) { | ||
|
|
@@ -299,3 +301,48 @@ func TestV4(t *testing.T) { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestPaserkV4Public(t *testing.T) { | ||
| data, err := os.ReadFile("test-vectors/PASERK/k4.public.json") | ||
| require.NoError(t, err) | ||
|
|
||
| var tests TestVectors | ||
| err = json.Unmarshal(data, &tests) | ||
| require.NoError(t, err) | ||
|
|
||
| for _, test := range tests.Tests { | ||
| t.Run(test.Name, func(t *testing.T) { | ||
|
|
||
| k, err := paseto.NewV4AsymmetricPublicKeyFromHex(test.Key) | ||
| if test.ExpectFail { | ||
| require.Error(t, err) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
|
|
||
| token, err := paseto.ExportPaserkRaw(&k) | ||
| if test.ExpectFail { | ||
| require.Error(t, err) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
|
|
||
| require.Equal(t, test.Paserk, token) | ||
| }) | ||
| } | ||
|
|
||
| for _, test := range tests.Tests { | ||
| t.Run(test.Name+"-reverse", func(t *testing.T) { | ||
|
|
||
| k, err := paseto.ParsePaserkRaw(test.Paserk) | ||
| if test.ExpectFail { | ||
| require.Error(t, err) | ||
| return | ||
| } | ||
| require.NoError(t, err) | ||
|
|
||
| v4key := k.(*paseto.V4AsymmetricPublicKey) | ||
| require.Equal(t, test.Key, v4key.ExportHex()) | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible without introducing dependency cycles, I think we can probably contain most of PASREK in its own submodule? (As opposed to including it directly in the main
pasetopackage)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried to do it, after getting deeper in the paserk standard, and I see the following possibilities:
[]byte) methods to export keys/create new ones. In this case paseto package would depend on paserk. An example (currently without error handling) would look like:v2_keys.goneeds to be extended as follows:I would plan to implement the Marshaler/Unmarshaler interface to call the raw paserk encoding of the keys. Such a function (like
func (k *V2SymmetricKey) UnmarshalJSON(jsonstr []byte) error) must be implemented in the paseto package because of the receiver. It can only be done of paseto package depends on paserk and not vica versa.So which option would you prefer? Or can you come up with a different idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Architecturally I think it probably makes the most sense for the paserk package to depend upon paseto (as in option 3). If you're using paserk, you'll be using it for paseto (but if you're using paseto, you might not use paserk). This would also matches the way the reference implementation is setup (not the only reason to do it, but makes things a bit simpler if we want to compare implementations).
I do also like the API simplicity of using paserk to first construct a key, and then using paseto to sign/encrypt the message. There's a nice separation of concerns there, and means that we can keep the key export options within the paseto package quite simple.