mirror of
https://github.com/openbsd/src.git
synced 2025-01-10 06:47:55 -08:00
Change X509_NAME_get_index_by[NID|OBJ] to be safer.
Currently these functions return raw ASN1_STRING bytes as a C string and ignore the encoding in a "hold my beer I am a toolkit not a functioning API surely it's just for testing and you'd never send nasty bytes" kind of way. Sadly some callers seem to use them to fetch things liks subject name components for comparisons, and often just use the result as a C string. Instead, encode the resulting bytes as UTF-8 so it is something like "text", Add a failure case if the length provided is inadequate or if the resulting text would contain an nul byte. based on boringssl. nits by dlg@ ok tb@
This commit is contained in:
beck
parent
e982dbc862
commit
de9b195a9c
@ -1,4 +1,4 @@
|
||||
.\" $OpenBSD: X509_NAME_get_index_by_NID.3,v 1.13 2022/07/02 17:09:09 jsing Exp $
|
||||
.\" $OpenBSD: X509_NAME_get_index_by_NID.3,v 1.14 2023/05/02 14:13:05 beck Exp $
|
||||
.\" OpenSSL aebb9aac Jul 19 09:27:53 2016 -0400
|
||||
.\"
|
||||
.\" This file was written by Dr. Stephen Henson <steve@openssl.org>.
|
||||
@ -49,7 +49,7 @@
|
||||
.\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
|
||||
.\" OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
.\"
|
||||
.Dd $Mdocdate: July 2 2022 $
|
||||
.Dd $Mdocdate: May 2 2023 $
|
||||
.Dt X509_NAME_GET_INDEX_BY_NID 3
|
||||
.Os
|
||||
.Sh NAME
|
||||
@ -136,22 +136,32 @@ run from 0 to
|
||||
.Fn X509_NAME_get_text_by_NID
|
||||
and
|
||||
.Fn X509_NAME_get_text_by_OBJ
|
||||
retrieve the "text" from the first entry in
|
||||
retrieve the bytes encoded as UTF-8 from the first entry in
|
||||
.Fa name
|
||||
which matches
|
||||
.Fa nid
|
||||
or
|
||||
.Fa obj .
|
||||
At most
|
||||
.Fa len
|
||||
bytes will be written and the text written to
|
||||
.Fa buf
|
||||
will be NUL terminated.
|
||||
If
|
||||
.Fa buf
|
||||
is
|
||||
.Dv NULL ,
|
||||
nothing is written, but the return value is calculated as usual.
|
||||
If
|
||||
.Fa buf
|
||||
is not
|
||||
.Dv NULL ,
|
||||
no more than
|
||||
.Fa len
|
||||
bytes will be written and the text written to
|
||||
.Fa buf
|
||||
will be NUL terminated.
|
||||
.Pp
|
||||
Nothing is written and it is a failure if
|
||||
.Fa len
|
||||
is not large enough to hold the NUL byte terminated UTF-8 encoding of
|
||||
the text, or if the UTF-8 encoding ot the text would contins a NUL
|
||||
byte.
|
||||
.Pp
|
||||
All relevant
|
||||
.Dv NID_*
|
||||
@ -189,8 +199,8 @@ if the index is invalid.
|
||||
.Fn X509_NAME_get_text_by_NID
|
||||
and
|
||||
.Fn X509_NAME_get_text_by_OBJ
|
||||
return the length of the output string written, not counting the
|
||||
terminating NUL, or -1 if no match is found.
|
||||
return the length of the output UTF-8 string written, not counting the
|
||||
terminating NUL, or -1 in the case of an error or no match being found.
|
||||
.Pp
|
||||
In some cases of failure of
|
||||
.Fn X509_NAME_get_index_by_NID
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
/* $OpenBSD: x509name.c,v 1.31 2023/02/16 08:38:17 tb Exp $ */
|
||||
/* $OpenBSD: x509name.c,v 1.32 2023/05/02 14:13:05 beck Exp $ */
|
||||
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
|
||||
* All rights reserved.
|
||||
*
|
||||
@ -66,6 +66,7 @@
|
||||
#include <openssl/stack.h>
|
||||
#include <openssl/x509.h>
|
||||
|
||||
#include "bytestring.h"
|
||||
#include "x509_local.h"
|
||||
|
||||
int
|
||||
@ -84,21 +85,37 @@ int
|
||||
X509_NAME_get_text_by_OBJ(X509_NAME *name, const ASN1_OBJECT *obj, char *buf,
|
||||
int len)
|
||||
{
|
||||
int i;
|
||||
unsigned char *text = NULL;
|
||||
ASN1_STRING *data;
|
||||
int i, text_len;
|
||||
int ret = -1;
|
||||
CBS cbs;
|
||||
|
||||
i = X509_NAME_get_index_by_OBJ(name, obj, -1);
|
||||
if (i < 0)
|
||||
return (-1);
|
||||
goto err;
|
||||
data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, i));
|
||||
i = (data->length > (len - 1)) ? (len - 1) : data->length;
|
||||
if (buf == NULL)
|
||||
return (data->length);
|
||||
if (i >= 0) {
|
||||
memcpy(buf, data->data, i);
|
||||
buf[i] = '\0';
|
||||
/*
|
||||
* Fail if we cannot encode as UTF-8, or if the UTF-8 encoding of the
|
||||
* string contains a 0 byte, because mortal callers seldom handle the
|
||||
* length difference correctly
|
||||
*/
|
||||
if ((text_len = ASN1_STRING_to_UTF8(&text, data)) < 0)
|
||||
goto err;
|
||||
CBS_init(&cbs, text, text_len);
|
||||
if (CBS_contains_zero_byte(&cbs))
|
||||
goto err;
|
||||
/* We still support the "pass NULL to find out how much" API */
|
||||
if (buf != NULL) {
|
||||
if (!CBS_write_bytes(&cbs, buf, len - 1, NULL))
|
||||
goto err;
|
||||
/* It must be a C string */
|
||||
buf[text_len] = '\0';
|
||||
}
|
||||
return (i);
|
||||
ret = text_len;
|
||||
err:
|
||||
free(text);
|
||||
return (ret);
|
||||
}
|
||||
LCRYPTO_ALIAS(X509_NAME_get_text_by_OBJ);
|
||||
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
/* $OpenBSD: x509_asn1.c,v 1.16 2023/05/01 11:02:23 job Exp $ */
|
||||
/* $OpenBSD: x509_asn1.c,v 1.17 2023/05/02 14:13:05 beck Exp $ */
|
||||
/*
|
||||
* Copyright (c) 2023 Job Snijders <job@openbsd.org>
|
||||
*
|
||||
@ -512,13 +512,88 @@ test_x509_req_setters(void)
|
||||
return failed;
|
||||
}
|
||||
|
||||
int main(void)
|
||||
static const struct testcase {
|
||||
char *data;
|
||||
int len;
|
||||
int len_to_pass;
|
||||
int encode_type;
|
||||
int expected_result;
|
||||
char *expected_string;
|
||||
} testCases[] = {
|
||||
/* should work */
|
||||
{"fozzie", 6, 80, MBSTRING_ASC, 6, "fozzie"},
|
||||
/* should work */
|
||||
{"fozzie", 6, -1, MBSTRING_ASC, 6, ""},
|
||||
/* should fail, truncation */
|
||||
{"muppet", 6, 5, MBSTRING_ASC, -1, ""},
|
||||
/* should fail, contains 0 byte */
|
||||
{"g\0nzo", 5, 80, MBSTRING_ASC, -1, ""},
|
||||
/* should fail, can't encode as utf-8 */
|
||||
{"\x30\x00", 2, 80, V_ASN1_SEQUENCE, -1, ""},
|
||||
};
|
||||
|
||||
#define NUM_TEST_CASES (sizeof(testCases) / sizeof(testCases[0]))
|
||||
|
||||
static int
|
||||
test_x509_name_get(void)
|
||||
{
|
||||
int failed = 0;
|
||||
size_t i;
|
||||
|
||||
for (i = 0; i < NUM_TEST_CASES; i++) {
|
||||
const struct testcase *test = testCases + i;
|
||||
X509_NAME_ENTRY *entry = NULL;
|
||||
X509_NAME *name = NULL;
|
||||
char textbuf[80];
|
||||
int result;
|
||||
|
||||
textbuf[0] = '\0';
|
||||
if ((name = X509_NAME_new()) == NULL)
|
||||
err(1, "X509_NAME_new");
|
||||
if ((entry = X509_NAME_ENTRY_new()) == NULL)
|
||||
err(1, "X509_NAME_ENTRY_new");
|
||||
if (!X509_NAME_ENTRY_set_object(entry,
|
||||
OBJ_nid2obj(NID_commonName)))
|
||||
err(1, "X509_NAME_ENTRY_set_object");
|
||||
if (!X509_NAME_ENTRY_set_data(entry, test->encode_type,
|
||||
test->data, test->len))
|
||||
err(1, "X509_NAME_ENTRY_set_data");
|
||||
if (!X509_NAME_add_entry(name, entry, -1, 0))
|
||||
err(1, "X509_NAME_add_entry");
|
||||
if (test->len_to_pass == -1)
|
||||
result = X509_NAME_get_text_by_NID(name, NID_commonName,
|
||||
NULL, 0);
|
||||
else
|
||||
result = X509_NAME_get_text_by_NID(name, NID_commonName,
|
||||
textbuf, test->len_to_pass);
|
||||
if (result != test->expected_result) {
|
||||
fprintf(stderr,
|
||||
"Test %zu X509_GET_text_by_NID returned %d,"
|
||||
"expected %d\n", i, result, test->expected_result);
|
||||
failed++;
|
||||
}
|
||||
if (result != -1 &&
|
||||
strcmp(test->expected_string, textbuf) != 0) {
|
||||
fprintf(stderr,
|
||||
"Test %zu, X509_GET_text_by_NID returned bytes do"
|
||||
"not match \n", i);
|
||||
failed++;
|
||||
}
|
||||
X509_NAME_ENTRY_free(entry);
|
||||
X509_NAME_free(name);
|
||||
}
|
||||
return failed;
|
||||
}
|
||||
|
||||
int
|
||||
main(void)
|
||||
{
|
||||
int failed = 0;
|
||||
|
||||
failed |= test_x509_setters();
|
||||
/* failed |= */ test_x509_crl_setters();
|
||||
/* failed |= */ test_x509_req_setters();
|
||||
failed |= test_x509_name_get();
|
||||
|
||||
OPENSSL_cleanup();
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user