Skip to content

Commit

Permalink
Introduce X509CheckFlags::UNDERSCORE_WILDCARDS
Browse files Browse the repository at this point in the history
  • Loading branch information
nox committed Dec 18, 2023
1 parent 3df4054 commit 2928982
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 8 deletions.
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,15 @@ jobs:
name: Run `rpk` tests
- run: cargo test --features pq-experimental
name: Run `pq-experimental` tests
- run: cargo test --features underscore-wildcards
name: Run `underscore-wildcards` tests
- run: cargo test --features pq-experimental,rpk
name: Run `pq-experimental,rpk` tests
- run: cargo test --features kx-safe-default,pq-experimental
name: Run `kx-safe-default` tests
- run: cargo test --features pq-experimental,underscore-wildcards
name: Run `pq-experimental,underscore-wildcards` tests
- run: cargo test --features rpk,underscore-wildcards
name: Run `rpk,underscore-wildcards` tests
- run: cargo test --features pq-experimental,rpk,underscore-wildcards
name: Run `pq-experimental,rpk,underscore-wildcards` tests
7 changes: 6 additions & 1 deletion boring-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ include = [
]

[package.metadata.docs.rs]
features = ["rpk", "pq-experimental"]
features = ["rpk", "pq-experimental", "underscore-wildcards"]
rustdoc-args = ["--cfg", "docsrs"]

[features]
Expand All @@ -71,6 +71,11 @@ rpk = []
# can be provided by setting `BORING_BSSL{,_FIPS}_SOURCE_PATH`.
pq-experimental = []

# Applies a patch (`patches/underscore-wildcards.patch`) to enable
# `ffi::X509_CHECK_FLAG_UNDERSCORE_WILDCARDS`. Same caveats as
# those for `pq-experimental` feature apply.
underscore-wildcards = []

[build-dependencies]
bindgen = { workspace = true }
cmake = { workspace = true }
Expand Down
8 changes: 7 additions & 1 deletion boring-sys/build/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub(crate) struct Features {
pub(crate) fips_link_precompiled: bool,
pub(crate) pq_experimental: bool,
pub(crate) rpk: bool,
pub(crate) underscore_wildcards: bool,
}

pub(crate) struct Env {
Expand Down Expand Up @@ -82,7 +83,10 @@ impl Config {
);
}

let features_with_patches_enabled = self.features.rpk || self.features.pq_experimental;
let features_with_patches_enabled = self.features.rpk
|| self.features.pq_experimental
|| self.features.underscore_wildcards;

let patches_required = features_with_patches_enabled && !self.env.assume_patched;
let build_from_sources_required = self.features.fips_link_precompiled || patches_required;

