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

refactor pem #605

Merged
merged 22 commits into from
Oct 9, 2023
Merged
Changes from 11 commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# IDE Artifacts
.metadata
.build
.vscode
.idea
*.d
Debug
2 changes: 2 additions & 0 deletions include/aws/io/io.h
Original file line number Diff line number Diff line change
@@ -252,6 +252,8 @@ enum aws_io_errors {

AWS_IO_TLS_ERROR_READ_FAILURE,

AWS_ERROR_PEM_MALFORMED_OBJECT,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you expect other systems to use a different error if the objects themselves are all valid, but the "document" itself is malformed due to missing required objects?

If we fear people will re-use this error-code for bad documents, give it a more generic name like AWS_ERROR_PEM_MALFORMED. But if you want distinct error-codes to be created for malformed documents of a specific type, then this is good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me just make it generic AWS_ERROR_PEM_MALFORMED. i dont think its worth distinguishing that its individual objects that are malformed


AWS_IO_ERROR_END_RANGE = AWS_ERROR_ENUM_END_RANGE(AWS_C_IO_PACKAGE_ID),
AWS_IO_INVALID_FILE_HANDLE = AWS_ERROR_INVALID_FILE_HANDLE,
};
1 change: 1 addition & 0 deletions include/aws/io/logging.h
Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ enum aws_io_log_subject {
AWS_LS_IO_EXPONENTIAL_BACKOFF_RETRY_STRATEGY,
AWS_LS_IO_STANDARD_RETRY_STRATEGY,
AWS_LS_IO_PKCS11,
AWS_LS_IO_PEM,
AWS_IO_LS_LAST = AWS_LOG_SUBJECT_END_RANGE(AWS_C_IO_PACKAGE_ID)
};
AWS_POP_SANE_WARNING_LEVEL
95 changes: 95 additions & 0 deletions include/aws/io/pem.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
#ifndef AWS_IO_PEM_H
#define AWS_IO_PEM_H

/**
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
#include <aws/io/io.h>

AWS_EXTERN_C_BEGIN

enum aws_pem_object_type {
AWS_PEM_TYPE_UNKNOWN = 0,
AWS_PEM_TYPE_X509_OLD,
AWS_PEM_TYPE_X509,
graebm marked this conversation as resolved.
Show resolved Hide resolved
AWS_PEM_TYPE_X509_TRUSTED,
AWS_PEM_TYPE_X509_REQ_OLD,
AWS_PEM_TYPE_X509_REQ,
AWS_PEM_TYPE_X509_CRL,
AWS_PEM_TYPE_EVP_PKEY,
AWS_PEM_TYPE_PUBLIC_PKCS8,
AWS_PEM_TYPE_PRIVATE_RSA_PKCS1,
AWS_PEM_TYPE_PUBLIC_RSA_PKCS1,
AWS_PEM_TYPE_PRIVATE_DSA_PKCS1,
AWS_PEM_TYPE_PUBLIC_DSA_PKCS1,
AWS_PEM_TYPE_PKCS7,
AWS_PEM_TYPE_PKCS7_SIGNED_DATA,
AWS_PEM_TYPE_PRIVATE_PKCS8_ENCRYPTED,
AWS_PEM_TYPE_PRIVATE_PKCS8,
AWS_PEM_TYPE_DH_PARAMETERS,
AWS_PEM_TYPE_DH_PARAMETERS_X942,
AWS_PEM_TYPE_SSL_SESSION_PARAMETERS,
AWS_PEM_TYPE_DSA_PARAMETERS,
AWS_PEM_TYPE_ECDSA_PUBLIC,
AWS_PEM_TYPE_EC_PARAMETERS,
AWS_PEM_TYPE_EC_PRIVATE,
AWS_PEM_TYPE_PARAMETERS,
AWS_PEM_TYPE_CMS,
AWS_PEM_TYPE_SM2_PARAMETERS
};

/*
* Describes PEM object decoded from file.
* data points to raw data bytes of object (decoding will do additional base 64
* decoding for each object).
* type will be set to object type or to AWS_PEM_TYPE_UNKNOWN if it could not
* figure out type.
* type_buf are the types bytes, i.e. the string between -----BEGIN and -----
*/
struct aws_pem_object {
graebm marked this conversation as resolved.
Show resolved Hide resolved
enum aws_pem_object_type type;
struct aws_byte_buf type_buf;
DmitriyMusatkin marked this conversation as resolved.
Show resolved Hide resolved
struct aws_byte_buf data;
};

/**
* Cleans up and securely zeroes out the outputs of 'aws_decode_pem_to_object_list()'
* and 'aws_read_and_decode_pem_file_to_object_list()'
*/
AWS_IO_API void aws_pem_objects_clean_up(struct aws_array_list *pem_objects);
DmitriyMusatkin marked this conversation as resolved.
Show resolved Hide resolved

