Algorithm Lucidity #8
Conversation
This wraps keys into a struct, which has a header flag. This flag is checked at runtime. KeyHeader is an enum. The least significant bit holds purpose (Local = 0, Public = 1); the remaining are reserved for the version. This results in the following pattern: - V1_LOCAL = 2 - V1_PUBLIC = 3 - V2_LOCAL = 4 - V2_PUBLIC = 5 - V3_LOCAL = 6 - V3_PUBLIC = 7 - V4_LOCAL = 8 - V4_PUBLIC = 9 - ...
|
(This is a draft until we're confident that we didn't screw anything up.) |
|
Thanks for the PR. Here's some feedback:
|
| key.header = V2_PUBLIC; | ||
| return key_load_base64(key.key_bytes, paseto_v2_PUBLIC_SECRETKEYBYTES, key_base64); | ||
| struct paseto_v2_public_sk tmp; | ||
| *key = &tmp; |
There was a problem hiding this comment.
There's no point in doing this, since tmp does not get initialized in C, you can just drop it. Also the &tmp is plain wrong, you can't the address of a variable on the stack and returns it; aside from the fact that this doesn't even compile. Please test your patches before you send them, the readme contains the few simple instructions necessary.
| @@ -17,6 +17,26 @@ extern "C"{ | |||
| #define paseto_v2_PUBLIC_PUBLICKEYBYTES 32U | |||
| #define paseto_v2_PUBLIC_SECRETKEYBYTES 64U | |||
|
|
|||
| enum paseto_key_header { | |||
| V2_LOCAL = 4, | |||
| V2_PUBLIC = 5 | |||
There was a problem hiding this comment.
Everything in the header should be namespaced/prefixed so it doesn't potentially collide with other libraries or code. Enums aren't namespaced in C(++).
There was a problem hiding this comment.
Understood. We don't normally do C development and was hoping the CI pipeline would catch these bugs.
This wraps keys into a struct, which has a header flag. This flag is checked at runtime.
KeyHeader is an enum. The least significant bit holds purpose (Local = 0, Public = 1); the remaining are reserved for the version. This results in the following pattern:
See #7