Skip to content

Commit 06d1dc3

Browse files
Viktor Dukhovnit8m
Viktor Dukhovni
authored and
t8m
committed
Avoid type errors in EAI-related name check logic.
The incorrectly typed data is read only, used in a compare operation, so neither remote code execution, nor memory content disclosure were possible. However, applications performing certificate name checks were vulnerable to denial of service. The GENERAL_TYPE data type is a union, and we must take care to access the correct member, based on `gen->type`, not all the member fields have the same structure, and a segfault is possible if the wrong member field is read. The code in question was lightly refactored with the intent to make it more obviously correct. Fixes CVE-2024-6119 Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (cherry picked from commit 0890cd1)
1 parent 9aae563 commit 06d1dc3

5 files changed

Lines changed: 142 additions & 26 deletions

File tree

‎crypto/x509/v3_utl.c‎

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -916,36 +916,64 @@ static int do_x509_check(X509 *x, const char *chk, size_t chklen,
916916
ASN1_STRING *cstr;
917917

918918
gen = sk_GENERAL_NAME_value(gens, i);
919-
if ((gen->type == GEN_OTHERNAME) && (check_type == GEN_EMAIL)) {
920-
if (OBJ_obj2nid(gen->d.otherName->type_id) ==
921-
NID_id_on_SmtpUTF8Mailbox) {
922-
san_present = 1;
923-
924-
/*
925-
* If it is not a UTF8String then that is unexpected and we
926-
* treat it as no match
927-
*/
928-
if (gen->d.otherName->value->type == V_ASN1_UTF8STRING) {
929-
cstr = gen->d.otherName->value->value.utf8string;
930-
931-
/* Positive on success, negative on error! */
932-
if ((rv = do_check_string(cstr, 0, equal, flags,
933-
chk, chklen, peername)) != 0)
934-
break;
935-
}
936-
} else
919+
switch (gen->type) {
920+
default:
921+
continue;
922+
case GEN_OTHERNAME:
923+
switch (OBJ_obj2nid(gen->d.otherName->type_id)) {
924+
default:
937925
continue;
938-
} else {
939-
if ((gen->type != check_type) && (gen->type != GEN_OTHERNAME))
926+
case NID_id_on_SmtpUTF8Mailbox:
927+
/*-
928+
* https://datatracker.ietf.org/doc/html/rfc8398#section-3
929+
*
930+
* Due to name constraint compatibility reasons described
931+
* in Section 6, SmtpUTF8Mailbox subjectAltName MUST NOT
932+
* be used unless the local-part of the email address
933+
* contains non-ASCII characters. When the local-part is
934+
* ASCII, rfc822Name subjectAltName MUST be used instead
935+
* of SmtpUTF8Mailbox. This is compatible with legacy
936+
* software that supports only rfc822Name (and not
937+
* SmtpUTF8Mailbox). [...]
938+
*
939+
* SmtpUTF8Mailbox is encoded as UTF8String.
940+
*
941+
* If it is not a UTF8String then that is unexpected, and
942+
* we ignore the invalid SAN (neither set san_present nor
943+
* consider it a candidate for equality). This does mean
944+
* that the subject CN may be considered, as would be the
945+
* case when the malformed SmtpUtf8Mailbox SAN is instead
946+
* simply absent.
947+
*
948+
* When CN-ID matching is not desirable, applications can
949+
* choose to turn it off, doing so is at this time a best
950+
* practice.
951+
*/
952+
if (check_type != GEN_EMAIL
953+
|| gen->d.otherName->value->type != V_ASN1_UTF8STRING)
954+
continue;
955+
alt_type = 0;
956+
cstr = gen->d.otherName->value->value.utf8string;
957+
break;
958+
}
959+
break;
960+
case GEN_EMAIL:
961+
if (check_type != GEN_EMAIL)
940962
continue;
941-
}
942-
san_present = 1;
943-
if (check_type == GEN_EMAIL)
944963
cstr = gen->d.rfc822Name;
945-
else if (check_type == GEN_DNS)
964+
break;
965+
case GEN_DNS:
966+
if (check_type != GEN_DNS)
967+
continue;
946968
cstr = gen->d.dNSName;
947-
else
969+
break;
970+
case GEN_IPADD:
971+
if (check_type != GEN_IPADD)
972+
continue;
948973
cstr = gen->d.iPAddress;
974+
break;
975+
}
976+
san_present = 1;
949977
/* Positive on success, negative on error! */
950978
if ((rv = do_check_string(cstr, alt_type, equal, flags,
951979
chk, chklen, peername)) != 0)

‎test/recipes/25-test_eai_data.t‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,18 @@ setup("test_eai_data");
2121
#./util/wrap.pl apps/openssl verify -nameopt utf8 -no_check_time -CAfile test/recipes/25-test_eai_data/utf8_chain.pem test/recipes/25-test_eai_data/ascii_leaf.pem
2222
#./util/wrap.pl apps/openssl verify -nameopt utf8 -no_check_time -CAfile test/recipes/25-test_eai_data/ascii_chain.pem test/recipes/25-test_eai_data/utf8_leaf.pem
2323

24-
plan tests => 12;
24+
plan tests => 16;
2525

2626
require_ok(srctop_file('test','recipes','tconversion.pl'));
2727
my $folder = "test/recipes/25-test_eai_data";
2828

2929
my $ascii_pem = srctop_file($folder, "ascii_leaf.pem");
3030
my $utf8_pem = srctop_file($folder, "utf8_leaf.pem");
31+
my $kdc_pem = srctop_file($folder, "kdc-cert.pem");
3132

3233
my $ascii_chain_pem = srctop_file($folder, "ascii_chain.pem");
3334
my $utf8_chain_pem = srctop_file($folder, "utf8_chain.pem");
35+
my $kdc_chain_pem = srctop_file($folder, "kdc-root-cert.pem");
3436

3537
my $out;
3638
my $outcnt = 0;
@@ -56,10 +58,18 @@ SKIP: {
5658

5759
ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $ascii_chain_pem, $ascii_pem])));
5860
ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $utf8_chain_pem, $utf8_pem])));
61+
ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $kdc_chain_pem, $kdc_pem])));
5962

