Reimplement DNS identifier validation.

bavshin-f5 · bavshin-f5 · commit f7cff1a61f2f · 2025-12-05T15:12:06.000-07:00
The signature of ngx_http_validate_host has changed in NGINX 1.29.4,
breaking the build of the module.  Instead of continuing to abuse an
internal API and wrapping it with version check, this commit
reimplements the validation in a more suitable for the module way.

Notable differences from the previous behavior:

 - Minimal supported NGINX version is now aligned to the ngx crate: 1.22

 - The new logic mostly follows ngx_http_validate_host behavior as of
   NGINX 1.29.4, and may reject some previously accepted values.

 - Wildcard DNS identifiers are now permitted.  Some ACME servers allow
   validating wildcard identifiers with challenges other than dns-01.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -61,11 +61,13 @@ jobs:
             build: aws-lc-static
 
         include:
+          # minimal supported nginx version
           - runner: ubuntu
             rust-version: stable
-            nginx-ref: stable-1.26
+            nginx-ref: stable-1.22
             build: debug
 
+          # minimal supported Rust version
           - runner: ubuntu
             rust-version: ${{ needs.rust-version.outputs.version }}
             nginx-ref: stable-1.28
diff --git a/README.md b/README.md
@@ -28,7 +28,7 @@ The module implements following specifications:
 
 ### Requirements
 
-- NGINX sources, 1.25.0 or later.
+- NGINX sources, 1.22.0 or later.
 - Regular NGINX build dependencies: C compiler, make, PCRE2, Zlib
 - System-wide installation of OpenSSL 1.1.1 or later
 - Rust toolchain (1.81.0 or later)
diff --git a/src/acme/solvers/tls_alpn.rs b/src/acme/solvers/tls_alpn.rs
@@ -27,7 +27,7 @@ use core::ffi::{c_int, c_uint, c_void, CStr};
 use core::net::{Ipv4Addr, Ipv6Addr};
 use core::ptr;
 
-use nginx_sys::{ngx_conf_t, ngx_connection_t, ngx_http_validate_host, ngx_str_t, NGX_LOG_WARN};
+use nginx_sys::{ngx_conf_t, ngx_connection_t, ngx_str_t, NGX_LOG_WARN};
 use ngx::allocator::Allocator;
 use ngx::collections::RbTreeMap;
 use ngx::core::{NgxString, SlabPool, Status};
@@ -408,22 +408,16 @@ unsafe extern "C" fn acme_ssl_cert_cb(ssl: *mut SSL, arg: *mut c_void) -> c_int
         return 0;
     };
 
