From a57b127569be53cb6f5fee1de226f29dcee95d0b Mon Sep 17 00:00:00 2001 From: schwarze Date: Sat, 11 Dec 2021 22:58:48 +0000 Subject: [PATCH] Merge two bugfixes in ASN1_STRING_TABLE_add(3) and ASN1_STRING_TABLE_get(3) from the OpenSSL 1.1.1 branch, which is still under a free license, mostly this commit: commit d35c0ff30b31be9fd5dcf3d552a16feb8de464bc Author: Dr. Stephen Henson Date: Fri Oct 19 15:06:31 2012 +0000 fix ASN1_STRING_TABLE_add so it can override existing string table values This fixes a segfault in ASN1_STRING_TABLE_add(3), which tried to change a static const entry when called with an nid already in the default table, and it switches the precedence of the two tables in ASN1_STRING_TABLE_get(3). In addition, it changes behaviour in the following minor ways: * Ignore negative minsize and maxsize arguments, not just -1. * Ignore a zero mask and zero flags. It's unclear whether these additional changes make the API absolutely better, but we want compatibility with OpenSSL in these functions. Tweaks & OK tb@. --- lib/libcrypto/asn1/a_strnid.c | 92 ++++++++++------ regress/lib/libcrypto/asn1/Makefile | 5 +- regress/lib/libcrypto/asn1/string_table.c | 128 ++++++++++++++++++++++ 3 files changed, 189 insertions(+), 36 deletions(-) create mode 100644 regress/lib/libcrypto/asn1/string_table.c diff --git a/lib/libcrypto/asn1/a_strnid.c b/lib/libcrypto/asn1/a_strnid.c index 0cc8dc84285..08043f723b6 100644 --- a/lib/libcrypto/asn1/a_strnid.c +++ b/lib/libcrypto/asn1/a_strnid.c @@ -1,4 +1,4 @@ -/* $OpenBSD: a_strnid.c,v 1.22 2021/12/11 22:34:36 schwarze Exp $ */ +/* $OpenBSD: a_strnid.c,v 1.23 2021/12/11 22:58:48 schwarze Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 1999. */ @@ -64,6 +64,8 @@ #include static STACK_OF(ASN1_STRING_TABLE) *stable = NULL; + +static ASN1_STRING_TABLE *stable_get(int nid); static void st_free(ASN1_STRING_TABLE *tbl); static int sk_table_cmp(const ASN1_STRING_TABLE * const *a, const ASN1_STRING_TABLE * const *b); @@ -235,20 +237,59 @@ ASN1_STRING_TABLE * ASN1_STRING_TABLE_get(int nid) { int idx; - ASN1_STRING_TABLE *ttmp; ASN1_STRING_TABLE fnd; fnd.nid = nid; - ttmp = OBJ_bsearch_table(&fnd, tbl_standard, + if (stable != NULL) { + idx = sk_ASN1_STRING_TABLE_find(stable, &fnd); + if (idx >= 0) + return sk_ASN1_STRING_TABLE_value(stable, idx); + } + return OBJ_bsearch_table(&fnd, tbl_standard, sizeof(tbl_standard)/sizeof(ASN1_STRING_TABLE)); - if (ttmp) - return ttmp; - if (!stable) +} + +/* + * Return a string table pointer which can be modified: either directly + * from table or a copy of an internal value added to the table. + */ + +static ASN1_STRING_TABLE * +stable_get(int nid) +{ + ASN1_STRING_TABLE *tmp, *rv; + + /* Always need a string table so allocate one if NULL */ + if (stable == NULL) { + stable = sk_ASN1_STRING_TABLE_new(sk_table_cmp); + if (stable == NULL) + return NULL; + } + tmp = ASN1_STRING_TABLE_get(nid); + if (tmp != NULL && (tmp->flags & STABLE_FLAGS_MALLOC) != 0) + return tmp; + + if ((rv = calloc(1, sizeof(*rv))) == NULL) { + ASN1error(ERR_R_MALLOC_FAILURE); return NULL; - idx = sk_ASN1_STRING_TABLE_find(stable, &fnd); - if (idx < 0) + } + if (!sk_ASN1_STRING_TABLE_push(stable, rv)) { + free(rv); return NULL; - return sk_ASN1_STRING_TABLE_value(stable, idx); + } + if (tmp != NULL) { + rv->nid = tmp->nid; + rv->minsize = tmp->minsize; + rv->maxsize = tmp->maxsize; + rv->mask = tmp->mask; + rv->flags = tmp->flags | STABLE_FLAGS_MALLOC; + } else { + rv->nid = nid; + rv->minsize = -1; + rv->maxsize = -1; + rv->flags = STABLE_FLAGS_MALLOC; + } + return rv; } int @@ -256,37 +297,20 @@ ASN1_STRING_TABLE_add(int nid, long minsize, long maxsize, unsigned long mask, unsigned long flags) { ASN1_STRING_TABLE *tmp; - char new_nid = 0; - flags &= ~STABLE_FLAGS_MALLOC; - if (!stable) - stable = sk_ASN1_STRING_TABLE_new(sk_table_cmp); - if (!stable) { + if ((tmp = stable_get(nid)) == NULL) { ASN1error(ERR_R_MALLOC_FAILURE); return 0; } - if (!(tmp = ASN1_STRING_TABLE_get(nid))) { - tmp = malloc(sizeof(ASN1_STRING_TABLE)); - if (!tmp) { - ASN1error(ERR_R_MALLOC_FAILURE); - return 0; - } - tmp->flags = flags | STABLE_FLAGS_MALLOC; - tmp->nid = nid; - new_nid = 1; - } else tmp->flags = (tmp->flags & STABLE_FLAGS_MALLOC) | flags; - if (minsize != -1) + if (minsize >= 0) tmp->minsize = minsize; - if (maxsize != -1) + if (maxsize >= 0) tmp->maxsize = maxsize; - tmp->mask = mask; - if (new_nid) { - if (sk_ASN1_STRING_TABLE_push(stable, tmp) == 0) { - free(tmp); - ASN1error(ERR_R_MALLOC_FAILURE); - return 0; - } - } + if (mask != 0) + tmp->mask = mask; + if (flags != 0) + tmp->flags = flags | STABLE_FLAGS_MALLOC; + return 1; } diff --git a/regress/lib/libcrypto/asn1/Makefile b/regress/lib/libcrypto/asn1/Makefile index 1c4a42b80b9..90eca92f8ec 100644 --- a/regress/lib/libcrypto/asn1/Makefile +++ b/regress/lib/libcrypto/asn1/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.9 2021/12/09 16:30:57 jsing Exp $ +# $OpenBSD: Makefile,v 1.10 2021/12/11 22:58:48 schwarze Exp $ TESTS = \ asn1basic \ @@ -7,7 +7,8 @@ TESTS = \ asn1string_copy \ asn1time \ asn1x509 \ - rfc5280time + rfc5280time \ + string_table PROGS = ${TESTS} diff --git a/regress/lib/libcrypto/asn1/string_table.c b/regress/lib/libcrypto/asn1/string_table.c new file mode 100644 index 00000000000..e80cf0f2069 --- /dev/null +++ b/regress/lib/libcrypto/asn1/string_table.c @@ -0,0 +1,128 @@ +/* $OpenBSD: string_table.c,v 1.1 2021/12/11 22:58:48 schwarze Exp $ */ +/* + * Copyright (c) 2021 Ingo Schwarze + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include +#include +#include + +static int errcount; + +static void +report(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + vwarnx(fmt, ap); + va_end(ap); + + errcount++; +} + +static void +stable_check(const char *testname, ASN1_STRING_TABLE *have, + ASN1_STRING_TABLE *want, unsigned long want_flags) +{ + if (have == NULL) { + report("%s returned NULL", testname); + return; + } + if (have->nid != want->nid) + report("%s nid %d, expected %d", testname, + have->nid, want->nid); + if (have->minsize != want->minsize) + report("%s minsize %ld, expected %ld", testname, + have->minsize, want->minsize); + if (have->maxsize != want->maxsize) + report("%s maxsize %ld, expected %ld", testname, + have->maxsize, want->maxsize); + if (have->mask != want->mask) + report("%s mask %lu, expected %lu", testname, + have->mask, want->mask); + if (have->flags != want_flags) + report("%s flags %lu, expected %lu", testname, + have->flags, want_flags); +} + +int +main(void) +{ + ASN1_STRING_TABLE orig, mine, *have; + int irc; + + orig.nid = NID_name; + orig.minsize = 1; + orig.maxsize = ub_name; + orig.mask = DIRSTRING_TYPE; + orig.flags = 0; + + mine.nid = NID_name; + mine.minsize = 4; + mine.maxsize = 64; + mine.mask = B_ASN1_PRINTABLESTRING; + mine.flags = STABLE_NO_MASK; + + /* Original entry. */ + + have = ASN1_STRING_TABLE_get(orig.nid); + stable_check("orig", have, &orig, 0); + + /* Copy, but don't really change. */ + + irc = ASN1_STRING_TABLE_add(orig.nid, -1, -1, 0, 0); + if (irc != 1) + report("set noop returned %d, expected 1", irc); + have = ASN1_STRING_TABLE_get(orig.nid); + stable_check("noop", have, &orig, STABLE_FLAGS_MALLOC); + + /* Change entry. */ + + irc = ASN1_STRING_TABLE_add(mine.nid, mine.minsize, mine.maxsize, + mine.mask, mine.flags); + if (irc != 1) + report("set returned %d, expected 1", irc); + have = ASN1_STRING_TABLE_get(mine.nid); + stable_check("set", have, &mine, STABLE_FLAGS_MALLOC | STABLE_NO_MASK); + + /* New entry. */ + + mine.nid = NID_title; + irc = ASN1_STRING_TABLE_add(mine.nid, mine.minsize, mine.maxsize, + mine.mask, mine.flags); + if (irc != 1) + report("new returned %d, expected 1", irc); + have = ASN1_STRING_TABLE_get(mine.nid); + stable_check("new", have, &mine, STABLE_FLAGS_MALLOC | STABLE_NO_MASK); + + /* Back to the initial state. */ + + ASN1_STRING_TABLE_cleanup(); + have = ASN1_STRING_TABLE_get(orig.nid); + stable_check("back", have, &orig, 0); + if (ASN1_STRING_TABLE_get(mine.nid) != NULL) + report("deleted entry is not NULL"); + + switch (errcount) { + case 0: + return 0; + case 1: + errx(1, "one error"); + default: + errx(1, "%d errors", errcount); + } +} -- 2.20.1