Skip to content

Algorithm Lucidity #8

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.

Sign up for GitHub

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

Draft
paragonie-security wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Algorithm Lucidity #8

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
167b725
Algorithm Lucidity
paragonie-security Aug 6, 2021
fa24b7a
Attempt to implement feedback
paragonie-security Aug 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions include/paseto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,25 @@ extern "C"{
#define paseto_v2_PUBLIC_PUBLICKEYBYTES 32U
#define paseto_v2_PUBLIC_SECRETKEYBYTES 64U

enum KeyHeader {
enum paseto_key_header {
V2_LOCAL = 4,
V2_PUBLIC = 5
Copy link
Copy Markdown
Member

@minus7 minus7 Aug 7, 2021

Choose a reason for hiding this comment

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

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(++).

Copy link
Copy Markdown
Author

@paragonie-security paragonie-security Aug 8, 2021

Choose a reason for hiding this comment

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

Understood. We don't normally do C development and was hoping the CI pipeline would catch these bugs.

};

typedef struct {
KeyHeader header;
struct paseto_v2_local_key {
paseto_key_header header;
uint8_t key_bytes[paseto_v2_LOCAL_KEYBYTES];
} v2_local_key;
};

typedef struct {
KeyHeader header;
struct paseto_v2_local_pk {
paseto_key_header header;
uint8_t key_bytes[paseto_v2_PUBLIC_PUBLICKEYBYTES];
} v2_public_pk;
};

typedef struct {
KeyHeader header;
struct paseto_v2_local_sk {
paseto_key_header header;
uint8_t key_bytes[paseto_v2_PUBLIC_SECRETKEYBYTES];
} v2_public_sk;
};

/**
* Initialize the library. Must be called before using any functionality.
Expand All @@ -52,14 +52,14 @@ void paseto_free(void *p);
* Returns false on error and sets errno.
*/
bool paseto_v2_local_load_key_hex(
v2_local_key key, const char *key_hex);
struct paseto_v2_local_key *key, const char *key_hex);

/**
* Load a base64-url-encoded key (without padding).
* Returns false on error and sets errno.
*/
bool paseto_v2_local_load_key_base64(
v2_local_key key, const char *key_base64);
struct paseto_v2_local_key *key, const char *key_base64);

/**
* Encrypt and encode `message` using `key`, attaching `footer` if it is not NULL.
Expand All @@ -69,7 +69,7 @@ bool paseto_v2_local_load_key_base64(
*/
char *paseto_v2_local_encrypt(
const uint8_t *message, size_t message_len,
const v2_local_key key,
const struct paseto_v2_local_key *key,
const uint8_t *footer, size_t footer_len);

/**
Expand All @@ -86,38 +86,38 @@ char *paseto_v2_local_encrypt(
*/
uint8_t *paseto_v2_local_decrypt(
const char *encoded, size_t *message_len,
const v2_local_key key,
const struct paseto_v2_local_key *key,
uint8_t **footer, size_t *footer_len);

/**
* Load a hex-encoded key. Returns false on error and sets errno.
*/
bool paseto_v2_public_load_public_key_hex(
v2_public_pk key,
struct paseto_v2_public_pk *key,
const char *key_hex);

/**
* Load a base64-url-encoded key (without padding).
* Returns false on error and sets errno.
*/
bool paseto_v2_public_load_public_key_base64(
v2_public_pk key,
struct paseto_v2_public_pk *key,
const char *key_base64);

/**
* Load a hex-encoded key.
* Returns false on error and sets errno.
*/
bool paseto_v2_public_load_secret_key_hex(
v2_public_sk key,
struct paseto_v2_public_sk *key,
const char *key_hex);

/**
* Load a base64-url-encoded key (without padding).
* Returns false on error and sets errno.
*/
bool paseto_v2_public_load_secret_key_base64(
v2_public_sk key,
struct paseto_v2_public_sk *key,
const char *key_base64);

/**
Expand All @@ -128,7 +128,7 @@ bool paseto_v2_public_load_secret_key_base64(
*/
char *paseto_v2_public_sign(
const uint8_t *message, size_t message_len,
const v2_public_sk key,
const struct paseto_v2_public_sk *key,
const uint8_t *footer, size_t footer_len);

/**
Expand All @@ -143,7 +143,7 @@ char *paseto_v2_public_sign(
*/
uint8_t *paseto_v2_public_verify(
const char *encoded, size_t *message_len,
const v2_public_pk key,
const struct paseto_v2_public_pk *key,
uint8_t **footer, size_t *footer_len);


Expand Down
28 changes: 16 additions & 12 deletions src/paseto_v2_local.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,22 @@ static const size_t mac_len = crypto_aead_xchacha20poly1305_ietf_ABYTES;


bool paseto_v2_local_load_key_hex(
v2_local_key key,
struct paseto_v2_local_key *key,
const char *key_hex) {
key.header = V2_LOCAL;
return key_load_hex(key.key_bytes, paseto_v2_LOCAL_KEYBYTES, key_hex);
struct paseto_v2_local_key tmp;
*key = &tmp;
key->header = V2_LOCAL;
return key_load_hex(key->key_bytes, paseto_v2_LOCAL_KEYBYTES, key_hex);
}


bool paseto_v2_local_load_key_base64(
v2_local_key key,
struct paseto_v2_local_key *key,
const char *key_base64) {
key.header = V2_LOCAL;
return key_load_base64(key.key_bytes, paseto_v2_LOCAL_KEYBYTES, key_base64);
struct paseto_v2_local_key tmp;
*key = &tmp;
key->header = V2_LOCAL;
return key_load_base64(key->key_bytes, paseto_v2_LOCAL_KEYBYTES, key_base64);
}