6063
ok(!run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $ascii_chain_pem, $utf8_pem])));
6164
ok(!run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $utf8_chain_pem, $ascii_pem])));
6265

66+
# Check an otherName does not get misparsed as an DNS name, (should trigger ASAN errors if violated).
67+
ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-verify_hostname", 'mx1.example.com', "-CAfile", $kdc_chain_pem, $kdc_pem])));
68+
# Check an otherName does not get misparsed as an email address, (should trigger ASAN errors if violated).
69+
ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-verify_email", 'joe@example.com', "-CAfile", $kdc_chain_pem, $kdc_pem])));
70+
# We expect SmtpUTF8Mailbox to be a UTF8 String, not an IA5String.
71+
ok(!run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-verify_email", 'moe@example.com', "-CAfile", $kdc_chain_pem, $kdc_pem])));
72+
6373
#Check that we get the expected failure return code
6474
with({ exit_checker => sub { return shift == 2; } },
6575
sub {

‎test/recipes/25-test_eai_data/kdc-cert.pem‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDbDCCAlSgAwIBAgIBAjANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDARSb290
3+
MCAXDTI0MDYyMDA2MTQxNVoYDzIxMjQwNjIwMDYxNDE1WjAXMRUwEwYDVQQDDAxU
4+
RVNULkVYQU1QTEUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC6wfP+
5+
6go79dkpo/dGLMlPZ7Gw/Q6gUYrCWZWUEgEeRVHCrqOlgUEyA+PcWas/XDPUxXry
6+
BQlJHLvlqamAQn8gs4QPBARFYWKNiTVGyaRkgNA1N5gqyZdrP9UE+ZJmdqxRAAe8
7+
vvpGZWSgevPhLUiSCFYDiD0Rtji2Hm3rGUrReQFBQDEw2pNGwz9zIaxUs08kQZcx
8+
Yzyiplz5Oau+R/6sAgUwDlrD9xOlUxx/tA/MSDIfkK8qioU11uUZtO5VjkNQy/bT
9+
7zQMmXxWgm2MIgOs1u4YN7YGOtgqHE9v9iPHHfgrkbQDtVDGQsa8AQEhkUDSCtW9
10+
3VFAKx6dGNXYzFwfAgMBAAGjgcgwgcUwHQYDVR0OBBYEFFR5tZycW19DmtbL4Zqj
11+
te1c2vZLMAkGA1UdIwQCMAAwCQYDVR0TBAIwADCBjQYDVR0RBIGFMIGCoD8GBisG
12+
AQUCAqA1MDOgDhsMVEVTVC5FWEFNUExFoSEwH6ADAgEBoRgwFhsGa3JidGd0GwxU
13+
RVNULkVYQU1QTEWgHQYIKwYBBQUHCAmgERYPbW9lQGV4YW1wbGUuY29tgQ9qb2VA
14+
ZXhhbXBsZS5jb22CD214MS5leGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEA
15+
T0xzVtVpRtaOzIhgzw7XQUdzWD5UEGSJJ1cBCOmKUWwDLTAouCYLFB4TbEE7MMUb
16+
iuMy60bjmVtvfJIXorGUgSadRe5RWJ5DamJWvPA0Q9x7blnEcXqEF+9Td+ypevgU
17+
UYHFmg83OYwxOsFXZ5cRuXMk3WCsDHQIBi6D1L6oDDZ2pfArs5mqm3thQKVlqyl1
18+
El3XRYEdqAz/5eCOFNfwxF0ALxjxVr/Z50StUZU8I7Zfev6+kHhyrR7dqzYJImv9
19+
0fTCOBEMjIETDsrA70OxAMu4V16nrWZdJdvzblS2qrt97Omkj+2kiPAJFB76RpwI
20+
oDQ9fKfUOAmUFth2/R/eGA==
21+
-----END CERTIFICATE-----

‎test/recipes/25-test_eai_data/kdc-root-cert.pem‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICnDCCAYQCCQCBswYcrlZSHjANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDARS
3+
b290MCAXDTI0MDYyMDA2MTQxNVoYDzIxMjQwNjIwMDYxNDE1WjAPMQ0wCwYDVQQD
4+
DARSb290MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqRj8S4kBbIUj
5+
61kZfi6nE35Q38U140+qt4uAiwAhKumfVHlBM0zQ98WFt5zMHIBQwIb3yjc2zj+0
6+
qzUnQfwm1r/RfcMmBPEti9Ge+aEMSsds2gMXziOFM8wd2aAFPy7UVE0XpEWofsRK
7+
MGi61MKVdPSbGIxBwY9VW38/7D/wf1HtJe7y0xpuecR7GB2XAs+qST59NjuF+7wS
8+
dLM8Hb3TATgeYbXXWsRJgwz+SPzExg5WmLnU+7y4brZ32dHtdSmkRVSgSlaIf7Xj
9+
3Tc6Zi7I+W/JYk7hy1zUexVdWCak4PHcoWrXe0gNNN/t8VfLfMExt5z/HIylXnU7
10+
pGUyqZlTGQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAHpLF1UCRy7b6Hk0rLokxI
11+
lgwiH9BU9mktigAGASvkbllpt+YbUbWnuYAvpHBGiP1qZtfX2r96UrSJaGO9BEzT
12+
Gp9ThnSjoj4Srul0+s/NArU22irFLmDzbalgevAmm9gMGkdqkiIm/mXbwrPj0ncl
13+
KGicevXryVpvaP62eZ8cc3C4p97frMmXxRX8sTdQpD/gRI7prdEILRSKveqT+AEW
14+
7rFGM5AOevb4U8ddop8A3D/kX0wcCAIBF6jCNk3uEJ57jVcagL04kPnVfdRiedTS
15+
vfq1DRNcD29d1H/9u0fHdSn1/+8Ep3X+afQ3C6//5NvOEaXcIGO4QSwkprQydfv8
16+
-----END CERTIFICATE-----

‎test/recipes/25-test_eai_data/kdc.sh‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#! /usr/bin/env bash
2+
3+
# Create a root CA, signing a leaf cert with a KDC principal otherName SAN, and
4+
# also a non-UTF8 smtpUtf8Mailbox SAN followed by an rfc822Name SAN and a DNS
5+
# name SAN. In the vulnerable EAI code, the KDC principal `otherName` should
6+
# trigger ASAN errors in DNS name checks, while the non-UTF8 `smtpUtf8Mailbox`
7+
# should likewise lead to ASAN issues with email name checks.
8+
9+
rm -f root-key.pem root-cert.pem
10+
openssl req -nodes -new -newkey rsa:2048 -keyout kdc-root-key.pem \
11+
-x509 -subj /CN=Root -days 36524 -out kdc-root-cert.pem
12+
13+
exts=$(
14+
printf "%s\n%s\n%s\n%s = " \
15+
"subjectKeyIdentifier = hash" \
16+
"authorityKeyIdentifier = keyid" \
17+
"basicConstraints = CA:false" \
18+
"subjectAltName"
19+
printf "%s, " "otherName:1.3.6.1.5.2.2;SEQUENCE:kdc_princ_name"
20+
printf "%s, " "otherName:1.3.6.1.5.5.7.8.9;IA5:moe@example.com"
21+
printf "%s, " "email:joe@example.com"
22+
printf "%s\n" "DNS:mx1.example.com"
23+
printf "[kdc_princ_name]\n"
24+
printf "realm = EXP:0, GeneralString:TEST.EXAMPLE\n"
25+
printf "principal_name = EXP:1, SEQUENCE:kdc_principal_seq\n"
26+
printf "[kdc_principal_seq]\n"
27+
printf "name_type = EXP:0, INTEGER:1\n"
28+
printf "name_string = EXP:1, SEQUENCE:kdc_principal_components\n"
29+
printf "[kdc_principal_components]\n"
30+
printf "princ1 = GeneralString:krbtgt\n"
31+
printf "princ2 = GeneralString:TEST.EXAMPLE\n"
32+
)
33+
34+
printf "%s\n" "$exts"
35+
36+
openssl req -nodes -new -newkey rsa:2048 -keyout kdc-key.pem \
37+
-subj "/CN=TEST.EXAMPLE" |
38+
openssl x509 -req -out kdc-cert.pem \
39+
-CA "kdc-root-cert.pem" -CAkey "kdc-root-key.pem" \
40+
-set_serial 2 -days 36524 \
41+
-extfile <(printf "%s\n" "$exts")

0 commit comments

Comments
 (0)