Ignore `server_name` extension containing IP address

This works around quality-of-implementation issues in OpenSSL and
Apple SecureTransport: they send `server_name` extensions containing
IP addresses.  RFC6066 specifically disallows that.

It is a similar work-around to that adopted by LibreSSL: ignore
SNI contents if they can be parsed as an IP address.

rustls/src/lib.rs

@@ -443,7 +443,8 @@ pub mod internal {
             pub use crate::msgs::handshake::{
                 CertificateChain, ClientExtension, ClientHelloPayload, DistinguishedName,
                 EchConfig, EchConfigContents, HandshakeMessagePayload, HandshakePayload,
-                HpkeKeyConfig, HpkeSymmetricCipherSuite, KeyShareEntry, Random, SessionId,
+                HpkeKeyConfig, HpkeSymmetricCipherSuite, KeyShareEntry, Random, ServerName,
+                SessionId,
             };
         }
         pub mod message {

rustls/src/msgs/handshake.rs

@@ -216,6 +216,7 @@ impl TlsListElement for SignatureScheme {
 #[derive(Clone, Debug)]
 pub(crate) enum ServerNamePayload {
     HostName(DnsName<'static>),
+    IpAddress(PayloadU16),
     Unknown(Payload<'static>),
 }
 
@@ -225,11 +226,13 @@ impl ServerNamePayload {
     }
 
     fn read_hostname(r: &mut Reader) -> Result<Self, InvalidMessage> {
+        use pki_types::ServerName;
         let raw = PayloadU16::read(r)?;
 
-        match DnsName::try_from(raw.0.as_slice()) {
-            Ok(dns_name) => Ok(Self::HostName(dns_name.to_owned())),
-            Err(_) => {
+        match ServerName::try_from(raw.0.as_slice()) {
+            Ok(ServerName::DnsName(d)) => Ok(Self::HostName(d.to_owned())),
+            Ok(ServerName::IpAddress(_)) => Ok(Self::IpAddress(raw)),
+            Ok(_) | Err(_) => {
                 warn!(
                     "Illegal SNI hostname received {:?}",
                     String::from_utf8_lossy(&raw.0)
@@ -245,6 +248,7 @@ impl ServerNamePayload {
                 (name.as_ref().len() as u16).encode(bytes);
                 bytes.extend_from_slice(name.as_ref().as_bytes());
             }
+            Self::IpAddress(ref r) => r.encode(bytes),
             Self::Unknown(ref r) => r.encode(bytes),
         }
     }
@@ -864,7 +868,23 @@ impl ClientHelloPayload {
     pub(crate) fn sni_extension(&self) -> Option<&[ServerName]> {
         let ext = self.find_extension(ExtensionType::ServerName)?;
         match *ext {
-            ClientExtension::ServerName(ref req) => Some(req),
+            // Does this comply with RFC6066?
+            //
+            // [RFC6066][] specifies that literal IP addresses are illegal in
+            // `ServerName`s with a `name_type` of `host_name`.
+            //
+            // Some clients incorrectly send such extensions: we choose to
+            // successfully parse these (into `ServerNamePayload::IpAddress`)
+            // but then act like the client sent no `server_name` extension.
+            //
+            // [RFC6066]: https://datatracker.ietf.org/doc/html/rfc6066#section-3
+            ClientExtension::ServerName(ref req)
+                if !req
+                    .iter()
+                    .any(|name| matches!(name.payload, ServerNamePayload::IpAddress(_))) =>
+            {
+                Some(req)
+            }
             _ => None,
         }
     }

rustls/src/msgs/message_test.rs

@@ -48,7 +48,7 @@ fn test_read_fuzz_corpus() {
 }
 
 #[test]
-fn can_read_safari_client_hello() {
+fn can_read_safari_client_hello_with_ip_address_in_sni_extension() {
     let _ = env_logger::Builder::new()
         .filter(None, log::LevelFilter::Trace)
         .try_init();
@@ -72,7 +72,7 @@ fn can_read_safari_client_hello() {
     let mut rd = Reader::init(bytes);
     let m = OutboundOpaqueMessage::read(&mut rd).unwrap();
     println!("m = {:?}", m);
-    assert!(Message::try_from(m.into_plain_message()).is_err());
+    Message::try_from(m.into_plain_message()).unwrap();
 }
 
 #[test]