From: Bob Beck Date: Mon, 11 May 2026 16:30:04 +0000 (-0600) Subject: Prepare a now opaque ASN1_STRING for the size_t rapture. X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=28179061bfcdfe579f63a129bf0b97766a5d90a7;p=thirdparty%2Fopenssl.git Prepare a now opaque ASN1_STRING for the size_t rapture. Now that ASN1_STRING is opaque, we can finally move away from an int for the length internally. The remaining problematic piece for this is that ASN1_STRING_length() returns an int and is public API. Therefore, we deprecate ASN1_STRING_length() and provide a replacement ASN1_STRING_length_ex() that returns a size_t length. We also provide setting functions that take size_t lengths, they are ASN1_STRING_set_data() which takes a uint8_t data pointer and a size_t length, and ASN1_STRING_set_string() which takes a argument that must be a c string and will use strlen to determine the length. (This replaces th previous arcane behaviour of calling "strlen" on a magical input length value of -1, which leads to bugs.) We then deprecate ASN1_STRING_set(). ASN1_STRING_set_string() requires a valid C string argument that may not be NULL - refer to the documentation. Both new functions do not magically add 0 bytes on the end of values, as ASN1_STRING has already been documented for a long time to not depend on this behaviour. Both new functions do not allow the setting of values on an ASN1_BIT_STRING, as ASN1_BIT_STRING_set1 must be used for that. Note that this does *NOT* yet change ASN1_STRING to use size_t internally, this must wait until the integer-returning ASN1_STRING_length() has been deprecated, and then removed in future major. Once ASN1_STRING_length() has been removed then ASN1_STRING internally can change to using a size_t for the length of the data. (And the setters will no longer return an error if the provided size_t length exceeds INT_MAX) Reviewed-by: Milan Broz Reviewed-by: Norbert Pocs MergeDate: Sat Jul 18 13:01:15 2026 (Merged from https://github.com/openssl/openssl/pull/31194) --- diff --git a/crypto/asn1/a_octet.c b/crypto/asn1/a_octet.c index 4efb8ec5178..99df7539a14 100644 --- a/crypto/asn1/a_octet.c +++ b/crypto/asn1/a_octet.c @@ -25,5 +25,11 @@ int ASN1_OCTET_STRING_cmp(const ASN1_OCTET_STRING *a, int ASN1_OCTET_STRING_set(ASN1_OCTET_STRING *x, const unsigned char *d, int len) { - return ASN1_STRING_set(x, d, len); + if (len < -1) { + ERR_raise(ERR_LIB_ASN1, ASN1_R_TOO_SMALL); + return 0; + } + if (len == -1) + return ASN1_STRING_set_string(x, (const char *)d); + return ASN1_STRING_set_data(x, d, len); } diff --git a/crypto/asn1/asn1_lib.c b/crypto/asn1/asn1_lib.c index 99903564e25..2bde572eaf8 100644 --- a/crypto/asn1/asn1_lib.c +++ b/crypto/asn1/asn1_lib.c @@ -265,7 +265,8 @@ int ASN1_STRING_copy(ASN1_STRING *dst, const ASN1_STRING *str) if (str == NULL) return 0; dst->type = str->type; - if (!ASN1_STRING_set(dst, str->data, str->length)) + if (!ossl_asn1_string_set_internal(dst, str->data, str->length, + /*add_nul_byte=*/0)) return 0; /* Copy flags but preserve embed value */ dst->flags &= ASN1_STRING_FLAG_EMBED; @@ -289,12 +290,18 @@ ASN1_STRING *ASN1_STRING_dup(const ASN1_STRING *str) return ret; } -int ASN1_STRING_set(ASN1_STRING *str, const void *_data, int len_in) +int ossl_asn1_string_set_internal(ASN1_STRING *str, const uint8_t *data, + int len_in, int add_nul_byte) { - unsigned char *c; - const char *data = _data; - size_t len; + size_t len, alloc_len; +#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION + /* + * Force no NUL byte for callers that are requesting it + * 0 length object data will be NULL + */ + add_nul_byte = 0; +#endif if (len_in < -1) { ERR_raise(ERR_LIB_ASN1, ASN1_R_TOO_SMALL); return 0; @@ -302,16 +309,17 @@ int ASN1_STRING_set(ASN1_STRING *str, const void *_data, int len_in) if (len_in == -1) { if (data == NULL) return 0; - len = strlen(data); + len = strlen((const char *)data); } else { len = (size_t)len_in; } /* - * Verify that the length fits within an integer for assignment to - * str->length below. The additional 1 is subtracted to allow for the - * '\0' terminator even though this isn't strictly necessary. + * Add one to the length to allow for adding an a '\0' terminator + * "even though this isn't strictly necessary". */ - if (len > INT_MAX - 1) { + alloc_len = add_nul_byte ? len + 1 : len; + + if (alloc_len > INT_MAX) { ERR_raise(ERR_LIB_ASN1, ASN1_R_TOO_LARGE); return 0; } @@ -322,39 +330,47 @@ int ASN1_STRING_set(ASN1_STRING *str, const void *_data, int len_in) str->flags &= ~ASN1_STRING_FLAG_DATA_NOT_OWNED; } - if ((size_t)str->length <= len || str->data == NULL) { - c = str->data; -#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION - /* No NUL terminator in fuzzing builds */ - str->data = OPENSSL_realloc(c, len != 0 ? len : 1); -#else - str->data = OPENSSL_realloc(c, len + 1); -#endif - if (str->data == NULL) { - str->data = c; + /* Ensure copying a 0 length data field is defined. */ + if (alloc_len == 0) { + OPENSSL_free(str->data); + str->data = NULL; + str->length = 0; + return 1; + } + + if ((size_t)str->length != alloc_len) { + uint8_t *c; + c = OPENSSL_realloc(str->length == 0 ? NULL : str->data, alloc_len); + if (c == NULL) return 0; - } + str->data = c; } + /* length never includes the added \0 byte */ str->length = (int)len; - if (data != NULL) { + + if (data != NULL && str->data != NULL) { memcpy(str->data, data, len); -#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION - /* Set the unused byte to something non NUL and printable. */ - if (len == 0) - str->data[len] = '~'; -#else - /* - * Add a NUL terminator. This should not be necessary - but we add it as - * a safety precaution - */ - str->data[len] = '\0'; -#endif + if (add_nul_byte) { + /* + * Add a '\0' terminator. This should not be necessary - but we add it as + * a safety precaution + */ + str->data[len] = '\0'; + } } ossl_asn1_bit_string_clear_unused_bits(str); return 1; } +#ifndef OPENSSL_NO_DEPRECATED_4_1 +int ASN1_STRING_set(ASN1_STRING *str, const void *_data, int len_in) +{ + return ossl_asn1_string_set_internal(str, (const uint8_t *)_data, len_in, + /*add_nul_byte=*/1); +} +#endif + void ASN1_STRING_set0(ASN1_STRING *str, void *data, int len) { if (!(str->flags & ASN1_STRING_FLAG_DATA_NOT_OWNED)) { @@ -365,6 +381,26 @@ void ASN1_STRING_set0(ASN1_STRING *str, void *data, int len) str->length = len; } +int ASN1_STRING_set_data(ASN1_STRING *str, const uint8_t *data, size_t len_in) +{ + if (str->type == V_ASN1_BIT_STRING) { + ERR_raise(ERR_LIB_ASN1, ASN1_R_ILLEGAL_BITSTRING_FORMAT); + return 0; + } + /* This will go away once ASN1_STRING can size_t internally */ + if (len_in > INT_MAX) { + ERR_raise(ERR_LIB_ASN1, ASN1_R_TOO_LARGE); + return 0; + } + return ossl_asn1_string_set_internal(str, data, (int)len_in, /*add_nul_byte=*/0); +} + +int ASN1_STRING_set_string(ASN1_STRING *str, const char *c_string) +{ + return ASN1_STRING_set_data(str, (const uint8_t *)c_string, + strlen(c_string)); +} + ASN1_STRING *ASN1_STRING_new(void) { return ASN1_STRING_type_new(V_ASN1_OCTET_STRING); @@ -469,10 +505,17 @@ int ASN1_STRING_cmp(const ASN1_STRING *a, const ASN1_STRING *b) } } +#ifndef OPENSSL_NO_DEPRECATED_4_1 int ASN1_STRING_length(const ASN1_STRING *x) { return x->length; } +#endif + +size_t ASN1_STRING_length_ex(const ASN1_STRING *x) +{ + return (size_t)x->length; +} #ifndef OPENSSL_NO_DEPRECATED_3_0 void ASN1_STRING_length_set(ASN1_STRING *x, int len) diff --git a/doc/man3/ASN1_STRING_length.pod b/doc/man3/ASN1_STRING_length.pod index 47cacb253a0..5b047f084a8 100644 --- a/doc/man3/ASN1_STRING_length.pod +++ b/doc/man3/ASN1_STRING_length.pod @@ -2,6 +2,7 @@ =head1 NAME +ASN1_STRING_set_data, ASN1_STRING_set_string, ASN1_STRING_length_ex, ASN1_STRING_dup, ASN1_STRING_cmp, ASN1_STRING_set, ASN1_STRING_length, ASN1_STRING_type, ASN1_STRING_get0_data, ASN1_STRING_to_UTF8 - ASN1_STRING utility functions @@ -10,19 +11,30 @@ ASN1_STRING_to_UTF8 - ASN1_STRING utility functions #include - int ASN1_STRING_length(ASN1_STRING *x); const unsigned char *ASN1_STRING_get0_data(const ASN1_STRING *x); ASN1_STRING *ASN1_STRING_dup(const ASN1_STRING *a); int ASN1_STRING_cmp(ASN1_STRING *a, ASN1_STRING *b); - int ASN1_STRING_set(ASN1_STRING *str, const void *data, int len); - int ASN1_STRING_type(const ASN1_STRING *x); int ASN1_STRING_to_UTF8(unsigned char **out, const ASN1_STRING *in); + int ASN1_STRING_set_data(ASN1_STRING *str, const uint8_t *data, size_t len); + + int ASN1_STRING_set_string(ASN1_STRING *str, const char *data); + + size_t ASN1_STRING_length_ex(const ASN1_STRING *x); + +The following functions have been deprecated since OpenSSL 4.1, and can be +hidden entirely by defining B with a suitable version value, +see L: + + int ASN1_STRING_set(ASN1_STRING *str, const void *data, int len); + + int ASN1_STRING_length(ASN1_STRING *x); + =head1 DESCRIPTION These functions allow an B structure to be manipulated. @@ -38,9 +50,25 @@ ASN1_STRING_dup() returns a copy of the structure I. ASN1_STRING_cmp() compares I and I returning 0 if the two are identical. The string types and content are compared. -ASN1_STRING_set() sets the data of string I to the buffer -I or length I. The supplied data is copied. If I -is -1 then the length is determined by strlen(data). +ASN1_STRING_set() allocates memory for string I to hold I +bytes of data. Any previously allocated memory owned by I will be +freed or re-used. If I is not NULL, I bytes are copied +from the memory pointed to by I to I. If I is -1 then +the length is determined by strlen(data). + +ASN1_STRING_set_data() allocates memory for string I to hold +I bytes of data. Any previously allocated memory owned by I +will be freed or re-used. If I is not NULL, I bytes are +copied from the memory pointed to by I to I. It is an error +to use this function on a string of type B. + +ASN1_STRING_set_string() allocates memory for the string I and makes +a copy of the characters from I. Any previously +allocated memory owned by I will be freed or re-used. I +must point to a valid NUL-terminated C string, and must not be +NULL. The terminating NUL byte is not included in the data copied into +I. It is an error to use this function on a string of type +B. ASN1_STRING_type() returns the type of I, using standard constants such as B. @@ -70,8 +98,9 @@ actual string type itself: for example for an IA5String the data will be ASCII, for a BMPString two bytes per character in big endian format, and for a UTF8String it will be in UTF8 format. -Similar care should be take to ensure the data is in the correct format -when calling ASN1_STRING_set(). +Similar care should be taken to ensure the data is in the correct +format when calling ASN1_STRING_set(), ASN1_STRING_set_data(), or +ASN1_STRING_set_string(). =head1 RETURN VALUES @@ -86,7 +115,8 @@ error occurred. ASN1_STRING_cmp() returns an integer greater than, equal to, or less than 0, according to whether I is greater than, equal to, or less than I. -ASN1_STRING_set() returns 1 on success or 0 on error. +ASN1_STRING_set(), ASN1_STRING_set_data(), and +ASN1_STRING_set_string() return 1 on success or 0 on error or failure. ASN1_STRING_type() returns the type of I. @@ -97,6 +127,11 @@ negative value if an error occurred. L +=head1 HISTORY + +ASN1_STRING_set_data(), ASN1_STRING_set_string(), and ASN1_STRING_length_ex() +were added in OpenSSL 4.1. + =head1 COPYRIGHT Copyright 2002-2020 The OpenSSL Project Authors. All Rights Reserved. diff --git a/include/openssl/asn1.h.in b/include/openssl/asn1.h.in index a7291738ff1..440a6b3766e 100644 --- a/include/openssl/asn1.h.in +++ b/include/openssl/asn1.h.in @@ -540,9 +540,16 @@ int ASN1_STRING_cmp(const ASN1_STRING *a, const ASN1_STRING *b); * Since this is used to store all sorts of things, via macros, for now, * make its data void * */ +#if !defined(OPENSSL_NO_DEPRECATED_4_1) +OSSL_DEPRECATEDIN_4_1_FOR(" Use ASN1_STRING_set_data() or ASN1_STRING_set_string() instead.") int ASN1_STRING_set(ASN1_STRING *str, const void *data, int len); -void ASN1_STRING_set0(ASN1_STRING *str, void *data, int len); +OSSL_DEPRECATEDIN_4_1_FOR(" Use ASN1_STRING_length_ex() instead.") int ASN1_STRING_length(const ASN1_STRING *x); +#endif /* !defined(OPENSSL_NO_DEPRECATED_4_1) */ +void ASN1_STRING_set0(ASN1_STRING *str, void *data, int len); +int ASN1_STRING_set_data(ASN1_STRING *str, const uint8_t *data, size_t len); +int ASN1_STRING_set_string(ASN1_STRING *str, const char *cstring); +size_t ASN1_STRING_length_ex(const ASN1_STRING *x); #ifndef OPENSSL_NO_DEPRECATED_3_0 OSSL_DEPRECATEDIN_3_0 void ASN1_STRING_length_set(ASN1_STRING *x, int n); #endif diff --git a/test/asn1_string_test.c b/test/asn1_string_test.c index 6edc7619f74..24d679444bb 100644 --- a/test/asn1_string_test.c +++ b/test/asn1_string_test.c @@ -390,6 +390,7 @@ asn1_bit_string_set1_test(int idx) } static int + asn1_string_new_not_owned_test(void) { int success = 0; @@ -410,13 +411,13 @@ asn1_string_new_not_owned_test(void) if (!TEST_ptr(tmp = ASN1_STRING_new_not_owned(V_ASN1_OCTET_STRING, data, sizeof(data)))) goto err; - if (!TEST_true(ASN1_STRING_set(tmp, "muppet", (int)strlen("muppet")))) + if (!TEST_true(ASN1_STRING_set_string(tmp, "muppet"))) goto err; if (!TEST_mem_eq(data, sizeof(data), data2, sizeof(data2))) goto err; - if (!TEST_int_eq(ASN1_STRING_length(tmp), (int)strlen("muppet"))) + if (!TEST_size_t_eq(ASN1_STRING_length_ex(tmp), strlen("muppet"))) goto err; if (!TEST_mem_eq(ASN1_STRING_get0_data(tmp), strlen("muppet"), "muppet", strlen("muppet"))) @@ -436,13 +437,13 @@ asn1_string_new_not_owned_test(void) if (!TEST_mem_eq(data, sizeof(data), data2, sizeof(data2))) goto err; - if (!TEST_int_eq(ASN1_STRING_length(tmp), 4)) + if (!TEST_size_t_eq(ASN1_STRING_length_ex(tmp), 4)) goto err; if (!TEST_mem_eq(ASN1_STRING_get0_data(tmp), strlen("puppet"), "puppet", strlen("puppet"))) goto err; - memset((uint8_t *)ASN1_STRING_get0_data(tmp), 'z', ASN1_STRING_length(tmp)); + memset((uint8_t *)ASN1_STRING_get0_data(tmp), 'z', ASN1_STRING_length_ex(tmp)); if (!TEST_mem_eq(data, sizeof(data), data2, sizeof(data2))) goto err; @@ -474,10 +475,88 @@ err: return success; } +static int +asn1_string_set_data_test(void) +{ + int success = 0; + ASN1_STRING *str = NULL; + const uint8_t *data; + + if (!TEST_ptr(str = ASN1_STRING_new())) + goto err; + + if (!TEST_false(ASN1_STRING_set_data(str, (uint8_t *)"hoobla", -1))) + goto err; + + if (!TEST_false(ASN1_STRING_set_data(str, (uint8_t *)"hoobla", (size_t)INT_MAX + 1))) + goto err; + + if (!TEST_true(ASN1_STRING_set_data(str, NULL, 10))) + goto err; + + if (!TEST_true(ASN1_STRING_set_data(str, (uint8_t *)"hoobla", strlen("hoobla")))) + goto err; + + data = ASN1_STRING_get0_data(str); + + if (!TEST_size_t_eq(ASN1_STRING_length_ex(str), 6)) + goto err; + + if (!TEST_int_eq(memcmp("hoobla", data, strlen("hoobla")), 0)) + goto err; + + if (!TEST_true(ASN1_STRING_set_data(str, (uint8_t *)"hoobla", strlen("hoobla") + 1))) + goto err; + + data = ASN1_STRING_get0_data(str); + + if (!TEST_size_t_eq(ASN1_STRING_length_ex(str), 7)) + goto err; + + if (!TEST_int_eq(strcmp("hoobla", (char *)data), 0)) + goto err; + + success = 1; + +err: + ASN1_STRING_free(str); + return success; +} + +static int +asn1_string_set_string_test(void) +{ + int success = 0; + ASN1_STRING *str = NULL; + + if (!TEST_ptr(str = ASN1_STRING_new())) + goto err; + + if (!TEST_true(ASN1_STRING_set_string(str, "foo"))) + goto err; + + if (!TEST_size_t_eq(ASN1_STRING_length_ex(str), 3)) + goto err; + + if (!TEST_true(ASN1_STRING_set_string(str, "hoob\0la"))) + goto err; + + if (!TEST_size_t_eq(ASN1_STRING_length_ex(str), 4)) + goto err; + + success = 1; + +err: + ASN1_STRING_free(str); + return success; +} + int setup_tests(void) { ADD_ALL_TESTS(asn1_bit_string_get_length_test, OSSL_NELEM(abs_get_length_tests)); ADD_ALL_TESTS(asn1_bit_string_set1_test, OSSL_NELEM(abs_set1_tests)); ADD_TEST(asn1_string_new_not_owned_test); + ADD_TEST(asn1_string_set_data_test); + ADD_TEST(asn1_string_set_string_test); return 1; } diff --git a/util/libcrypto.num b/util/libcrypto.num index a51b72cf939..c7b7777cc4d 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -2571,9 +2571,9 @@ ASN1_STRING_copy 2569 4_0_0 EXIST::FUNCTION: ASN1_STRING_dup 2570 4_0_0 EXIST::FUNCTION: ASN1_STRING_type_new 2571 4_0_0 EXIST::FUNCTION: ASN1_STRING_cmp 2572 4_0_0 EXIST::FUNCTION: -ASN1_STRING_set 2573 4_0_0 EXIST::FUNCTION: +ASN1_STRING_set 2573 4_0_0 EXIST::FUNCTION:DEPRECATEDIN_4_1 ASN1_STRING_set0 2574 4_0_0 EXIST::FUNCTION: -ASN1_STRING_length 2575 4_0_0 EXIST::FUNCTION: +ASN1_STRING_length 2575 4_0_0 EXIST::FUNCTION:DEPRECATEDIN_4_1 ASN1_STRING_length_set 2576 4_0_0 EXIST::FUNCTION:DEPRECATEDIN_3_0 ASN1_STRING_type 2577 4_0_0 EXIST::FUNCTION: ASN1_STRING_get0_data 2578 4_0_0 EXIST::FUNCTION: @@ -5724,3 +5724,6 @@ OPENSSL_sk_set_copy_thunks ? 4_1_0 EXIST::FUNCTION: ASN1_STRING_new_not_owned ? 4_1_0 EXIST::FUNCTION: EVP_KDF_CTX_get0_kdf ? 4_1_0 EXIST::FUNCTION: EVP_KDF_CTX_get1_kdf ? 4_1_0 EXIST::FUNCTION: +ASN1_STRING_set_data ? 4_1_0 EXIST::FUNCTION: +ASN1_STRING_set_string ? 4_1_0 EXIST::FUNCTION: +ASN1_STRING_length_ex ? 4_1_0 EXIST::FUNCTION: