2024-09-08 FreeBSD security patches

crypto/openssl/crypto/x509/v3_utl.c

@@ -916,36 +916,64 @@ static int do_x509_check(X509 *x, const char *chk, size_t chklen,
             ASN1_STRING *cstr;
 
             gen = sk_GENERAL_NAME_value(gens, i);
-            if ((gen->type == GEN_OTHERNAME) && (check_type == GEN_EMAIL)) {
-                if (OBJ_obj2nid(gen->d.otherName->type_id) ==
-                    NID_id_on_SmtpUTF8Mailbox) {
-                    san_present = 1;
-
-                    /*
-                     * If it is not a UTF8String then that is unexpected and we
-                     * treat it as no match
-                     */
-                    if (gen->d.otherName->value->type == V_ASN1_UTF8STRING) {
-                        cstr = gen->d.otherName->value->value.utf8string;
-
-                        /* Positive on success, negative on error! */
-                        if ((rv = do_check_string(cstr, 0, equal, flags,
-                                                chk, chklen, peername)) != 0)
-                            break;
-                    }
-                } else
+            switch (gen->type) {
+            default:
+                continue;
+            case GEN_OTHERNAME:
+		switch (OBJ_obj2nid(gen->d.otherName->type_id)) {
+                default:
                     continue;
-            } else {
-                if ((gen->type != check_type) && (gen->type != GEN_OTHERNAME))
+                case NID_id_on_SmtpUTF8Mailbox:
+                    /*-
+                     * https://datatracker.ietf.org/doc/html/rfc8398#section-3
+                     *
+                     *   Due to name constraint compatibility reasons described
+                     *   in Section 6, SmtpUTF8Mailbox subjectAltName MUST NOT
+                     *   be used unless the local-part of the email address
+                     *   contains non-ASCII characters. When the local-part is
+                     *   ASCII, rfc822Name subjectAltName MUST be used instead
+                     *   of SmtpUTF8Mailbox. This is compatible with legacy
+                     *   software that supports only rfc822Name (and not
+                     *   SmtpUTF8Mailbox). [...]
+                     *
+                     *   SmtpUTF8Mailbox is encoded as UTF8String.
+                     *
+                     * If it is not a UTF8String then that is unexpected, and
+                     * we ignore the invalid SAN (neither set san_present nor
+                     * consider it a candidate for equality).  This does mean
+                     * that the subject CN may be considered, as would be the
+                     * case when the malformed SmtpUtf8Mailbox SAN is instead
+                     * simply absent.
+                     *
+                     * When CN-ID matching is not desirable, applications can
+                     * choose to turn it off, doing so is at this time a best
+                     * practice.
+                     */
+                    if (check_type != GEN_EMAIL
+                        || gen->d.otherName->value->type != V_ASN1_UTF8STRING)
+                        continue;
+                    alt_type = 0;
+                    cstr = gen->d.otherName->value->value.utf8string;
+                    break;
+                }
+                break;
+            case GEN_EMAIL:
+                if (check_type != GEN_EMAIL)
                     continue;
-            }
-            san_present = 1;
-            if (check_type == GEN_EMAIL)
                 cstr = gen->d.rfc822Name;
-            else if (check_type == GEN_DNS)
+                break;
+            case GEN_DNS:
+                if (check_type != GEN_DNS)
+                    continue;
                 cstr = gen->d.dNSName;
-            else
+                break;
+            case GEN_IPADD:
+                if (check_type != GEN_IPADD)
+                    continue;
                 cstr = gen->d.iPAddress;
+                break;
+            }
+            san_present = 1;
             /* Positive on success, negative on error! */
             if ((rv = do_check_string(cstr, alt_type, equal, flags,
                                       chk, chklen, peername)) != 0)

crypto/openssl/test/recipes/25-test_eai_data.t

@@ -21,16 +21,18 @@ setup("test_eai_data");
 #./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
 #./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
 
-plan tests => 12;
+plan tests => 16;
 
 require_ok(srctop_file('test','recipes','tconversion.pl'));
 my $folder = "test/recipes/25-test_eai_data";
 
 my $ascii_pem = srctop_file($folder, "ascii_leaf.pem");
 my $utf8_pem  = srctop_file($folder, "utf8_leaf.pem");
+my $kdc_pem   = srctop_file($folder, "kdc-cert.pem");
 
 my $ascii_chain_pem = srctop_file($folder, "ascii_chain.pem");
 my $utf8_chain_pem  = srctop_file($folder, "utf8_chain.pem");
+my $kdc_chain_pem  = srctop_file($folder, "kdc-root-cert.pem");
 
 my $out;
 my $outcnt = 0;
@@ -56,10 +58,18 @@ SKIP: {
 
 ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $ascii_chain_pem, $ascii_pem])));
 ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $utf8_chain_pem, $utf8_pem])));
+ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $kdc_chain_pem, $kdc_pem])));
 
 ok(!run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $ascii_chain_pem, $utf8_pem])));
 ok(!run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-CAfile", $utf8_chain_pem,  $ascii_pem])));
 
+# Check an otherName does not get misparsed as an DNS name, (should trigger ASAN errors if violated).
+ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-verify_hostname", 'mx1.example.com', "-CAfile", $kdc_chain_pem,  $kdc_pem])));
+# Check an otherName does not get misparsed as an email address, (should trigger ASAN errors if violated).
+ok(run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-verify_email", 'joe@example.com', "-CAfile", $kdc_chain_pem,  $kdc_pem])));
+# We expect SmtpUTF8Mailbox to be a UTF8 String, not an IA5String.
+ok(!run(app(["openssl", "verify", "-nameopt", "utf8", "-no_check_time", "-verify_email", 'moe@example.com', "-CAfile", $kdc_chain_pem,  $kdc_pem])));
+
 #Check that we get the expected failure return code
 with({ exit_checker => sub { return shift == 2; } },
      sub {

crypto/openssl/test/recipes/25-test_eai_data/kdc-cert.pem

@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDbDCCAlSgAwIBAgIBAjANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDARSb290
+MCAXDTI0MDYyMDA2MTQxNVoYDzIxMjQwNjIwMDYxNDE1WjAXMRUwEwYDVQQDDAxU
+RVNULkVYQU1QTEUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC6wfP+
+6go79dkpo/dGLMlPZ7Gw/Q6gUYrCWZWUEgEeRVHCrqOlgUEyA+PcWas/XDPUxXry
+BQlJHLvlqamAQn8gs4QPBARFYWKNiTVGyaRkgNA1N5gqyZdrP9UE+ZJmdqxRAAe8
+vvpGZWSgevPhLUiSCFYDiD0Rtji2Hm3rGUrReQFBQDEw2pNGwz9zIaxUs08kQZcx
+Yzyiplz5Oau+R/6sAgUwDlrD9xOlUxx/tA/MSDIfkK8qioU11uUZtO5VjkNQy/bT
+7zQMmXxWgm2MIgOs1u4YN7YGOtgqHE9v9iPHHfgrkbQDtVDGQsa8AQEhkUDSCtW9
+3VFAKx6dGNXYzFwfAgMBAAGjgcgwgcUwHQYDVR0OBBYEFFR5tZycW19DmtbL4Zqj
+te1c2vZLMAkGA1UdIwQCMAAwCQYDVR0TBAIwADCBjQYDVR0RBIGFMIGCoD8GBisG
+AQUCAqA1MDOgDhsMVEVTVC5FWEFNUExFoSEwH6ADAgEBoRgwFhsGa3JidGd0GwxU
+RVNULkVYQU1QTEWgHQYIKwYBBQUHCAmgERYPbW9lQGV4YW1wbGUuY29tgQ9qb2VA
+ZXhhbXBsZS5jb22CD214MS5leGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEA
+T0xzVtVpRtaOzIhgzw7XQUdzWD5UEGSJJ1cBCOmKUWwDLTAouCYLFB4TbEE7MMUb
+iuMy60bjmVtvfJIXorGUgSadRe5RWJ5DamJWvPA0Q9x7blnEcXqEF+9Td+ypevgU
+UYHFmg83OYwxOsFXZ5cRuXMk3WCsDHQIBi6D1L6oDDZ2pfArs5mqm3thQKVlqyl1
+El3XRYEdqAz/5eCOFNfwxF0ALxjxVr/Z50StUZU8I7Zfev6+kHhyrR7dqzYJImv9
+0fTCOBEMjIETDsrA70OxAMu4V16nrWZdJdvzblS2qrt97Omkj+2kiPAJFB76RpwI
+oDQ9fKfUOAmUFth2/R/eGA==
+-----END CERTIFICATE-----

crypto/openssl/test/recipes/25-test_eai_data/kdc-root-cert.pem

@@ -0,0 +1,16 @@
+-----BEGIN CERTIFICATE-----
+MIICnDCCAYQCCQCBswYcrlZSHjANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDARS
+b290MCAXDTI0MDYyMDA2MTQxNVoYDzIxMjQwNjIwMDYxNDE1WjAPMQ0wCwYDVQQD
+DARSb290MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqRj8S4kBbIUj
+61kZfi6nE35Q38U140+qt4uAiwAhKumfVHlBM0zQ98WFt5zMHIBQwIb3yjc2zj+0
+qzUnQfwm1r/RfcMmBPEti9Ge+aEMSsds2gMXziOFM8wd2aAFPy7UVE0XpEWofsRK
+MGi61MKVdPSbGIxBwY9VW38/7D/wf1HtJe7y0xpuecR7GB2XAs+qST59NjuF+7wS
+dLM8Hb3TATgeYbXXWsRJgwz+SPzExg5WmLnU+7y4brZ32dHtdSmkRVSgSlaIf7Xj
+3Tc6Zi7I+W/JYk7hy1zUexVdWCak4PHcoWrXe0gNNN/t8VfLfMExt5z/HIylXnU7
+pGUyqZlTGQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAHpLF1UCRy7b6Hk0rLokxI
+lgwiH9BU9mktigAGASvkbllpt+YbUbWnuYAvpHBGiP1qZtfX2r96UrSJaGO9BEzT
+Gp9ThnSjoj4Srul0+s/NArU22irFLmDzbalgevAmm9gMGkdqkiIm/mXbwrPj0ncl
+KGicevXryVpvaP62eZ8cc3C4p97frMmXxRX8sTdQpD/gRI7prdEILRSKveqT+AEW
+7rFGM5AOevb4U8ddop8A3D/kX0wcCAIBF6jCNk3uEJ57jVcagL04kPnVfdRiedTS
+vfq1DRNcD29d1H/9u0fHdSn1/+8Ep3X+afQ3C6//5NvOEaXcIGO4QSwkprQydfv8
+-----END CERTIFICATE-----

crypto/openssl/test/recipes/25-test_eai_data/kdc.sh

@@ -0,0 +1,41 @@
+#! /usr/bin/env bash
+
+# Create a root CA, signing a leaf cert with a KDC principal otherName SAN, and
+# also a non-UTF8 smtpUtf8Mailbox SAN followed by an rfc822Name SAN and a DNS
+# name SAN.  In the vulnerable EAI code, the KDC principal `otherName` should
+# trigger ASAN errors in DNS name checks, while the non-UTF8 `smtpUtf8Mailbox`
+# should likewise lead to ASAN issues with email name checks.
+
+rm -f root-key.pem root-cert.pem
+openssl req -nodes -new -newkey rsa:2048 -keyout kdc-root-key.pem \
+        -x509 -subj /CN=Root -days 36524 -out kdc-root-cert.pem
+
+exts=$(
+    printf "%s\n%s\n%s\n%s = " \
+        "subjectKeyIdentifier = hash" \
+        "authorityKeyIdentifier = keyid" \
+        "basicConstraints = CA:false" \
+        "subjectAltName"
+    printf "%s, " "otherName:1.3.6.1.5.2.2;SEQUENCE:kdc_princ_name"
+    printf "%s, " "otherName:1.3.6.1.5.5.7.8.9;IA5:moe@example.com"
+    printf "%s, " "email:joe@example.com"
+    printf "%s\n" "DNS:mx1.example.com"
+    printf "[kdc_princ_name]\n"
+    printf "realm = EXP:0, GeneralString:TEST.EXAMPLE\n"
+    printf "principal_name = EXP:1, SEQUENCE:kdc_principal_seq\n"
+    printf "[kdc_principal_seq]\n"
+    printf "name_type = EXP:0, INTEGER:1\n"
+    printf "name_string = EXP:1, SEQUENCE:kdc_principal_components\n"
+    printf "[kdc_principal_components]\n"
+    printf "princ1 = GeneralString:krbtgt\n"
+    printf "princ2 = GeneralString:TEST.EXAMPLE\n"
+    )
+
+printf "%s\n" "$exts"
+
+openssl req -nodes -new -newkey rsa:2048 -keyout kdc-key.pem \
+    -subj "/CN=TEST.EXAMPLE" |
+    openssl x509 -req -out kdc-cert.pem \
+        -CA "kdc-root-cert.pem" -CAkey "kdc-root-key.pem" \
+        -set_serial 2 -days 36524 \
+        -extfile <(printf "%s\n" "$exts")

sys/cam/ctl/ctl.c

@@ -5592,7 +5592,7 @@
 	} else {
 		if (lun->write_buffer == NULL) {
 			lun->write_buffer = malloc(CTL_WRITE_BUFFER_SIZE,
-			    M_CTL, M_WAITOK);
+			    M_CTL, M_WAITOK | M_ZERO);
 		}
 		ctsio->kern_data_ptr = lun->write_buffer + buffer_offset;
 	}
@@ -5631,21 +5631,24 @@
 		return (CTL_RETVAL_COMPLETE);
 	}
 
+	if (lun->write_buffer == NULL) {
+		lun->write_buffer = malloc(CTL_WRITE_BUFFER_SIZE,
+			    M_CTL, M_WAITOK | M_ZERO);
+	}
+
 	/*
-	 * If we've got a kernel request that hasn't been malloced yet,
-	 * malloc it and tell the caller the data buffer is here.
+	 * If this kernel request hasn't started yet, initialize the data
+	 * buffer to the correct region of the LUN's write buffer.  Note that
+	 * this doesn't set CTL_FLAG_ALLOCATED since this points into a
+	 * persistent buffer belonging to the LUN rather than a buffer
+	 * dedicated to this request.
 	 */
-	if ((ctsio->io_hdr.flags & CTL_FLAG_ALLOCATED) == 0) {
-		if (lun->write_buffer == NULL) {
-			lun->write_buffer = malloc(CTL_WRITE_BUFFER_SIZE,
-			    M_CTL, M_WAITOK);
-		}
+	if (ctsio->kern_data_ptr == NULL) {
 		ctsio->kern_data_ptr = lun->write_buffer + buffer_offset;
 		ctsio->kern_data_len = len;
 		ctsio->kern_total_len = len;
 		ctsio->kern_rel_offset = 0;
 		ctsio->kern_sg_entries = 0;
-		ctsio->io_hdr.flags |= CTL_FLAG_ALLOCATED;
 		ctsio->be_move_done = ctl_config_move_done;
 		ctl_datamove((union ctl_io *)ctsio);
 
@@ -7473,20 +7476,19 @@
 	case RSO_OPTIONS_OC_SA:
 		if ((ctl_cmd_table[opcode].flags & CTL_CMD_FLAG_SA5) == 0 ||
 		    service_action >= 32) {
-			ctl_set_invalid_field(/*ctsio*/ ctsio,
-					      /*sks_valid*/ 1,
-					      /*command*/ 1,
-					      /*field*/ 2,
-					      /*bit_valid*/ 1,
-					      /*bit*/ 2);
-			ctl_done((union ctl_io *)ctsio);
-			return (CTL_RETVAL_COMPLETE);
+			goto invalid;
 		}
-		/* FALLTHROUGH */
+		total_len = sizeof(struct scsi_report_supported_opcodes_one) + 32;
+		break;
 	case RSO_OPTIONS_OC_ASA:
+		if ((ctl_cmd_table[opcode].flags & CTL_CMD_FLAG_SA5) != 0 &&
+		    service_action >= 32) {
+			goto invalid;
+		}
 		total_len = sizeof(struct scsi_report_supported_opcodes_one) + 32;
 		break;
 	default:
+invalid:
 		ctl_set_invalid_field(/*ctsio*/ ctsio,
 				      /*sks_valid*/ 1,
 				      /*command*/ 1,
@@ -9304,14 +9306,8 @@
 	sense_ptr = (struct scsi_sense_data *)ctsio->kern_data_ptr;
 	ctsio->kern_sg_entries = 0;
 	ctsio->kern_rel_offset = 0;
-
-	/*
-	 * struct scsi_sense_data, which is currently set to 256 bytes, is
-	 * larger than the largest allowed value for the length field in the
-	 * REQUEST SENSE CDB, which is 252 bytes as of SPC-4.
-	 */
-	ctsio->kern_data_len = cdb->length;
-	ctsio->kern_total_len = cdb->length;
+	ctsio->kern_data_len = ctsio->kern_total_len =
+	    MIN(cdb->length, sizeof(*sense_ptr));
 
 	/*
 	 * If we don't have a LUN, we don't have any pending sense.

sys/cam/ctl/ctl_private.h

@@ -355,6 +355,14 @@ struct ctl_lun {
 	uint8_t				pr_res_type;
 	int				prevent_count;
 	uint32_t			*prevent;
+
+	/*
+	 * The READ_BUFFER and WRITE_BUFFER commands permit access to a logical
+	 * data buffer associated with a LUN.  Accesses to the data buffer do
+	 * not affect data stored on the storage medium.  To support this,
+	 * allocate a buffer on first use that persists until the LUN is
+	 * destroyed.
+	 */
 	uint8_t				*write_buffer;
 	struct ctl_devid		*lun_devid;
 	TAILQ_HEAD(tpc_lists, tpc_list) tpc_lists;

sys/contrib/libnv/bsd_nvpair.c

@@ -988,6 +988,10 @@ nvpair_unpack_string_array(bool isbe __unused, nvpair_t *nvp,
 	for (ii = 0; ii < nvp->nvp_nitems; ii++) {
 		len = strnlen(tmp, size - 1) + 1;
 		size -= len;
+		if (tmp[len - 1] != '\0') {
+			ERRNO_SET(EINVAL);
+			return (NULL);
+		}
 		if (size < 0) {
 			ERRNO_SET(EINVAL);
 			return (NULL);
@@ -999,7 +1003,7 @@ nvpair_unpack_string_array(bool isbe __unused, nvpair_t *nvp,
 		return (NULL);
 	}
 
-	value = nv_malloc(sizeof(*value) * nvp->nvp_nitems);
+	value = nv_calloc(nvp->nvp_nitems, sizeof(*value));
 	if (value == NULL)
 		return (NULL);
 
@@ -1092,7 +1096,7 @@ nvpair_unpack_nvlist_array(bool isbe __unused, nvpair_t *nvp,
 		return (NULL);
 	}
 
-	value = nv_malloc(nvp->nvp_nitems * sizeof(*value));
+	value = nv_calloc(nvp->nvp_nitems, sizeof(*value));
 	if (value == NULL)
 		return (NULL);
 
@@ -1330,10 +1334,10 @@ nvpair_create_bool_array(const char *name, const bool *value, size_t nitems)
 		return (NULL);
 	}
 
-	size = sizeof(value[0]) * nitems;
-	data = nv_malloc(size);
+	data = nv_calloc(nitems, sizeof(value[0]));
 	if (data == NULL)
 		return (NULL);
+	size = sizeof(value[0]) * nitems;
 
 	memcpy(data, value, size);
 	nvp = nvpair_allocv(name, NV_TYPE_BOOL_ARRAY, (uintptr_t)data,
@@ -1360,10 +1364,10 @@ nvpair_create_number_array(const char *name, const uint64_t *value,
 		return (NULL);
 	}
 
-	size = sizeof(value[0]) * nitems;
-	data = nv_malloc(size);
+	data = nv_calloc(nitems, sizeof(value[0]));
 	if (data == NULL)
 		return (NULL);
+	size = sizeof(value[0]) * nitems;
 
 	memcpy(data, value, size);
 	nvp = nvpair_allocv(name, NV_TYPE_NUMBER_ARRAY,
@@ -1393,7 +1397,7 @@ nvpair_create_string_array(const char *name, const char * const *value,
 
 	nvp = NULL;
 	datasize = 0;
-	data = nv_malloc(sizeof(value[0]) * nitems);
+	data = nv_calloc(nitems, sizeof(value[0]));
 	if (data == NULL)
 		return (NULL);
 
@@ -1440,7 +1444,7 @@ nvpair_create_nvlist_array(const char *name, const nvlist_t * const *value,
 		return (NULL);
 	}
 
-	nvls = nv_malloc(sizeof(value[0]) * nitems);
+	nvls = nv_calloc(nitems, sizeof(value[0]));
 	if (nvls == NULL)
 		return (NULL);
 
@@ -1507,7 +1511,7 @@ nvpair_create_descriptor_array(const char *name, const int *value,
 
 	nvp = NULL;
 
-	fds = nv_malloc(sizeof(value[0]) * nitems);
+	fds = nv_calloc(nitems, sizeof(value[0]));
 	if (fds == NULL)
 		return (NULL);
 	for (ii = 0; ii < nitems; ii++) {