1
0
mirror of https://gitlab.isc.org/isc-projects/bind9.git synced 2025-04-18 09:44:09 +03:00

fix: usr: check_private failed to account for the length byte before the OID

In PRIVATEOID keys, the key data begins with a length byte followed 
by an ASN.1 object identifier that indicates the cryptographic algorithm 
to use. Previously, the length byte was not accounted for when 
checking the contents of keys and signatures, which could have led
to interoperability problems with any zones signed using PRIVATEOID.
This has been fixed.

Closes #5270

Merge branch '5270-fix-check-private' into 'main'

See merge request isc-projects/bind9!10372
This commit is contained in:
Mark Andrews 2025-04-09 20:05:54 +00:00
commit ecbae71fe9
3 changed files with 23 additions and 17 deletions

View File

@ -607,19 +607,26 @@ check_private(isc_buffer_t *source, dns_secalg_t alg) {
} else if (alg == DNS_KEYALG_PRIVATEOID) {
/*
* Check that we can extract the OID from the start of the
* key data.
* key data. We have a length byte followed by the OID BER
* encoded.
*/
const unsigned char *in = NULL;
ASN1_OBJECT *obj = NULL;
isc_buffer_activeregion(source, &sr);
in = sr.base;
obj = d2i_ASN1_OBJECT(NULL, &in, sr.length);
if (sr.length < 1 || (unsigned int)*sr.base + 1 > sr.length) {
RETERR(DNS_R_FORMERR);
}
in = sr.base + 1;
obj = d2i_ASN1_OBJECT(NULL, &in, *sr.base);
if (obj == NULL) {
ERR_clear_error();
RETERR(DNS_R_FORMERR);
}
ASN1_OBJECT_free(obj);
if ((in - sr.base) != (*sr.base + 1)) {
RETERR(DNS_R_FORMERR);
}
}
return ISC_R_SUCCESS;
}

View File

@ -164,11 +164,10 @@ generic_totext_key(ARGS_TOTEXT) {
} else if ((tctx->flags & DNS_STYLEFLAG_RRCOMMENT) != 0 &&
algorithm == DNS_KEYALG_PRIVATEOID)
{
const unsigned char *in = sr.base;
ASN1_OBJECT *obj = d2i_ASN1_OBJECT(NULL, &in, sr.length);
int n;
const unsigned char *in = sr.base + 1;
ASN1_OBJECT *obj = d2i_ASN1_OBJECT(NULL, &in, *sr.base);
INSIST(obj != NULL);
n = i2t_ASN1_OBJECT(algbuf, sizeof(buf), obj);
int n = i2t_ASN1_OBJECT(algbuf, sizeof(buf), obj);
ASN1_OBJECT_free(obj);
if (n == -1 || (size_t)n >= sizeof(algbuf)) {
dns_secalg_format((dns_secalg_t)algorithm, algbuf,

View File

@ -2098,17 +2098,17 @@ ISC_RUN_TEST_IMPL(key) {
/* PRIVATEOID */
WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x00),
/* PRIVATEOID 1.3.6.1.4.1.2495 without key data */
WIRE_VALID(0x00, 0x00, 0x00, 254, 0x06, 0x07, 0x2b, 0x06, 0x01,
0x04, 0x01, 0x93, 0x3f),
WIRE_VALID(0x00, 0x00, 0x00, 254, 0x09, 0x06, 0x07, 0x2b, 0x06,
0x01, 0x04, 0x01, 0x93, 0x3f),
/* PRIVATEOID 1.3.6.1.4.1.2495 + keydata */
WIRE_VALID(0x00, 0x00, 0x00, 254, 0x06, 0x07, 0x2b, 0x06, 0x01,
0x04, 0x01, 0x93, 0x3f, 0x00),
WIRE_VALID(0x00, 0x00, 0x00, 254, 0x09, 0x06, 0x07, 0x2b, 0x06,
0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
/* PRIVATEOID malformed OID - high-bit set on last octet */
WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x06, 0x07, 0x2b, 0x06,
0x01, 0x04, 0x01, 0x93, 0xbf, 0x00),
/* PRIVATEOID malformed OID - wrong tag */
WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x07, 0x07, 0x2b, 0x06,
0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x09, 0x07, 0x07, 0x2b,
0x06, 0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
WIRE_SENTINEL()
};
text_ok_t text_ok[] = { /* PRIVATEDNS example. */
@ -2120,14 +2120,14 @@ ISC_RUN_TEST_IMPL(key) {
/* PRIVATEOID */
TEXT_INVALID("0 0 254 AA=="),
/* PRIVATEOID 1.3.6.1.4.1.2495 */
TEXT_VALID("0 0 254 BgcrBgEEAZM/"),
TEXT_VALID("0 0 254 CQYHKwYBBAGTPw=="),
/* PRIVATEOID 1.3.6.1.4.1.2495 + keydata */
TEXT_VALID("0 0 254 BgcrBgEEAZM/AA=="),
TEXT_VALID("0 0 254 CQYHKwYBBAGTPwA="),
/* PRIVATEOID malformed OID - high-bit set on
last octet */
TEXT_INVALID("0 0 254 BgcrBgEEAZO/AA=="),
TEXT_INVALID("0 0 254 CQYHKwYBBAGTvwA="),
/* PRIVATEOID malformed OID - wrong tag */
TEXT_INVALID("0 0 254 BwcrBgEEAZM/AA=="),
TEXT_INVALID("0 0 254 CQcHKwYBBAGTPwA="),
/*
* Sentinel.
*/