Expand All @@ -98,12 +102,14 @@ impl Features {
let fips_link_precompiled = env::var_os("CARGO_FEATURE_FIPS_LINK_PRECOMPILED").is_some();
let pq_experimental = env::var_os("CARGO_FEATURE_PQ_EXPERIMENTAL").is_some();
let rpk = env::var_os("CARGO_FEATURE_RPK").is_some();
let underscore_wildcards = env::var_os("CARGO_FEATURE_UNDERSCORE_WILDCARDS").is_some();

Self {
fips,
fips_link_precompiled,
pq_experimental,
rpk,
underscore_wildcards,
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions boring-sys/build/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,11 @@ fn ensure_patches_applied(config: &Config) -> io::Result<()> {
apply_patch(config, "rpk.patch")?;
}

if config.features.underscore_wildcards {
println!("cargo:warning=applying underscore wildcards patch to boringssl");
apply_patch(config, "underscore-wildcards.patch")?;
}

Ok(())
}

Expand Down
61 changes: 61 additions & 0 deletions boring-sys/patches/underscore-wildcards.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
https://github.com/google/boringssl/compare/master...nox:boringssl:underscore-wildcards

--- a/src/crypto/x509v3/v3_utl.c
+++ b/src/crypto/x509v3/v3_utl.c
@@ -790,7 +790,9 @@ static int wildcard_match(const unsigned char *prefix, size_t prefix_len,
// Check that the part matched by the wildcard contains only
// permitted characters and only matches a single label.
for (p = wildcard_start; p != wildcard_end; ++p) {
- if (!OPENSSL_isalnum(*p) && *p != '-') {
+ if (!OPENSSL_isalnum(*p) && *p != '-' &&
+ !(*p == '_' &&
+ (flags & X509_CHECK_FLAG_UNDERSCORE_WILDCARDS))) {
return 0;
}
}
--- a/src/crypto/x509/x509_test.cc
+++ b/src/crypto/x509/x509_test.cc
@@ -4500,6 +4500,31 @@ TEST(X509Test, Names) {
/*invalid_emails=*/{},
/*flags=*/0,
},
+
+ // Underscores in DNS names are forbidden by default.
+ {
+ /*cert_subject=*/{},
+ /*cert_dns_names=*/{"*.example.com"},
+ /*cert_emails=*/{},
+ /*valid_dns_names=*/{},
+ /*invalid_dns_names=*/{"not_allowed.example.com"},
+ /*valid_emails=*/{},
+ /*invalid_emails=*/{},
+ /*flags=*/0,
+ },
+
+ // Underscores in DNS names can be allowed with the right flag.
+ {
+ /*cert_subject=*/{},
+ /*cert_dns_names=*/{"*.example.com"},
+ /*cert_emails=*/{},
+ /*valid_dns_names=*/{"now_allowed.example.com"},
+ /*invalid_dns_names=*/{},
+ /*valid_emails=*/{},
+ /*invalid_emails=*/{},
+ /*flags=*/X509_CHECK_FLAG_UNDERSCORE_WILDCARDS,
+ },
+
};

size_t i = 0;
--- a/src/include/openssl/x509c3.h
+++ b/src/include/openssl/x509v3.h
@@ -4497,6 +4497,8 @@ OPENSSL_EXPORT int X509_PURPOSE_get_id(const X509_PURPOSE *);
#define X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS 0
// Skip the subject common name fallback if subjectAltNames is missing.
#define X509_CHECK_FLAG_NEVER_CHECK_SUBJECT 0x20
+// Allow underscores in DNS wildcard matches.
+#define X509_CHECK_FLAG_UNDERSCORE_WILDCARDS 0x40

OPENSSL_EXPORT int X509_check_host(X509 *x, const char *chk, size_t chklen,
unsigned int flags, char **peername);
--
7 changes: 6 additions & 1 deletion boring/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ categories = ["cryptography", "api-bindings"]
edition = { workspace = true }

[package.metadata.docs.rs]
features = ["rpk", "pq-experimental"]
features = ["rpk", "pq-experimental", "underscore-wildcards"]
rustdoc-args = ["--cfg", "docsrs"]

[features]
Expand All @@ -38,6 +38,11 @@ rpk = ["boring-sys/rpk"]
# `BORING_BSSL{,_FIPS}_SOURCE_PATH` and `BORING_BSSL{,_FIPS}_ASSUME_PATCHED`.
pq-experimental = ["boring-sys/pq-experimental"]

# Applies a patch to enable
# `ffi::X509_CHECK_FLAG_UNDERSCORE_WILDCARDS`. Same caveats as
# those for `pq-experimental` feature apply.
underscore-wildcards = ["boring-sys/underscore-wildcards"]

# Controlling key exchange preferences at compile time

# Choose key exchange preferences at compile time. This prevents the user from
Expand Down
3 changes: 0 additions & 3 deletions boring/src/ssl/connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,10 +435,7 @@ fn setup_verify(ctx: &mut SslContextBuilder) {
}

fn setup_verify_hostname(ssl: &mut SslRef, domain: &str) -> Result<(), ErrorStack> {
use crate::x509::verify::X509CheckFlags;

let param = ssl.param_mut();
param.set_hostflags(X509CheckFlags::NO_PARTIAL_WILDCARDS);
match domain.parse() {
Ok(ip) => param.set_ip(ip),
Err(_) => param.set_host(domain),
Expand Down
73 changes: 71 additions & 2 deletions boring/src/ssl/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,12 +503,81 @@ fn verify_valid_hostname() {

client.ctx().set_verify(SslVerifyMode::PEER);

let mut client = client.build().builder();

client.ssl().param_mut().set_host("foobar.com").unwrap();
client.connect();
}

#[test]
fn verify_valid_hostname_with_wildcard() {
let mut server = Server::builder();

server
.ctx()
.set_certificate_chain_file("test/cert-wildcard.pem")
.unwrap();

let server = server.build();
let mut client = server.client_with_root_ca();

client.ctx().set_verify(SslVerifyMode::PEER);

let mut client = client.build().builder();
client.ssl().param_mut().set_host("yes.foobar.com").unwrap();
client.connect();
}

#[test]
fn verify_reject_underscore_hostname_with_wildcard() {
let mut server = Server::builder();

server.should_error();
server
.ctx()
.set_certificate_chain_file("test/cert-wildcard.pem")
.unwrap();

let server = server.build();
let mut client = server.client_with_root_ca();

client.ctx().set_verify(SslVerifyMode::PEER);

let mut client = client.build().builder();
client
.ssl()
.param_mut()
.set_hostflags(X509CheckFlags::NO_PARTIAL_WILDCARDS);
client.ssl().param_mut().set_host("foobar.com").unwrap();
.set_host("not_allowed.foobar.com")
.unwrap();
client.connect_err();
}

#[cfg(feature = "underscore-wildcards")]
#[test]
fn verify_allow_underscore_hostname_with_wildcard() {
let mut server = Server::builder();

server
.ctx()
.set_certificate_chain_file("test/cert-wildcard.pem")
.unwrap();

let server = server.build();
let mut client = server.client_with_root_ca();

client.ctx().set_verify(SslVerifyMode::PEER);

let mut client = client.build().builder();

client
.ssl()
.param_mut()
.set_hostflags(X509CheckFlags::UNDERSCORE_WILDCARDS);
client
.ssl()
.param_mut()
.set_host("now_allowed.foobar.com")
.unwrap();
client.connect();
}

Expand Down
2 changes: 2 additions & 0 deletions boring/src/x509/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ bitflags! {
const MULTI_LABEL_WILDCARDS = ffi::X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS as _;
const SINGLE_LABEL_SUBDOMAINS = ffi::X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS as _;
const NEVER_CHECK_SUBJECT = ffi::X509_CHECK_FLAG_NEVER_CHECK_SUBJECT as _;
#[cfg(feature = "underscore-wildcards")]
const UNDERSCORE_WILDCARDS = ffi::X509_CHECK_FLAG_UNDERSCORE_WILDCARDS as _;

#[deprecated(since = "0.10.6", note = "renamed to NO_WILDCARDS")]
const FLAG_NO_WILDCARDS = ffi::X509_CHECK_FLAG_NO_WILDCARDS as _;
Expand Down
20 changes: 20 additions & 0 deletions boring/test/cert-wildcard.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
notAfter=Aug 12 11:30:03 2026 GMT
-----BEGIN CERTIFICATE-----
MIIDKDCCAhACFGwwuilXOHjBjQ584FD9drp9Uh/LMA0GCSqGSIb3DQEBCwUAMEUx
CzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRl
cm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMjMxMjE4MTEzMDAzWhcNMjYwODEyMTEz
MDAzWjBcMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UE
BwwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMRUwEwYDVQQDDAwqLmZvb2Jhci5j
b20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCo9CWMRLMXo1CF/iOR
h9B4NhtJF/8tR9PlG95sNvyWuQQ/8jfev+8zErplxfLkt0pJqcoiZG8g9NU0kU6o
5T+/1QgZclCAoZaS0Jqxmoo2Yk/1Qsj16pnMBc10uSDk6V9aJSX1vKwONVNSwiHA
1MhX+i7Wf7/K0niq+k7hOkhleFkWgZtUq41gXh1VfOugka7UktYnk9mrBbAMjmal
oZNn2pMMAQxVg4ThiLm3zvuWqvXASWzUZc7IAd1GbN4AtDuhs252eqE9E4iTHk7F
14wAS1JWqv666hReGHrmZJGx0xQTM9vPD1HN5t2U3KTfhO/mTlAUWVyg9tCtOzbo
Kgs1AgMBAAEwDQYJKoZIhvcNAQELBQADggEBAHG83qKMl5bPoL2s7TaJZ909NaQO
4C69ueXlD4HJEFe7L9mkeQoDaF7RwWSBwN2RZT5hzQhghRotqLA06XwKbQHji/R7
sYYVUHunobFUHsr51tFN1BIDoAWJa0N2rm/OxbcK471eWNKjMiS2vvvPdaMxxHAx
IsjAJBJec4IxNIUNNKqCS/xNYcdiyrmmU3oFWGqb0As/eDOBw0Amd0aayasFJrRV
3KZI5OcFg/J3XvdaxMJD+RPyUysKRXg6K8jzYc/PB8LhWVXpLxjEzeO2IHCaZprh
dUTP8+Ob+ioxujvlslxc4nrrUD5EWwnpEIr7e4af27JHQVaNyHbRw6wI2uk=
-----END CERTIFICATE-----

0 comments on commit 2928982

Please sign in to comment.