Expand All @@ -54,13 +58,13 @@ generate_nonce_fn generate_nonce = default_generate_nonce;

char *paseto_v2_local_encrypt(
const uint8_t *message, size_t message_len,
const v2_local_key key,
const struct paseto_v2_local_key *key,
const uint8_t *footer, size_t footer_len) {
if (!message || !key) {
errno = EINVAL;
return NULL;
}
if (key.header != V2_LOCAL) {
if (key->header != V2_LOCAL) {
errno = EINVAL;
return NULL;
}
Expand Down Expand Up @@ -95,7 +99,7 @@ char *paseto_v2_local_encrypt(
ct, NULL,
message, message_len,
pa.base, pre_auth_len,
NULL, nonce, key.key_bytes);
NULL, nonce, key->key_bytes);

free(pa.base);

Expand Down Expand Up @@ -140,13 +144,13 @@ char *paseto_v2_local_encrypt(

uint8_t *paseto_v2_local_decrypt(
const char *encoded, size_t *message_len,
const v2_local_key key,
const struct paseto_v2_local_key *key,
uint8_t **footer, size_t *footer_len) {
if (!encoded || !message_len || !key) {
errno = EINVAL;
return NULL;
}
if (key.header != V2_LOCAL) {
if (key->header != V2_LOCAL) {
errno = EINVAL;
return NULL;
}
Expand Down Expand Up @@ -243,7 +247,7 @@ uint8_t *paseto_v2_local_decrypt(
NULL,
ct, ct_len,
pa.base, pre_auth_len,
nonce, key.key_bytes) != 0) {
nonce, key->key_bytes) != 0) {
free(decoded);
free(pa.base);
free(message);
Expand Down
42 changes: 25 additions & 17 deletions src/paseto_v2_public.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,46 +21,54 @@ static const size_t signature_len = crypto_sign_BYTES;


bool paseto_v2_public_load_public_key_hex(
v2_public_pk key,
struct paseto_v2_public_pk *key,
const char *key_hex) {
key.header = V2_PUBLIC;
return key_load_hex(key.key_bytes, paseto_v2_PUBLIC_PUBLICKEYBYTES, key_hex);
struct paseto_v2_public_pk tmp;
*key = &tmp;
key->header = V2_PUBLIC;
return key_load_hex(key->key_bytes, paseto_v2_PUBLIC_PUBLICKEYBYTES, key_hex);
}


bool paseto_v2_public_load_public_key_base64(
v2_public_pk key,
struct paseto_v2_public_pk *key,
const char *key_base64) {
key.header = V2_PUBLIC;
return key_load_base64(key.key_bytes, paseto_v2_PUBLIC_PUBLICKEYBYTES, key_base64);
struct paseto_v2_public_pk tmp;
*key = &tmp;
key->header = V2_PUBLIC;
return key_load_base64(key->key_bytes, paseto_v2_PUBLIC_PUBLICKEYBYTES, key_base64);
}


bool paseto_v2_public_load_secret_key_hex(
v2_public_sk key,
struct paseto_v2_public_sk key,
const char *key_hex) {
key.header = V2_PUBLIC;
return key_load_hex(key.key_bytes, paseto_v2_PUBLIC_SECRETKEYBYTES, key_hex);
struct paseto_v2_public_sk tmp;
*key = &tmp;
key->header = V2_PUBLIC;
return key_load_hex(key->key_bytes, paseto_v2_PUBLIC_SECRETKEYBYTES, key_hex);
}


bool paseto_v2_public_load_secret_key_base64(
v2_public_sk key,
const char *key_base64) {
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;
Copy link
Copy Markdown
Member

@minus7 minus7 Aug 7, 2021

Choose a reason for hiding this comment

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

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.

key->header = V2_PUBLIC;
return key_load_base64(key->key_bytes, paseto_v2_PUBLIC_SECRETKEYBYTES, key_base64);
}


char *paseto_v2_public_sign(
const uint8_t *message, size_t message_len,
v2_public_sk key,
struct paseto_v2_public_sk *key,
const uint8_t *footer, size_t footer_len) {
if (!message || !key) {
errno = EINVAL;
return NULL;
}
if (key.header != V2_PUBLIC) {
if (key->header != V2_PUBLIC) {
errno = EINVAL;
return NULL;
}
Expand All @@ -87,7 +95,7 @@ char *paseto_v2_public_sign(
size_t pre_auth_len = pa.current - pa.base;

uint8_t *ct = to_encode + message_len;
crypto_sign_detached(ct, NULL, pa.base, pre_auth_len, key.key_bytes);
crypto_sign_detached(ct, NULL, pa.base, pre_auth_len, key->key_bytes);

free(pa.base);

Expand Down Expand Up @@ -132,13 +140,13 @@ char *paseto_v2_public_sign(

uint8_t *paseto_v2_public_verify(
const char *encoded, size_t *message_len,
const v2_public_pk key,
const struct paseto_v2_public_pk *key,
uint8_t **footer, size_t *footer_len) {
if (!encoded || !message_len || !key) {
errno = EINVAL;
return NULL;
}
if (key.header != V2_PUBLIC) {
if (key->header != V2_PUBLIC) {
errno = EINVAL;
return NULL;
}
Expand Down Expand Up @@ -227,7 +235,7 @@ uint8_t *paseto_v2_public_verify(
return NULL;
}
if (crypto_sign_verify_detached(
signature, pa.base, pre_auth_len, key.key_bytes) != 0) {
signature, pa.base, pre_auth_len, key->key_bytes) != 0) {
free(decoded);
free(decoded_footer);
free(pa.base);
Expand Down