From 6b108606fad3bedd4f30e1bc807a1e9233fcd205 Mon Sep 17 00:00:00 2001 From: Hannes Tschofenig Date: Wed, 28 Dec 2022 18:38:53 +0100 Subject: [PATCH 01/14] Added ability to include the SubjectAltName extension to a CSR Signed-off-by: Hannes Tschofenig --- include/mbedtls/x509_csr.h | 27 +++++++++ library/x509write_csr.c | 73 ++++++++++++++++++++++++ programs/x509/cert_req.c | 113 ++++++++++++++++++++++++++++++++----- 3 files changed, 200 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h index 0c204be067..2ac5afa762 100644 --- a/include/mbedtls/x509_csr.h +++ b/include/mbedtls/x509_csr.h @@ -83,6 +83,19 @@ typedef struct mbedtls_x509write_csr { } mbedtls_x509write_csr; +typedef struct mbedtls_x509_san_node { + int type; /**< Subject Alternative Name types */ + char *name; /**< Value, following the syntax allowed bythe type */ + size_t len; /**< Length of the provided value */ +} +mbedtls_x509_san_node; + +typedef struct mbedtls_x509_san_list { + mbedtls_x509_san_node node; + struct mbedtls_x509_san_list *next; +} +mbedtls_x509_san_list; + #if defined(MBEDTLS_X509_CSR_PARSE_C) /** * \brief Load a Certificate Signing Request (CSR) in DER format @@ -220,6 +233,20 @@ void mbedtls_x509write_csr_set_md_alg(mbedtls_x509write_csr *ctx, mbedtls_md_typ */ int mbedtls_x509write_csr_set_key_usage(mbedtls_x509write_csr *ctx, unsigned char key_usage); +/** + * \brief Set Subject Alternative Name + * + * \param ctx CSR context to use + * \param san_list List of SAN values + * + * \return 0 if successful, or MBEDTLS_ERR_X509_ALLOC_FAILED + * + * \note Only "dnsName", "uniformResourceIdentifier" and "otherName", + * as defined in RFC 5280, are supported. + */ +int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ctx, + const mbedtls_x509_san_list *san_list); + /** * \brief Set the Netscape Cert Type flags * (e.g. MBEDTLS_X509_NS_CERT_TYPE_SSL_CLIENT | MBEDTLS_X509_NS_CERT_TYPE_EMAIL) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index d8d8e99ff0..1d46fdbdc3 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -85,6 +85,79 @@ int mbedtls_x509write_csr_set_extension(mbedtls_x509write_csr *ctx, critical, val, val_len); } +int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ctx, + const mbedtls_x509_san_list *san_list) +{ + int ret = 0; + size_t sandeep = 0; + const mbedtls_x509_san_list *cur = san_list; + unsigned char *buf; + unsigned char *p; + size_t len; + size_t buflen = 0; + + /* Determine the maximum size of the SubjectAltName list */ + while (cur != NULL) { + if (cur->node.len <= 0) { + return 0; + } + + /* Calculate size of the required buffer: + * + length of value for each name entry, + * + maximum 4 bytes for the length field, + * + 1 byte for the tag/type. + */ + buflen += cur->node.len + 4 + 1; + + cur = cur->next; + } + + /* Add the extra length field and tag */ + buflen += 4 + 1; + + /* Allocate buffer */ + buf = mbedtls_calloc(1, buflen); + if (buf == NULL) { + return MBEDTLS_ERR_ASN1_ALLOC_FAILED; + } + + mbedtls_platform_zeroize(buf, buflen); + p = buf + buflen; + + /* Write ASN.1-based structure */ + cur = san_list; + len = 0; + while (cur != NULL) { + MBEDTLS_ASN1_CHK_ADD(len, + mbedtls_asn1_write_raw_buffer(&p, buf, + (const unsigned char *) cur->node.name, + cur->node.len)); + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, cur->node.len)); + MBEDTLS_ASN1_CHK_ADD(len, + mbedtls_asn1_write_tag(&p, buf, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + cur->node.type)); + + cur = cur->next; + } + + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, len)); + MBEDTLS_ASN1_CHK_ADD(len, + mbedtls_asn1_write_tag(&p, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)); + + ret = mbedtls_x509write_csr_set_extension( + ctx, + MBEDTLS_OID_SUBJECT_ALT_NAME, + MBEDTLS_OID_SIZE(MBEDTLS_OID_SUBJECT_ALT_NAME), + 0, + buf + buflen - len, + len); + + mbedtls_free(buf); + return ret; +} + int mbedtls_x509write_csr_set_key_usage(mbedtls_x509write_csr *ctx, unsigned char key_usage) { unsigned char buf[4] = { 0 }; diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 8ef59325ef..db80be3430 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -63,6 +63,11 @@ int main(void) " debug_level=%%d default: 0 (disabled)\n" \ " output_file=%%s default: cert.req\n" \ " subject_name=%%s default: CN=Cert,O=mbed TLS,C=UK\n" \ + " san=%%s default: (none)\n" \ + " Comma-separated-list of values:\n" \ + " DNS:value\n" \ + " URI:value\n" \ + " OTHER:value\n" \ " key_usage=%%s default: (empty)\n" \ " Comma-separated-list of values:\n" \ " digital_signature\n" \ @@ -96,16 +101,17 @@ int main(void) * global options */ struct options { - const char *filename; /* filename of the key file */ - const char *password; /* password for the key file */ - int debug_level; /* level of debugging */ - const char *output_file; /* where to store the constructed key file */ - const char *subject_name; /* subject name for certificate request */ - unsigned char key_usage; /* key usage flags */ - int force_key_usage; /* Force adding the KeyUsage extension */ - unsigned char ns_cert_type; /* NS cert type */ - int force_ns_cert_type; /* Force adding NsCertType extension */ - mbedtls_md_type_t md_alg; /* Hash algorithm used for signature. */ + const char *filename; /* filename of the key file */ + const char *password; /* password for the key file */ + int debug_level; /* level of debugging */ + const char *output_file; /* where to store the constructed key file */ + const char *subject_name; /* subject name for certificate request */ + mbedtls_x509_san_list *san_list; /* subjectAltName for certificate request */ + unsigned char key_usage; /* key usage flags */ + int force_key_usage; /* Force adding the KeyUsage extension */ + unsigned char ns_cert_type; /* NS cert type */ + int force_ns_cert_type; /* Force adding NsCertType extension */ + mbedtls_md_type_t md_alg; /* Hash algorithm used for signature. */ } opt; int write_certificate_request(mbedtls_x509write_csr *req, const char *output_file, @@ -145,11 +151,12 @@ int main(int argc, char *argv[]) mbedtls_pk_context key; char buf[1024]; int i; - char *p, *q, *r; + char *p, *q, *r, *r2; mbedtls_x509write_csr req; mbedtls_entropy_context entropy; mbedtls_ctr_drbg_context ctr_drbg; const char *pers = "csr example app"; + mbedtls_x509_san_list *cur, *prev; /* * Set to sane values @@ -175,6 +182,7 @@ usage: opt.ns_cert_type = DFL_NS_CERT_TYPE; opt.force_ns_cert_type = DFL_FORCE_NS_CERT_TYPE; opt.md_alg = DFL_MD_ALG; + opt.san_list = NULL; for (i = 1; i < argc; i++) { @@ -197,6 +205,52 @@ usage: } } else if (strcmp(p, "subject_name") == 0) { opt.subject_name = q; + } else if (strcmp(p, "san") == 0) { + prev = NULL; + + while (q != NULL) { + if ((r = strchr(q, ',')) != NULL) { + *r++ = '\0'; + } + + cur = mbedtls_calloc(1, sizeof(mbedtls_x509_san_list)); + if (cur == NULL) { + mbedtls_printf("Not enough memory for subjectAltName list\n"); + goto usage; + } + + cur->next = NULL; + + if ((r2 = strchr(q, ':')) != NULL) { + *r2++ = '\0'; + } + + if (strcmp(q, "URI") == 0) { + cur->node.type = MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER; + } else if (strcmp(q, "DNS") == 0) { + cur->node.type = MBEDTLS_X509_SAN_DNS_NAME; + } else if (strcmp(q, "OTHER") == 0) { + cur->node.type = MBEDTLS_X509_SAN_OTHER_NAME; + } else { + mbedtls_free(cur); + goto usage; + } + + q = r2; + + cur->node.name = q; + cur->node.len = strlen(q); + + if (prev == NULL) { + opt.san_list = cur; + } else { + prev->next = cur; + } + + prev = cur; + q = r; + } + } else if (strcmp(p, "md") == 0) { const mbedtls_md_info_t *md_info = mbedtls_md_info_from_string(q); @@ -274,14 +328,39 @@ usage: } } + /* Set the MD algorithm to use for the signature in the CSR */ mbedtls_x509write_csr_set_md_alg(&req, opt.md_alg); + /* Set the Key Usage Extension flags in the CSR */ if (opt.key_usage || opt.force_key_usage == 1) { - mbedtls_x509write_csr_set_key_usage(&req, opt.key_usage); + ret = mbedtls_x509write_csr_set_key_usage(&req, opt.key_usage); + + if (ret != 0) { + mbedtls_printf(" failed\n ! mbedtls_x509write_csr_set_key_usage returned %d", ret); + goto exit; + } } + /* Set the Cert Type flags in the CSR */ if (opt.ns_cert_type || opt.force_ns_cert_type == 1) { - mbedtls_x509write_csr_set_ns_cert_type(&req, opt.ns_cert_type); + ret = mbedtls_x509write_csr_set_ns_cert_type(&req, opt.ns_cert_type); + + if (ret != 0) { + mbedtls_printf(" failed\n ! mbedtls_x509write_csr_set_ns_cert_type returned %d", ret); + goto exit; + } + } + + /* Set the SubjectAltName in the CSR */ + if (opt.san_list != NULL) { + ret = mbedtls_x509write_csr_set_subject_alternative_name(&req, opt.san_list); + + if (ret != 0) { + mbedtls_printf( + " failed\n ! mbedtls_x509write_csr_set_subject_alternative_name returned %d", + ret); + goto exit; + } } /* @@ -363,6 +442,14 @@ exit: mbedtls_ctr_drbg_free(&ctr_drbg); mbedtls_entropy_free(&entropy); + cur = opt.san_list; + while (cur != NULL) { + prev = cur; + cur = cur->next; + mbedtls_free(prev); + } + + mbedtls_exit(exit_code); } #endif /* MBEDTLS_X509_CSR_WRITE_C && MBEDTLS_PK_PARSE_C && MBEDTLS_FS_IO && From 18904acc9339189d2bdc0aceab0d50fbfdc3479a Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 14 Feb 2023 11:54:37 +0100 Subject: [PATCH 02/14] Adapt the code to support SAN types: uniformResourceIdentifier, dNSName and IPAddress According to documentation OPCUA requires: uniformResourceIdentifier, dNSName and IPAddress https://reference.opcfoundation.org/Core/Part6/v105/docs/6.2 Signed-off-by: Przemek Stekiel --- library/x509write_csr.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index 1d46fdbdc3..68930b627c 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -128,16 +128,25 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct cur = san_list; len = 0; while (cur != NULL) { - MBEDTLS_ASN1_CHK_ADD(len, - mbedtls_asn1_write_raw_buffer(&p, buf, - (const unsigned char *) cur->node.name, - cur->node.len)); - MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, cur->node.len)); - MBEDTLS_ASN1_CHK_ADD(len, - mbedtls_asn1_write_tag(&p, buf, - MBEDTLS_ASN1_CONTEXT_SPECIFIC | - cur->node.type)); - + switch (cur->node.type) { + case MBEDTLS_X509_SAN_DNS_NAME: + case MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER: + case MBEDTLS_X509_SAN_IP_ADDRESS: + MBEDTLS_ASN1_CHK_ADD(len, + mbedtls_asn1_write_raw_buffer(&p, buf, + (const unsigned char *) cur->node + .name, + cur->node.len)); + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, cur->node.len)); + MBEDTLS_ASN1_CHK_ADD(len, + mbedtls_asn1_write_tag(&p, buf, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + cur->node.type)); + break; + default: + /* Skip unsupported names. */ + break; + } cur = cur->next; } From 8e83d3aaa90452e1f82f6cda85aebc1f97106ebd Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 14 Feb 2023 12:01:16 +0100 Subject: [PATCH 03/14] Add tests for writting SAN to CSR Signed-off-by: Przemek Stekiel --- tests/data_files/Makefile | 2 +- tests/data_files/server1.req.sha256.ext | 19 +++++++++--------- tests/suites/test_suite_x509write.function | 23 ++++++++++++++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 7f39d318d4..61ed4de42f 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -1003,7 +1003,7 @@ all_final += server1.req.sha256 server1.req.sha256.ext: server1.key # Generating this with OpenSSL as a comparison point to test we're getting the same result - openssl req -new -out $@ -key $< -subj '/C=NL/O=PolarSSL/CN=PolarSSL Server 1' -sha256 -addext "extendedKeyUsage=serverAuth" + openssl req -new -out $@ -key $< -subj '/C=NL/O=PolarSSL/CN=PolarSSL Server 1' -sha256 -addext "extendedKeyUsage=serverAuth" -addext "subjectAltName=URI:http://pki.example.com/,IP:127.1.1.0,DNS:example.com" all_final += server1.req.sha256.ext server1.req.sha384: server1.key diff --git a/tests/data_files/server1.req.sha256.ext b/tests/data_files/server1.req.sha256.ext index 3f26f09ef0..c5ff5c5731 100644 --- a/tests/data_files/server1.req.sha256.ext +++ b/tests/data_files/server1.req.sha256.ext @@ -1,17 +1,18 @@ -----BEGIN CERTIFICATE REQUEST----- -MIICpzCCAY8CAQAwPDELMAkGA1UEBhMCTkwxETAPBgNVBAoMCFBvbGFyU1NMMRow +MIIC3jCCAcYCAQAwPDELMAkGA1UEBhMCTkwxETAPBgNVBAoMCFBvbGFyU1NMMRow GAYDVQQDDBFQb2xhclNTTCBTZXJ2ZXIgMTCCASIwDQYJKoZIhvcNAQEBBQADggEP ADCCAQoCggEBAKkCHz1AatVVU4v9Nu6CZS4VYV6Jv7joRZDb7ogWUtPxQ1BHlhJZ ZIdr/SvgRvlzvt3PkuGRW+1moG+JKXlFgNCDatVBQ3dfOXwJBEeCsFc5cO2j7BUZ HqgzCEfBBUKp/UzDtN/dBh9NEFFAZ3MTD0D4bYElXwqxU8YwfhU5rPla7n+SnqYF W+cTl4W1I5LZ1CQG1QkliXUH3aYajz8JGb6tZSxk65Wb3P5BXhem2mxbacwCuhQs FiScStzN0PdSZ3PxLaAj/X70McotcMqJCwTbLqZPcG6ezr1YieJTWZ5uWpJl4og/ -DJQZo93l6J2VE+0p26twEtxaymsXq1KCVLECAwEAAaAmMCQGCSqGSIb3DQEJDjEX -MBUwEwYDVR0lBAwwCgYIKwYBBQUHAwEwDQYJKoZIhvcNAQELBQADggEBAHi0yEGu -Fh5tuLiLuT95UrRnly55+lTY9xchFiKtlcoEdSheybYxqk3JHuSSqojOFKZBlRdk -oG6Azg56/aMHPWyvtCMSRQX4b+FgjeQsm9IfhYNMquQOxyPxm62vjuU3MfZIofXH -hKdI6Ci2CDF4Fyvw50KBWniV38eE9+kjsvDLdXD3ESZJGhjjuFl8ReUiA2wdBTcP -XEZaXUIc6B4tUnlPeqn/2zp4GBqqWzNZx6TXBpApASGG3BEJnM52FVPC7E9p+8YZ -qIGuiF5Cz/rYZkpwffBWIfS2zZakHLm5TB8FgZkWlyReJU9Ihk2Tl/sZ1kllFdYa -xLPnLCL82KFL1Co= +DJQZo93l6J2VE+0p26twEtxaymsXq1KCVLECAwEAAaBdMFsGCSqGSIb3DQEJDjFO +MEwwEwYDVR0lBAwwCgYIKwYBBQUHAwEwNQYDVR0RBC4wLIYXaHR0cDovL3BraS5l +eGFtcGxlLmNvbS+HBH8BAQCCC2V4YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4IB +AQCGmTIXEUvTqwChkzRtxPIQDDchrMnCXgUrTSxre5nvUOpjVlcIIPGWAwxRovfe +pW6OaGZ/3xD0dRAcOW08sTD6GRUazFrubPA1eZiNC7vYdWV59qm84N5yRR/s8Hm+ +okwI47m7W9C0pfaNXchgFUQBn16TrZxPXklbCpBJ/TFV+1ODY0sJPHYiCFpYI+Jz +YuJmadP2BHucl8wv2RyVHywOmV1sDc74i9igVrBCAh8wu+kqImMtrnkGZDxrnj/L +5P1eDfdqG2cN+s40RnMQMosh3UfqpNV/bTgAqBPP2uluT9L1KpWcjZeuvisOgVTq +XwFI5s34fen2DUVw6MWNfbDK -----END CERTIFICATE REQUEST----- diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index cd1f203eaf..e7fc268f0c 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -152,6 +152,27 @@ void x509_csr_check(char *key_file, char *cert_req_check_file, int md_type, int der_len = -1; const char *subject_name = "C=NL,O=PolarSSL,CN=PolarSSL Server 1"; mbedtls_test_rnd_pseudo_info rnd_info; + mbedtls_x509_san_list san_ip; + mbedtls_x509_san_list san_dns; + mbedtls_x509_san_list san_uri; + mbedtls_x509_san_list *san_list = NULL; + const char san_ip_name[] = { 0x7f, 0x01, 0x01, 0x00 }; // 127.1.1.0 + const char *san_dns_name = "example.com"; + const char *san_uri_name = "http://pki.example.com/"; + + san_uri.node.type = MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER; + san_uri.node.name = (char *) san_uri_name; + san_uri.node.len = strlen(san_uri_name); + san_uri.next = NULL; + san_ip.node.type = MBEDTLS_X509_SAN_IP_ADDRESS; + san_ip.node.name = (char *) san_ip_name; + san_ip.node.len = sizeof(san_ip_name); + san_ip.next = &san_uri; + san_dns.node.type = MBEDTLS_X509_SAN_DNS_NAME; + san_dns.node.name = (char *) san_dns_name; + san_dns.node.len = strlen(san_dns_name); + san_dns.next = &san_ip; + san_list = &san_dns; memset(&rnd_info, 0x2a, sizeof(mbedtls_test_rnd_pseudo_info)); @@ -175,6 +196,8 @@ void x509_csr_check(char *key_file, char *cert_req_check_file, int md_type, if (set_extension != 0) { TEST_ASSERT(csr_set_extended_key_usage(&req, MBEDTLS_OID_SERVER_AUTH, MBEDTLS_OID_SIZE(MBEDTLS_OID_SERVER_AUTH)) == 0); + + TEST_ASSERT(mbedtls_x509write_csr_set_subject_alternative_name(&req, san_list) == 0); } ret = mbedtls_x509write_csr_pem(&req, buf, sizeof(buf), From f40de93b1a4a378449d4ff639958ed5ea8442f47 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 14 Feb 2023 13:04:00 +0100 Subject: [PATCH 04/14] Remove redundant variable Signed-off-by: Przemek Stekiel --- library/x509write_csr.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index 68930b627c..c0fe0a8732 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -89,7 +89,6 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct const mbedtls_x509_san_list *san_list) { int ret = 0; - size_t sandeep = 0; const mbedtls_x509_san_list *cur = san_list; unsigned char *buf; unsigned char *p; From 3a92593d1e8689d1fd4cd381fb5a493328137226 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 14 Feb 2023 13:05:24 +0100 Subject: [PATCH 05/14] Adapt cert_req app to support SAN IP Signed-off-by: Przemek Stekiel --- programs/x509/cert_req.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index db80be3430..23e9844cbc 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -67,7 +67,7 @@ int main(void) " Comma-separated-list of values:\n" \ " DNS:value\n" \ " URI:value\n" \ - " OTHER:value\n" \ + " IP:value\n" \ " key_usage=%%s default: (empty)\n" \ " Comma-separated-list of values:\n" \ " digital_signature\n" \ @@ -114,6 +114,19 @@ struct options { mbedtls_md_type_t md_alg; /* Hash algorithm used for signature. */ } opt; +static int ip_string_to_bytes(const char *str, uint8_t *bytes, int maxBytes) +{ + for (int i = 0; i < maxBytes; i++) { + bytes[i] = strtoul(str, NULL, 16); + str = strchr(str, '.'); + if (str == NULL || *str == '\0') { + break; + } + str++; + } + return 0; +} + int write_certificate_request(mbedtls_x509write_csr *req, const char *output_file, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng) @@ -157,6 +170,7 @@ int main(int argc, char *argv[]) mbedtls_ctr_drbg_context ctr_drbg; const char *pers = "csr example app"; mbedtls_x509_san_list *cur, *prev; + uint8_t ip[4]; /* * Set to sane values @@ -229,17 +243,22 @@ usage: cur->node.type = MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER; } else if (strcmp(q, "DNS") == 0) { cur->node.type = MBEDTLS_X509_SAN_DNS_NAME; - } else if (strcmp(q, "OTHER") == 0) { - cur->node.type = MBEDTLS_X509_SAN_OTHER_NAME; + } else if (strcmp(q, "IP") == 0) { + cur->node.type = MBEDTLS_X509_SAN_IP_ADDRESS; + ip_string_to_bytes(r2, ip, 4); } else { mbedtls_free(cur); goto usage; } - q = r2; - - cur->node.name = q; - cur->node.len = strlen(q); + if (strcmp(q, "IP") == 0) { + cur->node.name = (char *) ip; + cur->node.len = sizeof(ip); + } else { + q = r2; + cur->node.name = q; + cur->node.len = strlen(q); + } if (prev == NULL) { opt.san_list = cur; From 5a49d3cce370822d9c81f4df4c1a3a3fec6723c2 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Fri, 24 Feb 2023 13:12:55 +0100 Subject: [PATCH 06/14] Replace mbedtls_x509_san_node with mbedtls_x509_subject_alternative_name Signed-off-by: Przemek Stekiel --- include/mbedtls/x509_csr.h | 9 +------ library/x509write_csr.c | 31 +++++++++++++--------- programs/x509/cert_req.c | 8 +++--- tests/suites/test_suite_x509write.function | 12 ++++----- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/include/mbedtls/x509_csr.h b/include/mbedtls/x509_csr.h index 2ac5afa762..0ac844fc9e 100644 --- a/include/mbedtls/x509_csr.h +++ b/include/mbedtls/x509_csr.h @@ -83,15 +83,8 @@ typedef struct mbedtls_x509write_csr { } mbedtls_x509write_csr; -typedef struct mbedtls_x509_san_node { - int type; /**< Subject Alternative Name types */ - char *name; /**< Value, following the syntax allowed bythe type */ - size_t len; /**< Length of the provided value */ -} -mbedtls_x509_san_node; - typedef struct mbedtls_x509_san_list { - mbedtls_x509_san_node node; + mbedtls_x509_subject_alternative_name node; struct mbedtls_x509_san_list *next; } mbedtls_x509_san_list; diff --git a/library/x509write_csr.c b/library/x509write_csr.c index c0fe0a8732..a1a1206c0b 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -26,6 +26,7 @@ #if defined(MBEDTLS_X509_CSR_WRITE_C) +#include "mbedtls/x509.h" #include "mbedtls/x509_csr.h" #include "mbedtls/asn1write.h" #include "mbedtls/error.h" @@ -97,16 +98,23 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct /* Determine the maximum size of the SubjectAltName list */ while (cur != NULL) { - if (cur->node.len <= 0) { - return 0; + /* Calculate size of the required buffer */ + switch(cur->node.type) { + case MBEDTLS_X509_SAN_DNS_NAME: + case MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER: + case MBEDTLS_X509_SAN_IP_ADDRESS: + /* + length of value for each name entry, + * + maximum 4 bytes for the length field, + * + 1 byte for the tag/type. + */ + buflen += cur->node.san.unstructured_name.len + 4 + 1; + break; + + default: + /* Not supported - skip. */ + break; } - /* Calculate size of the required buffer: - * + length of value for each name entry, - * + maximum 4 bytes for the length field, - * + 1 byte for the tag/type. - */ - buflen += cur->node.len + 4 + 1; cur = cur->next; } @@ -133,10 +141,9 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct case MBEDTLS_X509_SAN_IP_ADDRESS: MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_raw_buffer(&p, buf, - (const unsigned char *) cur->node - .name, - cur->node.len)); - MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, cur->node.len)); + (const unsigned char *) cur->node.san.unstructured_name.p, + cur->node.san.unstructured_name.len)); + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, cur->node.san.unstructured_name.len)); MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_tag(&p, buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 23e9844cbc..1588be164c 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -252,12 +252,12 @@ usage: } if (strcmp(q, "IP") == 0) { - cur->node.name = (char *) ip; - cur->node.len = sizeof(ip); + cur->node.san.unstructured_name.p = (unsigned char *) ip; + cur->node.san.unstructured_name.len = sizeof(ip); } else { q = r2; - cur->node.name = q; - cur->node.len = strlen(q); + cur->node.san.unstructured_name.p = (unsigned char *) q; + cur->node.san.unstructured_name.len = strlen(q); } if (prev == NULL) { diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index e7fc268f0c..5e8230f379 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -161,16 +161,16 @@ void x509_csr_check(char *key_file, char *cert_req_check_file, int md_type, const char *san_uri_name = "http://pki.example.com/"; san_uri.node.type = MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER; - san_uri.node.name = (char *) san_uri_name; - san_uri.node.len = strlen(san_uri_name); + san_uri.node.san.unstructured_name.p = (unsigned char *) san_uri_name; + san_uri.node.san.unstructured_name.len = strlen(san_uri_name); san_uri.next = NULL; san_ip.node.type = MBEDTLS_X509_SAN_IP_ADDRESS; - san_ip.node.name = (char *) san_ip_name; - san_ip.node.len = sizeof(san_ip_name); + san_ip.node.san.unstructured_name.p = (unsigned char *) san_ip_name; + san_ip.node.san.unstructured_name.len = sizeof(san_ip_name); san_ip.next = &san_uri; san_dns.node.type = MBEDTLS_X509_SAN_DNS_NAME; - san_dns.node.name = (char *) san_dns_name; - san_dns.node.len = strlen(san_dns_name); + san_dns.node.san.unstructured_name.p = (unsigned char *) san_dns_name; + san_dns.node.san.unstructured_name.len = strlen(san_dns_name); san_dns.next = &san_ip; san_list = &san_dns; From 57207711d8287e1c283797faeb402a1c16d8fc4b Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Fri, 24 Feb 2023 14:03:30 +0100 Subject: [PATCH 07/14] Add MBEDTLS_ASN1_CHK_CLEANUP_ADD macro to be able to release memory on failure Signed-off-by: Przemek Stekiel --- include/mbedtls/asn1write.h | 9 +++++++++ library/x509write_csr.c | 35 +++++++++++++++++++++-------------- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/include/mbedtls/asn1write.h b/include/mbedtls/asn1write.h index acfc073915..da7375962c 100644 --- a/include/mbedtls/asn1write.h +++ b/include/mbedtls/asn1write.h @@ -35,6 +35,15 @@ (g) += ret; \ } while (0) +#define MBEDTLS_ASN1_CHK_CLEANUP_ADD(g, f) \ + do \ + { \ + if ((ret = (f)) < 0) \ + goto cleanup; \ + else \ + (g) += ret; \ + } while (0) + #ifdef __cplusplus extern "C" { #endif diff --git a/library/x509write_csr.c b/library/x509write_csr.c index a1a1206c0b..45e9187766 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -99,7 +99,7 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct /* Determine the maximum size of the SubjectAltName list */ while (cur != NULL) { /* Calculate size of the required buffer */ - switch(cur->node.type) { + switch (cur->node.type) { case MBEDTLS_X509_SAN_DNS_NAME: case MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER: case MBEDTLS_X509_SAN_IP_ADDRESS: @@ -139,15 +139,20 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct case MBEDTLS_X509_SAN_DNS_NAME: case MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER: case MBEDTLS_X509_SAN_IP_ADDRESS: - MBEDTLS_ASN1_CHK_ADD(len, - mbedtls_asn1_write_raw_buffer(&p, buf, - (const unsigned char *) cur->node.san.unstructured_name.p, - cur->node.san.unstructured_name.len)); - MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, cur->node.san.unstructured_name.len)); - MBEDTLS_ASN1_CHK_ADD(len, - mbedtls_asn1_write_tag(&p, buf, - MBEDTLS_ASN1_CONTEXT_SPECIFIC | - cur->node.type)); + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, + mbedtls_asn1_write_raw_buffer(&p, buf, + (const unsigned char *) + cur->node.san. + unstructured_name.p, + cur->node.san. + unstructured_name.len)); + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_len(&p, buf, + cur->node.san. + unstructured_name.len)); + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, + mbedtls_asn1_write_tag(&p, buf, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + cur->node.type)); break; default: /* Skip unsupported names. */ @@ -156,10 +161,11 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct cur = cur->next; } - MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&p, buf, len)); - MBEDTLS_ASN1_CHK_ADD(len, - mbedtls_asn1_write_tag(&p, buf, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)); + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_len(&p, buf, len)); + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, + mbedtls_asn1_write_tag(&p, buf, + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE)); ret = mbedtls_x509write_csr_set_extension( ctx, @@ -169,6 +175,7 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct buf + buflen - len, len); +cleanup: mbedtls_free(buf); return ret; } From 6cb59c55c32fbd077dfd2db6732f116fd2484aea Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Mon, 6 Mar 2023 09:57:16 +0100 Subject: [PATCH 08/14] ip_string_to_bytes: remove status, add info about supported ip version Signed-off-by: Przemek Stekiel --- programs/x509/cert_req.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 1588be164c..74ade34d50 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -67,7 +67,7 @@ int main(void) " Comma-separated-list of values:\n" \ " DNS:value\n" \ " URI:value\n" \ - " IP:value\n" \ + " IP:value (Only IPv4 is supported)\n" \ " key_usage=%%s default: (empty)\n" \ " Comma-separated-list of values:\n" \ " digital_signature\n" \ @@ -114,7 +114,7 @@ struct options { mbedtls_md_type_t md_alg; /* Hash algorithm used for signature. */ } opt; -static int ip_string_to_bytes(const char *str, uint8_t *bytes, int maxBytes) +static void ip_string_to_bytes(const char *str, uint8_t *bytes, int maxBytes) { for (int i = 0; i < maxBytes; i++) { bytes[i] = strtoul(str, NULL, 16); @@ -124,7 +124,6 @@ static int ip_string_to_bytes(const char *str, uint8_t *bytes, int maxBytes) } str++; } - return 0; } int write_certificate_request(mbedtls_x509write_csr *req, const char *output_file, From 07c5ea348c27642976d6adc5756c621a6a981f78 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 7 Mar 2023 15:43:38 +0100 Subject: [PATCH 09/14] Add check for buffer overflow and fix style. Signed-off-by: Przemek Stekiel --- library/x509write_csr.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index 45e9187766..ca0f88ca9d 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -141,14 +141,10 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct case MBEDTLS_X509_SAN_IP_ADDRESS: MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_raw_buffer(&p, buf, - (const unsigned char *) - cur->node.san. - unstructured_name.p, - cur->node.san. - unstructured_name.len)); + (const unsigned char *) cur->node.san.unstructured_name.p, + cur->node.san.unstructured_name.len)); MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_len(&p, buf, - cur->node.san. - unstructured_name.len)); + cur->node.san.unstructured_name.len)); MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_tag(&p, buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | @@ -175,6 +171,12 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct buf + buflen - len, len); + /* If we exceeded the allocated buffer it means that maximum size of the SubjectAltName list + * was incorrectly calculated and memory is corrupted. */ + if ( p < buf ) { + ret = MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; + } + cleanup: mbedtls_free(buf); return ret; From 68ca81c8fea37cd14f743baa532033a93384ee72 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Wed, 8 Mar 2023 15:14:47 +0100 Subject: [PATCH 10/14] Change separator for SAN names to ';' When ';' is used as a separator san names must be provided in quotation marks: ./cert_req filename=../../tests/data_files/server8.key subject_name=dannybackx.hopto.org san="URI:http://pki.example.com/;IP:127.1.1.0;DNS:example.com" Signed-off-by: Przemek Stekiel --- programs/x509/cert_req.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 74ade34d50..8baca81d8a 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -198,13 +198,11 @@ usage: opt.san_list = NULL; for (i = 1; i < argc; i++) { - p = argv[i]; if ((q = strchr(p, '=')) == NULL) { goto usage; } *q++ = '\0'; - if (strcmp(p, "filename") == 0) { opt.filename = q; } else if (strcmp(p, "password") == 0) { @@ -222,7 +220,7 @@ usage: prev = NULL; while (q != NULL) { - if ((r = strchr(q, ',')) != NULL) { + if ((r = strchr(q, ';')) != NULL) { *r++ = '\0'; } From 42510a91c40f0fd1b5708ed55164d14abdc05644 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Thu, 9 Mar 2023 08:18:30 +0100 Subject: [PATCH 11/14] Use for loop instead while loop Signed-off-by: Przemek Stekiel --- library/x509write_csr.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index ca0f88ca9d..a830fc2a7d 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -90,22 +90,22 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct const mbedtls_x509_san_list *san_list) { int ret = 0; - const mbedtls_x509_san_list *cur = san_list; + const mbedtls_x509_san_list *cur; unsigned char *buf; unsigned char *p; size_t len; size_t buflen = 0; /* Determine the maximum size of the SubjectAltName list */ - while (cur != NULL) { + for(cur = san_list; cur != NULL; cur = cur->next) { /* Calculate size of the required buffer */ switch (cur->node.type) { case MBEDTLS_X509_SAN_DNS_NAME: case MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER: case MBEDTLS_X509_SAN_IP_ADDRESS: - /* + length of value for each name entry, - * + maximum 4 bytes for the length field, - * + 1 byte for the tag/type. + /* length of value for each name entry, + * maximum 4 bytes for the length field, + * 1 byte for the tag/type. */ buflen += cur->node.san.unstructured_name.len + 4 + 1; break; @@ -114,9 +114,6 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct /* Not supported - skip. */ break; } - - - cur = cur->next; } /* Add the extra length field and tag */ From 89e268dfb93c40af19f1c712225371f6154a8e53 Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Thu, 9 Mar 2023 08:23:18 +0100 Subject: [PATCH 12/14] Add change log entry (SubjectAltName extension in CSR) Signed-off-by: Przemek Stekiel --- ChangeLog.d/san_csr.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/san_csr.txt diff --git a/ChangeLog.d/san_csr.txt b/ChangeLog.d/san_csr.txt new file mode 100644 index 0000000000..b5c6cf3cbb --- /dev/null +++ b/ChangeLog.d/san_csr.txt @@ -0,0 +1,2 @@ +Features + * Add support to include the SubjectAltName extension to a CSR. From 55ceff6d2f48000d979c3e335816b6cf36b297fb Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Fri, 10 Mar 2023 12:40:41 +0100 Subject: [PATCH 13/14] Code optimization and style fixes Signed-off-by: Przemek Stekiel --- library/x509write_csr.c | 28 +++++++++++++++++----------- programs/x509/cert_req.c | 3 ++- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/library/x509write_csr.c b/library/x509write_csr.c index a830fc2a7d..deb66174b2 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -97,7 +97,7 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct size_t buflen = 0; /* Determine the maximum size of the SubjectAltName list */ - for(cur = san_list; cur != NULL; cur = cur->next) { + for (cur = san_list; cur != NULL; cur = cur->next) { /* Calculate size of the required buffer */ switch (cur->node.type) { case MBEDTLS_X509_SAN_DNS_NAME: @@ -136,17 +136,23 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct case MBEDTLS_X509_SAN_DNS_NAME: case MBEDTLS_X509_SAN_UNIFORM_RESOURCE_IDENTIFIER: case MBEDTLS_X509_SAN_IP_ADDRESS: + { + const unsigned char *unstructured_name = + (const unsigned char *) cur->node.san.unstructured_name.p; + size_t unstructured_name_len = cur->node.san.unstructured_name.len; + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, - mbedtls_asn1_write_raw_buffer(&p, buf, - (const unsigned char *) cur->node.san.unstructured_name.p, - cur->node.san.unstructured_name.len)); - MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_len(&p, buf, - cur->node.san.unstructured_name.len)); + mbedtls_asn1_write_raw_buffer( + &p, buf, + unstructured_name, unstructured_name_len)); + MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, mbedtls_asn1_write_len( + &p, buf, unstructured_name_len)); MBEDTLS_ASN1_CHK_CLEANUP_ADD(len, - mbedtls_asn1_write_tag(&p, buf, - MBEDTLS_ASN1_CONTEXT_SPECIFIC | - cur->node.type)); - break; + mbedtls_asn1_write_tag( + &p, buf, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | cur->node.type)); + } + break; default: /* Skip unsupported names. */ break; @@ -170,7 +176,7 @@ int mbedtls_x509write_csr_set_subject_alternative_name(mbedtls_x509write_csr *ct /* If we exceeded the allocated buffer it means that maximum size of the SubjectAltName list * was incorrectly calculated and memory is corrupted. */ - if ( p < buf ) { + if (p < buf) { ret = MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; } diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 8baca81d8a..2f7126b920 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -169,7 +169,6 @@ int main(int argc, char *argv[]) mbedtls_ctr_drbg_context ctr_drbg; const char *pers = "csr example app"; mbedtls_x509_san_list *cur, *prev; - uint8_t ip[4]; /* * Set to sane values @@ -220,6 +219,8 @@ usage: prev = NULL; while (q != NULL) { + uint8_t ip[4] = { 0 }; + if ((r = strchr(q, ';')) != NULL) { *r++ = '\0'; } From f86fe73d59e3774ee1f1ee80d5ca2b41edde60df Mon Sep 17 00:00:00 2001 From: Przemek Stekiel Date: Tue, 14 Mar 2023 09:55:29 +0100 Subject: [PATCH 14/14] Fix error on Windows builds (conversion from 'unsigned long' to 'uint8_t') Signed-off-by: Przemek Stekiel --- programs/x509/cert_req.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/x509/cert_req.c b/programs/x509/cert_req.c index 2f7126b920..5241438440 100644 --- a/programs/x509/cert_req.c +++ b/programs/x509/cert_req.c @@ -117,7 +117,7 @@ struct options { static void ip_string_to_bytes(const char *str, uint8_t *bytes, int maxBytes) { for (int i = 0; i < maxBytes; i++) { - bytes[i] = strtoul(str, NULL, 16); + bytes[i] = (uint8_t) strtoul(str, NULL, 16); str = strchr(str, '.'); if (str == NULL || *str == '\0') { break;