Skip to content
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

(WIP) Ed25519 support. #206

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

(WIP) Ed25519 support. #206

wants to merge 7 commits into from

Conversation

DmitriyMusatkin
Copy link
Contributor

Issue #, if available:

Description of changes:
support ed25519 on linux through libcrypto and experimental support for mac/win through libcrypto

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

#include <aws/cal/ed25519.h>

struct aws_ed25519_key_pair *aws_ed25519_key_pair_new_generate(struct aws_allocator *allocator) {
return NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return NULL;
aws_raise_error(AWS_ERROR_CAL_UNSUPPORTED_ALGORITHM);
return NULL;

or AWS_ERROR_PLATFORM_NOT_SUPPORTED

}

struct aws_ed25519_key_pair *aws_ed25519_key_pair_acquire(struct aws_ed25519_key_pair *key_pair) {
return NULL;
Copy link
Contributor

@graebm graebm Feb 12, 2025

Choose a reason for hiding this comment

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

freak out if this normally-infallible function is going to fail

Suggested change
return NULL;
AWS_FATAL_ASSERT(key_pair == NULL);
return NULL;

}

struct aws_ed25519_key_pair *aws_ed25519_key_pair_release(struct aws_ed25519_key_pair *key_pair) {
return NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return NULL;
AWS_FATAL_ASSERT(key_pair == NULL);
return NULL;

}

size_t aws_ed25519_key_pair_get_private_key_size(enum aws_ed25519_key_export_format format) {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return 0;
AWS_FATAL_ASSERT(0);
return 0;

}

size_t aws_ed25519_key_pair_get_public_key_size(enum aws_ed25519_key_export_format format) {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return 0;
AWS_FATAL_ASSERT(0);
return 0;

if (!aws_byte_buf_write_be32(out, s_key_type_literal.len) ||
aws_byte_buf_append(out, &s_key_type_literal) != AWS_OP_SUCCESS || !aws_byte_buf_write_be32(out, 32)) {
return aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be AWS_ERROR_SHORT_BUFFER?

}

size_t pub_len = 32;
AWS_FATAL_ASSERT(out->capacity - out->len >= pub_len);
Copy link
Contributor

Choose a reason for hiding this comment

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

properly raise error and return,
(or have a length check earlier that makes it OK to simply assert here


int s_ed25519_export_public_raw(const struct aws_ed25519_key_pair *key_pair, struct aws_byte_buf *out) {
size_t remaining = out->capacity - out->len;
if (remaining < 32) {
Copy link
Contributor

Choose a reason for hiding this comment

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

debatable: should we be more flexible and call EVP_PKEY_get_raw_public_key() with NULL to learn the size?

Comment on lines +157 to +158

return AWS_OP_SUCCESS;
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: not necessary, since switch has a default:

Suggested change
return AWS_OP_SUCCESS;


on_error:
aws_byte_buf_clean_up(&key_buf);
return aws_raise_error(AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED);
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be nice to raise specific errors: SHORT_BUFFER vs AWS_ERROR_RANDOM_GEN_FAILED vs AWS_ERROR_CAL_CRYPTO_OPERATION_FAILED, instead of this blanket one

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.

2 participants