From d355cb80385dcfbab03805fc9407d5a4db11db7a Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfackler@gmail.com>
Date: Thu, 30 Mar 2023 19:25:05 -0400
Subject: [PATCH] Don't use IP addresses in SNI

---
 openssl/src/ssl/connector.rs |  5 ++--
 openssl/src/ssl/test/mod.rs  | 57 +++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/openssl/src/ssl/connector.rs b/openssl/src/ssl/connector.rs
index 39f729df90..66d1bd8939 100644
--- a/openssl/src/ssl/connector.rs
+++ b/openssl/src/ssl/connector.rs
@@ -11,6 +11,7 @@ use crate::ssl::{
     SslOptions, SslRef, SslStream, SslVerifyMode,
 };
 use crate::version;
+use std::net::IpAddr;
 
 const FFDHE_2048: &str = "
 -----BEGIN DH PARAMETERS-----
@@ -177,9 +178,9 @@ impl ConnectConfiguration {
 
     /// Returns an `Ssl` configured to connect to the provided domain.
     ///
-    /// The domain is used for SNI and hostname verification if enabled.
+    /// The domain is used for SNI (if it is not an IP address) and hostname verification if enabled.
     pub fn into_ssl(mut self, domain: &str) -> Result<Ssl, ErrorStack> {
-        if self.sni {
+        if self.sni && domain.parse::<IpAddr>().is_err() {
             self.ssl.set_hostname(domain)?;
         }
 
diff --git a/openssl/src/ssl/test/mod.rs b/openssl/src/ssl/test/mod.rs
index 03dc89e5c3..a34309a7d6 100644
--- a/openssl/src/ssl/test/mod.rs
+++ b/openssl/src/ssl/test/mod.rs
@@ -21,10 +21,10 @@ use crate::hash::MessageDigest;
 use crate::ocsp::{OcspResponse, OcspResponseStatus};
 use crate::pkey::PKey;
 use crate::srtp::SrtpProfileId;
-use crate::ssl;
 use crate::ssl::test::server::Server;
 #[cfg(any(ossl110, ossl111, libressl261))]
 use crate::ssl::SslVersion;
+use crate::ssl::{self, NameType, SslConnectorBuilder};
 #[cfg(ossl111)]
 use crate::ssl::{ClientHelloResponse, ExtensionContext};
 use crate::ssl::{
@@ -767,6 +767,61 @@ fn connector_can_disable_verify() {
     s.read_exact(&mut [0]).unwrap();
 }
 
+#[test]
+fn connector_does_use_sni_with_dnsnames() {
+    static CALLED_BACK: AtomicBool = AtomicBool::new(false);
+
+    let mut builder = Server::builder();
+    builder.ctx().set_servername_callback(|ssl, _| {
+        assert_eq!(ssl.servername(NameType::HOST_NAME), Some("foobar.com"));
+        CALLED_BACK.store(true, Ordering::SeqCst);
+        Ok(())
+    });
+    let server = builder.build();
+
+    let mut connector = SslConnector::builder(SslMethod::tls()).unwrap();
+    connector.set_ca_file("test/root-ca.pem").unwrap();
+
+    let s = server.connect_tcp();
+    let mut s = connector
+        .build()
+        .configure()
+        .unwrap()
+        .connect("foobar.com", s)
+        .unwrap();
+    s.read_exact(&mut [0]).unwrap();
+
+    assert!(CALLED_BACK.load(Ordering::SeqCst));
+}
+
+#[test]
+fn connector_doesnt_use_sni_with_ips() {
+    static CALLED_BACK: AtomicBool = AtomicBool::new(false);
+
+    let mut builder = Server::builder();
+    builder.ctx().set_servername_callback(|ssl, _| {
+        assert_eq!(ssl.servername(NameType::HOST_NAME), None);
+        CALLED_BACK.store(true, Ordering::SeqCst);
+        Ok(())
+    });
+    let server = builder.build();
+
+    let mut connector = SslConnector::builder(SslMethod::tls()).unwrap();
+    // The server's cert isn't issued for 127.0.0.1 but we don't care for this test.
+    connector.set_verify(SslVerifyMode::NONE);
+
+    let s = server.connect_tcp();
+    let mut s = connector
+        .build()
+        .configure()
+        .unwrap()
+        .connect("127.0.0.1", s)
+        .unwrap();
+    s.read_exact(&mut [0]).unwrap();
+
+    assert!(CALLED_BACK.load(Ordering::SeqCst));
+}
+
 fn test_mozilla_server(new: fn(SslMethod) -> Result<SslAcceptorBuilder, ErrorStack>) {
     let listener = TcpListener::bind("127.0.0.1:0").unwrap();
     let port = listener.local_addr().unwrap().port();