/**
* Decodes PEM data and reads objects sequentially adding them to pem_objects.
* If it comes across an object it cannot read, list of all object read until
* that point is returned.
* If no objects can be read PEM or objects could not be base 64 decoded,
* AWS_ERROR_PEM_MALFORMED_OBJECT is raised.
* out_pem_objects stores aws_pem_object struct by value.
* Caller must initialize out_pem_objects before calling the function.
* This code is slow, and it allocates, so please try
* not to call this in the middle of something that needs to be fast or resource sensitive.
*/
AWS_IO_API int aws_decode_pem_to_object_list(
struct aws_allocator *alloc,
struct aws_byte_cursor pem_cursor,
struct aws_array_list *out_pem_objects);

/**
* Decodes PEM data from file and reads objects sequentially adding them to pem_objects.
* If it comes across an object it cannot read, list of all object read until
* that point is returned.
* If no objects can be read PEM or objects could not be base 64 decoded,
* AWS_ERROR_PEM_MALFORMED_OBJECT is raised.
* out_pem_objects stores aws_pem_object struct by value.
* Caller must initialize out_pem_objects before calling the function.
* This code is slow, and it allocates, so please try
* not to call this in the middle of something that needs to be fast or resource sensitive.
*/
AWS_IO_API int aws_read_and_decode_pem_file_to_object_list(
Copy link
Contributor

Choose a reason for hiding this comment

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

debatable: would it be simpler if these functions were alternate array-list initializers? I can't imagine this function ever being used without the user needing to initialize the list first. Like:

int aws_array_list_init_from_pem_file_contents(struct aws_array_list *list, struct aws_allocator, struct aws_byte_cursor pem_contents);

int aws_array_list_init_from_pem_file_path(struct aws_array_list *list, struct aws_allocator, const char *pem_path);

Or call it "pem_objects" instead of "array_list" (I like this idea best)?

int aws_pem_objects_init_from_file_contents(struct aws_array_list *pem_objects, struct aws_allocator, struct aws_byte_cursor pem_contents);

int aws_pem_objects_init_from_file_path(struct aws_array_list *pem_objects, struct aws_allocator, const char *pem_path);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current names were mostly based on existing names for internal function with a modification to refer to elements as pem objects instead of cert_or_key.
I like your suggestion of just naming it pem_objects_init. next commit switches to that.

I think original code wanted array to be initialized outside of the call because the caller might have better context on how many elements are there in the pem file. But in practice the answer to that is either probably just one or probably many, so initializing outside was not super helpful. Moved array init inside of the function.

struct aws_allocator *allocator,
const char *filename,
struct aws_array_list *out_pem_objects);

AWS_EXTERN_C_END
#endif /* AWS_IO_PEM_H */
30 changes: 0 additions & 30 deletions include/aws/io/private/pki_utils.h
Original file line number Diff line number Diff line change
@@ -21,43 +21,13 @@ struct aws_string;

AWS_EXTERN_C_BEGIN

/**
* Cleans up and securely zeroes out the outputs of 'aws_decode_pem_to_buffer_list()'
* and 'aws_read_and_decode_pem_file_to_buffer_list()'
*/
AWS_IO_API void aws_cert_chain_clean_up(struct aws_array_list *cert_chain);

/**
* Decodes a PEM file and adds the results to 'cert_chain_or_key' if successful.
* Otherwise, 'cert_chain_or_key' will be empty. The type stored in 'cert_chain_or_key'
* is 'struct aws_byte_buf' by value. This code is slow, and it allocates, so please try
* not to call this in the middle of something that needs to be fast or resource sensitive.
*/
AWS_IO_API int aws_decode_pem_to_buffer_list(
struct aws_allocator *alloc,
const struct aws_byte_cursor *pem_cursor,
struct aws_array_list *cert_chain_or_key);

/**
* Returns the path to the directory and file, respectively, which holds the
* SSL certificate trust store on the system.
*/
AWS_IO_API const char *aws_determine_default_pki_dir(void);
AWS_IO_API const char *aws_determine_default_pki_ca_file(void);

/**
* Decodes a PEM file at 'filename' and adds the results to 'cert_chain_or_key' if successful.
* Otherwise, 'cert_chain_or_key' will be empty.
* The passed-in parameter 'cert_chain_or_key' should be empty and dynamically initialized array_list
* with item type 'struct aws_byte_buf' in value.
* This code is slow, and it allocates, so please try not to call this in the middle of
* something that needs to be fast or resource sensitive.
*/
AWS_IO_API int aws_read_and_decode_pem_file_to_buffer_list(
struct aws_allocator *alloc,
const char *filename,
struct aws_array_list *cert_chain_or_key);

