diff --git a/stuffer/s2n_stuffer_pem.c b/stuffer/s2n_stuffer_pem.c index cf8ae838353..ce01810c86b 100644 --- a/stuffer/s2n_stuffer_pem.c +++ b/stuffer/s2n_stuffer_pem.c @@ -20,8 +20,9 @@ #include "stuffer/s2n_stuffer.h" #include "utils/s2n_safety.h" -#define S2N_PEM_DELIMTER_CHAR '-' -#define S2N_PEM_DELIMITER_MIN_COUNT 1 +#define S2N_PEM_DELIMITER_CHAR '-' +#define S2N_PEM_DELIMITER_TOKEN "--" +#define S2N_PEM_DELIMITER_MIN_COUNT 2 #define S2N_PEM_DELIMITER_MAX_COUNT 64 #define S2N_PEM_BEGIN_TOKEN "BEGIN " #define S2N_PEM_END_TOKEN "END " @@ -36,12 +37,17 @@ static int s2n_stuffer_pem_read_encapsulation_line(struct s2n_stuffer *pem, const char *encap_marker, const char *keyword) { - /* Skip any number of Chars until a "-" is reached */ - POSIX_GUARD(s2n_stuffer_skip_to_char(pem, S2N_PEM_DELIMTER_CHAR)); + /* Skip any number of Chars until a "--" is reached. + * We use "--" instead of "-" to account for dashes that appear in comments. + * We do not accept comments that contain "--". + */ + POSIX_GUARD(s2n_stuffer_skip_read_until(pem, S2N_PEM_DELIMITER_TOKEN)); - /* Ensure between 1 and 64 '-' chars at start of line */ - POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMTER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT, - S2N_PEM_DELIMITER_MAX_COUNT, NULL)); + /* Ensure between 2 and 64 '-' chars at start of line. + * We already read 2 '-' chars when we read the S2N_PEM_DELIMITER_TOKEN. + */ + POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMITER_CHAR, 0, + S2N_PEM_DELIMITER_MAX_COUNT - 2, NULL)); /* Ensure next string in stuffer is "BEGIN " or "END " */ POSIX_GUARD(s2n_stuffer_read_expected_str(pem, encap_marker)); @@ -49,8 +55,8 @@ static int s2n_stuffer_pem_read_encapsulation_line(struct s2n_stuffer *pem, cons /* Ensure next string is stuffer is the keyword (Eg "CERTIFICATE", "PRIVATE KEY", etc) */ POSIX_GUARD(s2n_stuffer_read_expected_str(pem, keyword)); - /* Ensure between 1 and 64 '-' chars at end of line */ - POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMTER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT, + /* Ensure between 2 and 64 '-' chars at end of line */ + POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMITER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT, S2N_PEM_DELIMITER_MAX_COUNT, NULL)); /* Check for missing newline between dashes case: "-----END CERTIFICATE----------BEGIN CERTIFICATE-----" */ @@ -60,7 +66,7 @@ static int s2n_stuffer_pem_read_encapsulation_line(struct s2n_stuffer *pem, cons POSIX_GUARD(s2n_stuffer_rewind_read(pem, 1)); } - /* Skip newlines and other whitepsace that may be after the dashes */ + /* Skip newlines and other whitespace that may be after the dashes */ return s2n_stuffer_skip_whitespace(pem, NULL); } diff --git a/tests/unit/s2n_certificate_parsing_test.c b/tests/unit/s2n_certificate_parsing_test.c new file mode 100644 index 00000000000..0c8b64e30a6 --- /dev/null +++ b/tests/unit/s2n_certificate_parsing_test.c @@ -0,0 +1,444 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include "api/s2n.h" +#include "crypto/s2n_crypto.h" +#include "crypto/s2n_openssl_x509.h" +#include "s2n_test.h" +#include "testlib/s2n_testlib.h" +#include "tls/s2n_tls.h" +#include "utils/s2n_safety.h" + +#define CERTIFICATE_1 \ + "MIIBljCCATygAwIBAgIUHxKbtYzLM4Bct5v5Sagb8aZU/BcwCgYIKoZIzj0EAwIw" \ + "HjELMAkGA1UEBhMCVVMxDzANBgNVBAMMBmJyYW5jaDAgFw0yNDAxMjMwMDU3MTha" \ + "GA8yMjAzMDYzMDAwNTcxOFowHDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBGxlYWYw" \ + "WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASAmJKXt9P8hSz9ubntEokFn06+Rexr" \ + "lEujwWUIq5Kl6QwvtfDCzrJN/sUmM5mssjq7pF6XVr6zFcQ6G4BfnwGio1gwVjAU" \ + "BgNVHREEDTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFNlOWJn7XfzC6xORzzjrdnqK" \ + "UlJwMB8GA1UdIwQYMBaAFFkQhpDCJ3bAbXw1tXpmk5Fi7YIGMAoGCCqGSM49BAMC" \ + "A0gAMEUCIB58OBwIruTJIy1f3tUgM/wXoO7fCoU25sMcioHBV9dYAiEA7Ufxa2JF" \ + "I5LP6RGyllsjjnh0MJy1ZMXhw7X6GqFn4Rw=" + +#define CERTIFICATE_2 \ + "MIIBoDCCAUegAwIBAgIUQud1+tNUPAEBnJLb2Lyl3/vuA4EwCgYIKoZIzj0EAwIw" \ + "HDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBHJvb3QwIBcNMjQwMTIzMDA1NzE4WhgP" \ + "MjIwMzA2MzAwMDU3MThaMB4xCzAJBgNVBAYTAlVTMQ8wDQYDVQQDDAZicmFuY2gw" \ + "WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATfScB9w/LkHBAVXiyKN941555oyBpv" \ + "IZeCNXX+gbSvnS0pNRr35BalgFmij86DaXLl68RrHQsnhZByJvnIplN+o2MwYTAP" \ + "BgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQEAwICBDAdBgNVHQ4EFgQUWRCGkMIn" \ + "dsBtfDW1emaTkWLtggYwHwYDVR0jBBgwFoAUfCtcQDvXYwkR37fHwe/mi7JyQIQw" \ + "CgYIKoZIzj0EAwIDRwAwRAIgaK6aOuCfMwgAASavkZpoxWag49yco4d9AlxIU+rt" \ + "U2UCIHRieWdQICIWSEHdRTXWPPEnOd7A3UmTgoqbMl+Imhy8" + +#define CERTIFICATE_3 \ + "MIIBnzCCAUWgAwIBAgIUe0/XdFLyc4+Sj1NMkvbagE8DFaUwCgYIKoZIzj0EAwIw" \ + "HDELMAkGA1UEBhMCVVMxDTALBgNVBAMMBHJvb3QwIBcNMjQwMTIzMDA1NzE4WhgP" \ + "MjIwMzA2MzAwMDU3MThaMBwxCzAJBgNVBAYTAlVTMQ0wCwYDVQQDDARyb290MFkw" \ + "EwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEPPtDvucc4UGOdIjED2P/vDxVYDhO5P8s" \ + "7lyys3QZpKapMuc9wOV0cQ6tN9h4kVY+FJocYgqDAl2vv6Rg/wSbl6NjMGEwHQYD" \ + "VR0OBBYEFHwrXEA712MJEd+3x8Hv5ouyckCEMB8GA1UdIwQYMBaAFHwrXEA712MJ" \ + "Ed+3x8Hv5ouyckCEMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgIEMAoG" \ + "CCqGSM49BAMCA0gAMEUCIQCcbhKRAsqQCj2pCXh+9og3sLw9q8nU4xAB9xuV3vPA" \ + "FAIgBxFWfRdu09dtE0IUkLTY0WPxiWaYYrKexlD4wUquqJE=" + +#define BEGIN_CERT "BEGIN CERTIFICATE" +#define BEGIN_CERT_LINE "-----" BEGIN_CERT "-----" +#define END_CERT "END CERTIFICATE" +#define END_CERT_LINE "-----" END_CERT "-----" + +static S2N_RESULT s2n_test_validate_cert(struct s2n_cert *cert, struct s2n_blob *expected) +{ + RESULT_ENSURE_REF(cert); + RESULT_ENSURE_REF(expected); + RESULT_ENSURE_EQ(cert->raw.size, expected->size); + RESULT_ENSURE_EQ(memcmp(cert->raw.data, expected->data, expected->size), 0); + return S2N_RESULT_OK; +} + +int main(int argc, char **argv) +{ + BEGIN_TEST(); + + const char *expected_cert_strs[] = { + CERTIFICATE_1, + CERTIFICATE_2, + CERTIFICATE_3, + }; + struct s2n_blob expected_certs[s2n_array_len(expected_cert_strs)] = { 0 }; + + for (size_t i = 0; i < s2n_array_len(expected_certs); i++) { + const char *base64_cert = expected_cert_strs[i]; + struct s2n_blob *out = &expected_certs[i]; + + /* base64 requires more than one character per byte, so the binary + * output will be smaller than the base64 input. + */ + EXPECT_SUCCESS(s2n_alloc(out, strlen(base64_cert))); + + struct s2n_stuffer out_stuffer = { 0 }; + EXPECT_SUCCESS(s2n_stuffer_init(&out_stuffer, out)); + + DEFER_CLEANUP(struct s2n_stuffer in_stuffer = { 0 }, s2n_stuffer_free); + EXPECT_SUCCESS(s2n_stuffer_alloc(&in_stuffer, strlen(base64_cert))); + EXPECT_SUCCESS(s2n_stuffer_write_str(&in_stuffer, base64_cert)); + + EXPECT_SUCCESS(s2n_stuffer_read_base64(&in_stuffer, &out_stuffer)); + out->size = s2n_stuffer_data_available(&out_stuffer); + } + + struct s2n_test_case { + const char *name; + const char *input; + }; + + const struct s2n_test_case test_cases[] = { + { + .name = "basic format", + .input = + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n", + }, + { + .name = "minimal newlines", + .input = + BEGIN_CERT_LINE + CERTIFICATE_1 + END_CERT_LINE "\n" + BEGIN_CERT_LINE + CERTIFICATE_2 + END_CERT_LINE "\n" + BEGIN_CERT_LINE + CERTIFICATE_3 + END_CERT_LINE, + }, + { + .name = "empty lines", + .input = + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n" + "\n", + }, + { + .name = "double empty lines", + .input = + "\n" + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + "\n" + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + "\n" + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n" + "\n" + "\n", + }, + { + .name = "variable number of newlines", + .input = + "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + "\n\n\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n" + "\n\n" + }, + { + .name = "whitespace", + .input = + " " BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE " \n" + " " + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE " \n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE " \n", + }, + { + .name = "extra dashes", + .input = + "-" BEGIN_CERT_LINE "--\n" + CERTIFICATE_1 "\n" + "---" END_CERT_LINE "----\n" + "-----" BEGIN_CERT_LINE "------\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "---------------\n" + "--" BEGIN_CERT_LINE "---\n" + CERTIFICATE_3 "\n" + "-" END_CERT_LINE "-\n", + }, + { + .name = "64 dashes", + .input = + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "----" + BEGIN_CERT + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "----" + "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n", + }, + { + .name = "missing dashes", + .input = + "--" BEGIN_CERT "--\n" + CERTIFICATE_1 "\n" + "---" END_CERT "---\n" + "----" BEGIN_CERT "----\n" + CERTIFICATE_2 "\n" + "--" END_CERT "---\n" + "---" BEGIN_CERT "----\n" + CERTIFICATE_3 "\n" + "----" END_CERT "--\n", + }, + { + .name = "comments", + .input = + "# cert1" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + "\n" + "# cert2" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + "\n" + "# cert3" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n", + }, + { + .name = "comments containing dashes", + .input = + "# cert-1\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + "\n" + "# -c-e-r-t-2-\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + "\n" + "# -\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n", + }, + { + .name = "comments containing other special characters", + .input = + "# cert_1\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + "\n" + "# cert #2\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_2 "\n" + END_CERT_LINE "\n" + "\n" + "# cert !@$%! \n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_3 "\n" + END_CERT_LINE "\n", + }, + }; + + for (size_t i = 0; i < s2n_array_len(test_cases); i++) { + DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain_and_key = s2n_cert_chain_and_key_new(), + s2n_cert_chain_and_key_ptr_free); + const struct s2n_test_case *test_case = &test_cases[i]; + + DEFER_CLEANUP(struct s2n_stuffer input = { 0 }, s2n_stuffer_free); + EXPECT_SUCCESS(s2n_stuffer_alloc(&input, strlen(test_case->input))); + EXPECT_SUCCESS(s2n_stuffer_write_str(&input, test_case->input)); + + struct s2n_cert_chain *cert_chain = chain_and_key->cert_chain; + if (s2n_create_cert_chain_from_stuffer(cert_chain, &input) != S2N_SUCCESS) { + fprintf(stderr, "Failed to parse \"%s\"\n", test_case->name); + FAIL_MSG("Failed to parse certificate chain input"); + } + + struct s2n_cert *cert = cert_chain->head; + for (size_t j = 0; j < s2n_array_len(expected_certs); j++) { + if (s2n_result_is_error(s2n_test_validate_cert(cert, &expected_certs[j]))) { + fprintf(stderr, "\"%s\" failed to parse cert %li\n", test_case->name, j); + FAIL_MSG("Did not correctly read all certificates"); + } + cert = cert->next; + } + } + + const struct s2n_test_case bad_test_cases[] = { + { + .name = "double begin marker", + .input = + BEGIN_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "double end marker", + .input = + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + END_CERT_LINE "\n" + }, + { + .name = "missing begin marker", + .input = + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "missing end marker", + .input = + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + }, + { + .name = "no dashes before marker", + .input = + BEGIN_CERT "-----\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "no dashes after marker", + .input = + "-----" BEGIN_CERT "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "single dash before marker", + .input = + "-" BEGIN_CERT "------\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "single dash after marker", + .input = + "------" BEGIN_CERT "-\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "65 dashes before marker", + .input = + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "-----" + BEGIN_CERT "-----\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "65 dashes after marker", + .input = + "-----" BEGIN_CERT + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "-----" "-----" "-----" "-----" + "-----" "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "dashes in comment", + .input = + "# --cert--\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + { + .name = "delimiter in comment", + .input = + "# " BEGIN_CERT_LINE "\n" + BEGIN_CERT_LINE "\n" + CERTIFICATE_1 "\n" + END_CERT_LINE "\n" + }, + }; + for (size_t i = 0; i < s2n_array_len(bad_test_cases); i++) { + DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain_and_key = s2n_cert_chain_and_key_new(), + s2n_cert_chain_and_key_ptr_free); + const struct s2n_test_case *test_case = &bad_test_cases[i]; + + DEFER_CLEANUP(struct s2n_stuffer test_input = { 0 }, s2n_stuffer_free); + EXPECT_SUCCESS(s2n_stuffer_alloc(&test_input, strlen(test_case->input))); + EXPECT_SUCCESS(s2n_stuffer_write_str(&test_input, test_case->input)); + + struct s2n_cert_chain *cert_chain = chain_and_key->cert_chain; + if (s2n_create_cert_chain_from_stuffer(cert_chain, &test_input) == S2N_SUCCESS) { + fprintf(stderr, "Successfully parsed invalid cert chain \"%s\"\n", test_case->name); + FAIL_MSG("Successfully parsed invalid cert chain"); + }; + } + + END_TEST(); +} diff --git a/utils/s2n_safety.h b/utils/s2n_safety.h index d0ac5f2fc1a..ec624d44cdb 100644 --- a/utils/s2n_safety.h +++ b/utils/s2n_safety.h @@ -97,7 +97,12 @@ S2N_CLEANUP_RESULT s2n_connection_apply_error_blinding(struct s2n_connection** c } \ struct __useless_struct_to_allow_trailing_semicolon__ -#define s2n_array_len(array) ((array != NULL) ? (sizeof(array) / sizeof(array[0])) : 0) +/* Careful: this method works on ARRAYS, not POINTERS. + * You can call sizeof to determine the size of an array declared on the stack. + * However, once an array is passed to another function, it is treated as a pointer + * and sizeof will return the size of the memory address, not the size of the array. + */ +#define s2n_array_len(array) (sizeof(array) / sizeof(array[0])) int s2n_mul_overflow(uint32_t a, uint32_t b, uint32_t* out);