🐛 fix setting assert_hostname to False triggered an error when the pe…

…er certificate contained at least one IP in subject alt names (#29)
jawah · Apr 20, 2024 · befa8b3 · befa8b3
1 parent 736e457
commit befa8b3
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 23 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -1,3 +1,9 @@
+1.0.3 (2024-04-20)
+=====================
+
+**Fixed**
+- setting assert_hostname to False triggered an error when the peer certificate contained at least one IP in subject alt names.
+
 1.0.2 (2024-04-20)
 =====================
 

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "qh3"
-version = "1.0.2"
+version = "1.0.3"
 edition = "2021"
 rust-version = "1.75"
 license = "BSD-3"

diff --git a/qh3/__init__.py b/qh3/__init__.py
@@ -11,7 +11,7 @@
 from .quic.packet import QuicProtocolVersion
 from .tls import CipherSuite, SessionTicket
 
-__version__ = "1.0.2"
+__version__ = "1.0.3"
 
 __all__ = (
     "connect",

diff --git a/qh3/tls.py b/qh3/tls.py
@@ -376,10 +376,29 @@ def verify_certificate(
                                 authorities.append(cert.public_bytes())
 
     if server_name is None or assert_server_name is False:
+        # get_subject_alt_names()... caution for :
+        # IPAddress(20:01:48:60:48:60:00:00:00:00:00:00:00:00:00:64)
+        # or..
+        # IPAddress(08:08:08:08)
         for alt_name in certificate.get_subject_alt_names():
-            server_name = alt_name.decode()
-            server_name = server_name[server_name.find("(") + 1 : server_name.find(")")]
-            server_name.replace("*.", "unverified.")
+            server_name_candidate = alt_name.decode()
+            server_name_candidate = server_name_candidate[
+                server_name_candidate.find("(") + 1 : server_name_candidate.find(")")
+            ]
+            server_name_candidate.replace("*.", "unverified.")
+
+            if ":" in server_name_candidate:
+                if len(server_name_candidate) == 11:
+                    server_name = ".".join(
+                        str(int(p)) for p in server_name_candidate.split(":")
+                    )
+                else:
+                    continue
+
+            else:
+                server_name = server_name_candidate
+
+            break
 
     if server_name is None:
         raise AlertBadCertificate("unable to determine server name target")

diff --git a/src/certificate.rs b/src/certificate.rs
@@ -344,6 +344,7 @@ impl ServerVerifier {
         }
     }
 
+    #[allow(unreachable_code)]
     pub fn verify(&mut self, peer: &PyBytes, intermediaries: Vec<&PyBytes>, server_name: String) -> PyResult<()> {
         let peer_der = CertificateDer::from(peer.as_bytes());
         let mut intermediaries_der = Vec::new();
@@ -352,26 +353,36 @@ impl ServerVerifier {
             intermediaries_der.push(CertificateDer::from(intermediary.as_bytes()));
         }
 
-        let res = self.inner.verify_server_cert(
-            &peer_der,
-            &intermediaries_der,
-            &ServerName::try_from(server_name).expect("invalid DNS name"),
-            &[],
-            UnixTime::now(),
-        );
+        let parsed_name_res = ServerName::try_from(server_name);
 
-        match res {
-            Ok(_) => Ok(()),
-            Err(Error::InvalidCertificate(err)) => {
-                match err {
-                    CertificateError::UnknownIssuer => Err(SelfSignedCertificateError::new_err("unable to get local issuer certificate")),
-                    CertificateError::NotValidForName => Err(InvalidNameCertificateError::new_err("invalid server name for certificate")),
-                    CertificateError::Expired => Err(ExpiredCertificateError::new_err("server certificate expired")),
-                    CertificateError::NotValidYet => Err(ExpiredCertificateError::new_err("server certificate is not yet valid")),
-                    _ => Err(UnacceptableCertificateError::new_err("the server certificate is unacceptable"))
+        return match parsed_name_res {
+            Ok(parsed_name) => {
+                let res = self.inner.verify_server_cert(
+                    &peer_der,
+                    &intermediaries_der,
+                    &parsed_name,
+                    &[],
+                    UnixTime::now(),
+                );
+
+                return match res {
+                    Ok(_) => Ok(()),
+                    Err(Error::InvalidCertificate(err)) => {
+                        match err {
+                            CertificateError::UnknownIssuer => Err(SelfSignedCertificateError::new_err("unable to get local issuer certificate")),
+                            CertificateError::NotValidForName => Err(InvalidNameCertificateError::new_err("invalid server name for certificate")),
+                            CertificateError::Expired => Err(ExpiredCertificateError::new_err("server certificate expired")),
+                            CertificateError::NotValidYet => Err(ExpiredCertificateError::new_err("server certificate is not yet valid")),
+                            _ => Err(UnacceptableCertificateError::new_err("the server certificate is unacceptable"))
+                        }
+                    },
+                    Err(_) => Err(CryptoError::new_err("the x509 certificate store encountered an error"))
                 }
             },
-            Err(_) => Err(CryptoError::new_err("the x509 certificate store encountered an error"))
+            Err(_) => {
+                return Err(InvalidNameCertificateError::new_err("unparseable server name"));
+            }
         }
+
     }
 }