#ifdef AWS_OS_APPLE
# if !defined(AWS_OS_IOS)
/**
43 changes: 25 additions & 18 deletions source/darwin/darwin_pki_utils.c
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@
#include <aws/common/mutex.h>
#include <aws/common/string.h>
#include <aws/io/logging.h>
#include <aws/io/pem.h>

#include <Security/SecCertificate.h>
#include <Security/SecKey.h>
@@ -33,16 +34,17 @@ int aws_import_ecc_key_into_keychain(
SecKeychainRef import_keychain) {
// Ensure imported_keychain is not NULL
AWS_PRECONDITION(import_keychain != NULL);
AWS_PRECONDITION(private_key != NULL);

int result = AWS_OP_ERR;
struct aws_array_list decoded_key_buffer_list;
/* Init empty array list, ideally, the PEM should only has one key included. */
if (aws_array_list_init_dynamic(&decoded_key_buffer_list, alloc, 1, sizeof(struct aws_byte_buf))) {
if (aws_array_list_init_dynamic(&decoded_key_buffer_list, alloc, 1, sizeof(struct aws_pem_object))) {
return result;
}

/* Decode PEM format file to DER format */
if (aws_decode_pem_to_buffer_list(alloc, private_key, &decoded_key_buffer_list)) {
if (aws_decode_pem_to_object_list(alloc, *private_key, &decoded_key_buffer_list)) {
AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: Failed to decode PEM private key to DER format.");
goto ecc_import_cleanup;
}
@@ -51,11 +53,11 @@ int aws_import_ecc_key_into_keychain(
// A PEM file could contains multiple PEM data section. Try importing each PEM section until find the first
// succeed key.
for (size_t index = 0; index < aws_array_list_length(&decoded_key_buffer_list); index++) {
struct aws_byte_buf *decoded_key_buffer = NULL;
struct aws_pem_object *pem_object_ptr = NULL;
/* We only check the first pem section. Currently, we dont support key with multiple pem section. */
aws_array_list_get_at_ptr(&decoded_key_buffer_list, (void **)&decoded_key_buffer, index);
aws_array_list_get_at_ptr(&decoded_key_buffer_list, (void **)&pem_object_ptr, index);
AWS_ASSERT(decoded_key_buffer);
CFDataRef key_data = CFDataCreate(cf_alloc, decoded_key_buffer->buffer, decoded_key_buffer->len);
CFDataRef key_data = CFDataCreate(cf_alloc, pem_object_ptr->data.buffer, pem_object_ptr->data.len);
if (!key_data) {
AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: error in creating ECC key data system call.");
continue;
@@ -68,6 +70,7 @@ int aws_import_ecc_key_into_keychain(
AWS_ZERO_STRUCT(import_params);
import_params.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION;
import_params.passphrase = CFSTR("");
format = kSecFormatUnknown;

OSStatus key_status =
SecItemImport(key_data, NULL, &format, &item_type, 0, &import_params, import_keychain, NULL);
@@ -87,7 +90,7 @@ int aws_import_ecc_key_into_keychain(

ecc_import_cleanup:
// Zero out the array list and release it
aws_cert_chain_clean_up(&decoded_key_buffer_list);
aws_pem_objects_clean_up(&decoded_key_buffer_list);
aws_array_list_clean_up(&decoded_key_buffer_list);
return result;
}
@@ -99,6 +102,8 @@ int aws_import_public_and_private_keys_to_identity(
const struct aws_byte_cursor *private_key,
CFArrayRef *identity,
const struct aws_string *keychain_path) {
AWS_PRECONDITION(public_cert_chain != NULL);
AWS_PRECONDITION(private_key != NULL);

int result = AWS_OP_ERR;

@@ -179,7 +184,7 @@ int aws_import_public_and_private_keys_to_identity(

/*
* If the key format is unknown, we tried to decode the key into DER format import it.
* The PEM file might contians multiple key sections, we will only add the first succeed key into the keychain.
* The PEM file might contains multiple key sections, we will only add the first succeed key into the keychain.
*/
if (key_status == errSecUnknownFormat) {
AWS_LOGF_TRACE(AWS_LS_IO_PKI, "static: error reading private key format, try ECC key format.");
@@ -203,29 +208,29 @@ int aws_import_public_and_private_keys_to_identity(
"Using key from Keychain instead of the one provided.");
struct aws_array_list cert_chain_list;

if (aws_array_list_init_dynamic(&cert_chain_list, alloc, 2, sizeof(struct aws_byte_buf))) {
if (aws_array_list_init_dynamic(&cert_chain_list, alloc, 2, sizeof(struct aws_pem_object))) {
result = AWS_OP_ERR;
goto done;
}

if (aws_decode_pem_to_buffer_list(alloc, public_cert_chain, &cert_chain_list)) {
if (aws_decode_pem_to_object_list(alloc, *public_cert_chain, &cert_chain_list)) {
AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: decoding certificate PEM failed.");
aws_array_list_clean_up(&cert_chain_list);
result = AWS_OP_ERR;
goto done;
}

struct aws_byte_buf *root_cert_ptr = NULL;
struct aws_pem_object *root_cert_ptr = NULL;
aws_array_list_get_at_ptr(&cert_chain_list, (void **)&root_cert_ptr, 0);
AWS_ASSERT(root_cert_ptr);
CFDataRef root_cert_data = CFDataCreate(cf_alloc, root_cert_ptr->buffer, root_cert_ptr->len);
CFDataRef root_cert_data = CFDataCreate(cf_alloc, root_cert_ptr->data.buffer, root_cert_ptr->data.len);

if (root_cert_data) {
certificate_ref = SecCertificateCreateWithData(cf_alloc, root_cert_data);
CFRelease(root_cert_data);
}

aws_cert_chain_clean_up(&cert_chain_list);
aws_pem_objects_clean_up(&cert_chain_list);
aws_array_list_clean_up(&cert_chain_list);
} else {
certificate_ref = (SecCertificateRef)CFArrayGetValueAtIndex(cert_import_output, 0);
@@ -316,13 +321,15 @@ int aws_import_trusted_certificates(
CFAllocatorRef cf_alloc,
const struct aws_byte_cursor *certificates_blob,
CFArrayRef *certs) {
AWS_PRECONDITION(certificates_blob != NULL);

struct aws_array_list certificates;

if (aws_array_list_init_dynamic(&certificates, alloc, 2, sizeof(struct aws_byte_buf))) {
if (aws_array_list_init_dynamic(&certificates, alloc, 2, sizeof(struct aws_pem_object))) {
return AWS_OP_ERR;
}

if (aws_decode_pem_to_buffer_list(alloc, certificates_blob, &certificates)) {
if (aws_decode_pem_to_object_list(alloc, *certificates_blob, &certificates)) {
AWS_LOGF_ERROR(AWS_LS_IO_PKI, "static: decoding CA PEM failed.");
aws_array_list_clean_up(&certificates);
return AWS_OP_ERR;
@@ -334,10 +341,10 @@ int aws_import_trusted_certificates(
int err = AWS_OP_SUCCESS;
aws_mutex_lock(&s_sec_mutex);
for (size_t i = 0; i < cert_count; ++i) {
struct aws_byte_buf *byte_buf_ptr = NULL;
aws_array_list_get_at_ptr(&certificates, (void **)&byte_buf_ptr, i);
struct aws_pem_object *pem_object_ptr = NULL;
aws_array_list_get_at_ptr(&certificates, (void **)&pem_object_ptr, i);

CFDataRef cert_blob = CFDataCreate(cf_alloc, byte_buf_ptr->buffer, byte_buf_ptr->len);
CFDataRef cert_blob = CFDataCreate(cf_alloc, pem_object_ptr->data.buffer, pem_object_ptr->data.len);

if (cert_blob) {
SecCertificateRef certificate_ref = SecCertificateCreateWithData(cf_alloc, cert_blob);
@@ -351,7 +358,7 @@ int aws_import_trusted_certificates(
aws_mutex_unlock(&s_sec_mutex);

*certs = temp_cert_array;
aws_cert_chain_clean_up(&certificates);
aws_pem_objects_clean_up(&certificates);
aws_array_list_clean_up(&certificates);
return err;
}
3 changes: 2 additions & 1 deletion source/io.c
Original file line number Diff line number Diff line change
@@ -304,6 +304,7 @@ static struct aws_error_info s_errors[] = {
AWS_DEFINE_ERROR_INFO_IO(
AWS_IO_TLS_ERROR_READ_FAILURE,
"Failure during TLS read."),
AWS_DEFINE_ERROR_INFO_IO(AWS_ERROR_PEM_MALFORMED_OBJECT, "Malformed PEM object encountered."),

};
/* clang-format on */
@@ -341,7 +342,7 @@ static struct aws_log_subject_info s_io_log_subject_infos[] = {
"standard-retry-strategy",
"Subject for standard retry strategy"),
DEFINE_LOG_SUBJECT_INFO(AWS_LS_IO_PKCS11, "pkcs11", "Subject for PKCS#11 library operations"),
};
DEFINE_LOG_SUBJECT_INFO(AWS_LS_IO_PEM, "pem", "Subject for pem operations")};

static struct aws_log_subject_info_list s_io_log_subject_list = {
.subject_list = s_io_log_subject_infos,
Loading