-    let name = unsafe { SSL_get_servername(ssl, openssl_sys::TLSEXT_NAMETYPE_host_name as _) };
-    if name.is_null() {
+    let Some(mut name) = (unsafe {
+        SSL_get_servername(ssl, openssl_sys::TLSEXT_NAMETYPE_host_name as _)
+            .as_ref()
+            .map(|x| CStr::from_ptr(x).to_bytes())
+            // Reallocate, because we will need to mutate the name in place.
+            .and_then(|x| ngx_str_t::from_bytes(c.pool, x))
+    }) else {
         return 0;
-    }
-
-    let mut name = ngx_str_t {
-        data: name.cast_mut().cast(),
-        len: unsafe { CStr::from_ptr(name).count_bytes() },
     };
 
-    // Validate `name` and reallocate on the connection pool.
-    if !Status(unsafe { ngx_http_validate_host(&mut name, c.pool, 1) }).is_ok() {
-        ngx_log_error!(NGX_LOG_WARN, c.log, "acme/tls-alpn-01: invalid server name");
-        return 0;
-    }
-
     let id = match acme_parse_ssl_server_name(&mut name) {
         Ok(x) => x,
         Err(err) => {
@@ -496,6 +490,8 @@ enum ParseIdentifierError {
     InvalidV4Ptr,
     #[error("invalid IPv6 reverse mapping")]
     InvalidV6Ptr,
+    #[error("invalid name")]
+    Name,
     #[error(transparent)]
     Utf8(#[from] core::str::Utf8Error),
 }
@@ -504,6 +500,8 @@ enum ParseIdentifierError {
 fn acme_parse_ssl_server_name(
     name: &mut ngx_str_t,
 ) -> Result<Identifier<&str>, ParseIdentifierError> {
+    name.as_bytes_mut().make_ascii_lowercase();
+
     if let Some(v4) = name.strip_suffix(".in-addr.arpa") {
         // RFC1035 § 3.5 encoded IPv4 address.
 
@@ -572,7 +570,12 @@ fn acme_parse_ssl_server_name(
 
         Ok(Identifier::Ip(name.to_str()?))
     } else {
-        Ok(Identifier::Dns(name.to_str()?))
+        let name = name.to_str()?;
+        if !name.is_ascii() {
+            return Err(ParseIdentifierError::Name);
+        }
+
+        Ok(Identifier::Dns(name))
     }
 }
 
@@ -676,7 +679,9 @@ mod tests {
 
         let pairs: &[(&str, Result<Identifier<&str>, _>)] = &[
             ("example.com", Ok(Identifier::Dns("example.com"))),
+            ("EXAMPLE.COM", Ok(Identifier::Dns("example.com"))),
             ("1.0.0.127.in-addr.arpa", Ok(Identifier::Ip("127.0.0.1"))),
+            ("1.0.0.127.IN-ADDR.ARPA", Ok(Identifier::Ip("127.0.0.1"))),
             ("1.0.0.0.127.in-addr.arpa", Err(Error::InvalidV4Ptr)),
             ("1.0..0.127.in-addr.arpa", Err(Error::InvalidV4Ptr)),
             ("256.0.0.127.in-addr.arpa", Err(Error::InvalidV4Ptr)),
@@ -685,6 +690,10 @@ mod tests {
                 "1.a.b.c.d.e.f.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.e.f.ip6.arpa",
                 Ok(Identifier::Ip("fe80::fed:cba1")),
             ),
+            (
+                "1.A.B.C.D.E.F.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.E.F.IP6.ARPA",
+                Ok(Identifier::Ip("fe80::fed:cba1")),
+            ),
             (
                 "1.a.b.c.d.e.f.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.e.f.ip6.arpa",
                 Err(Error::InvalidV6Ptr),
diff --git a/src/conf/order.rs b/src/conf/order.rs
@@ -7,10 +7,10 @@ use core::fmt;
 use core::hash::{self, Hash, Hasher};
 use core::str::Utf8Error;
 
-use nginx_sys::{ngx_conf_t, ngx_http_server_name_t, ngx_str_t};
+use nginx_sys::{ngx_conf_t, ngx_http_server_name_t};
 use ngx::allocator::{AllocError, Allocator, TryCloneIn};
 use ngx::collections::Vec;
-use ngx::core::{NgxString, Pool, Status};
+use ngx::core::{NgxString, Pool};
 use ngx::ngx_log_error;
 use siphasher::sip::SipHasher;
 use thiserror::Error;
@@ -136,12 +136,10 @@ where
 pub enum IdentifierError {
     #[error("memory allocation failed")]
     Alloc(#[from] AllocError),
-    #[error("invalid server name")]
-    Invalid,
+    #[error("invalid server name: {0}")]
+    Invalid(#[from] NameError),
     #[error("invalid UTF-8 string")]
     Utf8(#[from] Utf8Error),
-    #[error("unsupported wildcard server name")]
-    Wildcard,
 }
 
 impl CertificateOrder<&'static str, Pool> {
@@ -193,37 +191,39 @@ impl CertificateOrder<&'static str, Pool> {
             return self.push(Identifier::Ip(addr)).map_err(Into::into);
         }
 
-        if value.contains('*') {
-            return Err(IdentifierError::Wildcard);
-        }
-
-        let host = validate_host(cf, value).map_err(|st| {
-            if st == Status::NGX_ERROR {
-                IdentifierError::Alloc(AllocError)
-            } else {
-                IdentifierError::Invalid
-            }
-        })?;
+        let realloc = validate_dns_identifier(value)?;
 
         /*
          * The only special syntax we want to support is a leading dot, which matches the domain
          * with "www." and without it.
          * See <https://nginx.org/en/docs/http/server_names.html>
          */
 
-        if let Some(host) = host.strip_prefix(".") {
+        if let Some(host) = value.strip_prefix(".") {
             let mut www = Vec::new_in(self.identifiers.allocator().clone());
             www.try_reserve_exact(host.len() + 4)
                 .map_err(|_| AllocError)?;
             www.extend_from_slice(b"www.");
             www.extend_from_slice(host.as_bytes());
+            www.make_ascii_lowercase();
+            // SAFETY: www is a pre-validated ASCII character slice.
             // The buffer is owned by ngx_pool_t and does not leak.
-            let www = core::str::from_utf8(www.leak())?;
+            let www = unsafe { core::str::from_utf8_unchecked(www.leak()) };
 
             self.push(Identifier::Dns(www))?;
-            self.push(Identifier::Dns(host))?;
+            self.push(Identifier::Dns(&www[4..]))?;
+        } else if realloc {
+            let mut out = Vec::new_in(self.identifiers.allocator().clone());
+            out.try_reserve_exact(value.len()).map_err(|_| AllocError)?;
+            out.extend_from_slice(value.as_bytes());
+            out.make_ascii_lowercase();
+            // SAFETY: out is a pre-validated ASCII character slice.
+            // The buffer is owned by ngx_pool_t and does not leak.
+            let out = unsafe { core::str::from_utf8_unchecked(out.leak()) };
+
+            self.push(Identifier::Dns(out))?;
         } else {
-            self.push(Identifier::Dns(host))?;
+            self.push(Identifier::Dns(value))?;
         }
 
         Ok(())
@@ -289,17 +289,119 @@ fn parse_ip_identifier(
     Ok(Some(out))
 }
 
-/// Checks if the value is a valid domain name and returns a canonical (lowercase) form,
-/// reallocated on the configuration pool if necessary.
-fn validate_host(cf: &ngx_conf_t, host: &'static str) -> Result<&'static str, Status> {
-    let mut host = ngx_str_t {
-        data: host.as_ptr().cast_mut(),
-        len: host.len(),
-    };
-    let rc = Status(unsafe { nginx_sys::ngx_http_validate_host(&mut host, cf.pool, 0) });
-    if rc != Status::NGX_OK {
-        return Err(rc);
+#[derive(Debug, Error, PartialEq)]
+pub enum NameError {
+    #[error("invalid character {0:x} at position {1}")]
+    Invalid(u8, usize),
+    #[error("unexpected character '{0}' at position {1}")]
+    Unexpected(char, usize),
+    #[error("does not end with a label")]
+    NotALabel,
+}
+
+/// Validates that the `name` can be used as a DNS identifier.
+///
+/// RFC8555 § 7.1.4 states that the "dns" identifier is a fully qualified domain name,
+/// encoded according to the rules in RFC5280 § 7.
+/// In practice, existing ACME servers and clients are rather relaxed about this requirement,
+/// trusting the user to specify valid values and the underlying protocol implementations (DNS or
+/// HTTP) to reject invalid ones.
+///
+/// Given that, the validation logic is loosely based on the `ngx_http_validate_host` as of 1.29.4,
+/// asserting that the name
+///
+///  - is a printable ASCII string, as required both by ACME and by NGINX[^1]
+///  - is a valid Host value accepted by NGINX
+///  - can have a leading dot or a single leading wildcard label
+///  - ends with a non-empty label
+///
+/// Returns true if the name needs to be reallocated and converted to lower case.
+///
+/// [^1]: https://nginx.org/en/docs/http/server_names.html
+fn validate_dns_identifier(name: &str) -> Result<bool, NameError> {
+    #[derive(PartialEq, Eq)]
+    enum State {
+        Start,
+        Label,
+        Dot,
+        Wildcard,
+    }
+
+    let mut alloc = false;
+    let mut state = State::Start;
+
+    for (i, ch) in name.bytes().enumerate() {
+        state = match ch {
+            // non-printable - handle separately for better error formatting
+            0x00..=0x20 | 0x7f.. => return Err(NameError::Invalid(ch, i)),
+
+            b'*' if state == State::Start => State::Wildcard,
+            b'.' if state != State::Dot => State::Dot,
+            // A wildcard domain name consists of a single asterisk character followed by a single
+            // full stop character ("*.") followed by a domain name (RFC8555 § 7.1.3)
+            _ if state == State::Wildcard => return Err(NameError::Unexpected(ch as _, i)),
+
+            // unreserved
+            b'A'..=b'Z' => {
+                alloc = true;
+                State::Label
+            }
+            b'0'..=b'9' | b'a'..=b'z' | b'-' | b'_' | b'~' => State::Label,
+
+            // pct-encoded
+            b'%' => State::Label,
+
+            // sub-delims
+            b'!' | b'$' | b'&' | b'\'' | b'(' | b')' | b'+' | b',' | b';' | b'=' => State::Label,
+
+            _ => return Err(NameError::Unexpected(ch as _, i)),
+        };
     }
 
-    unsafe { super::conf_value_to_str(&host) }.map_err(|_| Status::NGX_ERROR)
+    // We intentionally reject the trailing dot, as it should not appear in the TLS layer.
+
+    if state != State::Label {
+        return Err(NameError::NotALabel);
+    }
+
+    Ok(alloc)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_validate_dns_identifier() {
+        for (name, expect) in [
+            ("example", Ok(false)),
+            ("_example", Ok(false)),
+            ("exam_ple", Ok(false)),
+            ("Example", Ok(true)),
+            ("Example.", Err(NameError::NotALabel)),
+            ("E\x10ample", Err(NameError::Invalid(0x10, 1))),
+            ("00.example.test", Ok(false)),
+            ("www1.example.Test", Ok(true)),
+            ("www.example..test", Err(NameError::Unexpected('.', 12))),
+            ("www.example.test.", Err(NameError::NotALabel)),
+            ("пример.испытание", Err(NameError::Invalid(0xd0, 0))),
+            ("xn--e1afmkfd.xn--80akhbyknj4f", Ok(false)),
+            // wildcards
+            ("*", Err(NameError::NotALabel)),
+            ("*.example.test", Ok(false)),
+            ("*.xn--e1afmkfd.xn--80akhbyknj4f", Ok(false)),
+            ("*ww.example.test", Err(NameError::Unexpected('w', 1))),
+            ("ww*.example.test", Err(NameError::Unexpected('*', 2))),
+            ("www.example.*", Err(NameError::Unexpected('*', 12))),
+            // incorrect syntax
+            ("", Err(NameError::NotALabel)),
+            ("*.", Err(NameError::NotALabel)),
+        ] {
+            assert_eq!(
+                super::validate_dns_identifier(name),
+                expect,
+                "incorrect validation result for \"{name}\"",
+            );
+        }
+    }
 }
diff --git a/t/acme_conf_certificate.t b/t/acme_conf_certificate.t
@@ -24,7 +24,7 @@ use Test::Nginx;
 select STDERR; $| = 1;
 select STDOUT; $| = 1;
 
-my $t = Test::Nginx->new()->has(qw/http http_ssl/)->plan(4);
+my $t = Test::Nginx->new()->has(qw/http http_ssl/)->plan(6);
 
 use constant TEMPLATE_CONF => <<'EOF';
 
@@ -80,6 +80,22 @@ acme_certificate example;
 EOF
 
 
+like(check($t, <<'EOF' ), qr/\[emerg].*invalid server name/, 'bad name - 1');
+
+server_name www.example.*;
+acme_certificate example;
+
+EOF
+
+
+like(check($t, <<'EOF' ), qr/\[emerg].*invalid server name/, 'bad name - 2');
+
+server_name .example..test;
+acme_certificate example;
+
+EOF
+
+
 like(check($t, <<'EOF' ), qr/\[emerg].*no identifiers/, 'no identifiers');
 
 acme_certificate example;
