Skip to content

Commit

Permalink
Refactor openssl code one more time before rustls integration
Browse files Browse the repository at this point in the history
Co-authored-by: Harald Gutmann <harald@gutmann.one>
  • Loading branch information
eaufavor and hargut committed Oct 12, 2024
1 parent 9921fe4 commit b939776
Show file tree
Hide file tree
Showing 48 changed files with 1,330 additions and 1,341 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ members = [
"pingora-openssl",
"pingora-boringssl",
"pingora-runtime",
"pingora-rustls",
"pingora-ketama",
"pingora-load-balancing",
"pingora-memory-cache",
Expand Down
3 changes: 2 additions & 1 deletion pingora-cache/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ name = "lru_serde"
harness = false

[features]
default = ["openssl"]
default = []
openssl = ["pingora-core/openssl"]
boringssl = ["pingora-core/boringssl"]
rustls = ["pingora-core/rustls"]
14 changes: 9 additions & 5 deletions pingora-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pingora-pool = { version = "0.3.0", path = "../pingora-pool" }
pingora-error = { version = "0.3.0", path = "../pingora-error" }
pingora-timeout = { version = "0.3.0", path = "../pingora-timeout" }
pingora-http = { version = "0.3.0", path = "../pingora-http" }
pingora-rustls = { version = "0.3.0", path = "../pingora-rustls", optional = true }
tokio = { workspace = true, features = ["net", "rt-multi-thread", "signal"] }
futures = "0.3"
async-trait = { workspace = true }
Expand Down Expand Up @@ -66,6 +67,7 @@ openssl-probe = "0.1"
tokio-test = "0.4"
zstd = "0"
httpdate = "1"
x509-parser = { version = "0.16.0", optional = true }

[target.'cfg(unix)'.dependencies]
daemonize = "0.5.0"
Expand All @@ -87,9 +89,11 @@ hyperlocal = "0.8"
jemallocator = "0.5"

[features]
default = ["openssl"]
openssl = ["pingora-openssl", "some_tls"]
boringssl = ["pingora-boringssl", "some_tls"]
default = []
openssl = ["pingora-openssl", "openssl_derived",]
boringssl = ["pingora-boringssl", "openssl_derived",]
rustls = ["pingora-rustls", "any_tls", "dep:x509-parser"]
patched_http1 = ["pingora-http/patched_http1"]
openssl_derived = ["any_tls"]
any_tls = []
sentry = ["dep:sentry"]
patched_http1 = []
some_tls = []
2 changes: 1 addition & 1 deletion pingora-core/src/connectors/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl Connector {
}

#[cfg(test)]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
mod tests {
use super::*;
use crate::protocols::http::v1::client::HttpSession as Http1Session;
Expand Down
2 changes: 1 addition & 1 deletion pingora-core/src/connectors/http/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ mod tests {
}

#[tokio::test]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
async fn test_connect_tls() {
let connector = Connector::new(None);
let peer = HttpPeer::new(("1.1.1.1", 443), true, "one.one.one.one".into());
Expand Down
8 changes: 4 additions & 4 deletions pingora-core/src/connectors/http/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ mod tests {
use crate::upstreams::peer::HttpPeer;

#[tokio::test]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
async fn test_connect_h2() {
let connector = Connector::new(None);
let mut peer = HttpPeer::new(("1.1.1.1", 443), true, "one.one.one.one".into());
Expand All @@ -473,7 +473,7 @@ mod tests {
}

#[tokio::test]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
async fn test_connect_h1() {
let connector = Connector::new(None);
let mut peer = HttpPeer::new(("1.1.1.1", 443), true, "one.one.one.one".into());
Expand All @@ -499,7 +499,7 @@ mod tests {
}

#[tokio::test]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
async fn test_h2_single_stream() {
let connector = Connector::new(None);
let mut peer = HttpPeer::new(("1.1.1.1", 443), true, "one.one.one.one".into());
Expand Down Expand Up @@ -531,7 +531,7 @@ mod tests {
}

#[tokio::test]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
async fn test_h2_multiple_stream() {
let connector = Connector::new(None);
let mut peer = HttpPeer::new(("1.1.1.1", 443), true, "one.one.one.one".into());
Expand Down
19 changes: 12 additions & 7 deletions pingora-core/src/connectors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
pub mod http;
pub mod l4;
mod offload;

#[cfg(feature = "any_tls")]
mod tls;

#[cfg(not(feature = "any_tls"))]
use crate::tls::connectors as tls;

use crate::protocols::Stream;
use crate::server::configuration::ServerConf;
use crate::tls::ssl::SslConnector;
use crate::upstreams::peer::{Peer, ALPN};

pub use l4::Connect as L4Connect;
Expand All @@ -34,6 +38,7 @@ use pingora_pool::{ConnectionMeta, ConnectionPool};
use std::collections::HashMap;
use std::net::SocketAddr;
use std::sync::Arc;
use tls::TlsConnector;
use tokio::sync::Mutex;

/// The options to configure a [TransportConnector]
Expand Down Expand Up @@ -293,7 +298,7 @@ async fn do_connect<P: Peer + Send + Sync>(
peer: &P,
bind_to: Option<BindTo>,
alpn_override: Option<ALPN>,
tls_ctx: &SslConnector,
tls_ctx: &TlsConnector,
) -> Result<Stream> {
// Create the future that does the connections, but don't evaluate it until
// we decide if we need a timeout or not
Expand All @@ -316,7 +321,7 @@ async fn do_connect_inner<P: Peer + Send + Sync>(
peer: &P,
bind_to: Option<BindTo>,
alpn_override: Option<ALPN>,
tls_ctx: &SslConnector,
tls_ctx: &TlsConnector,
) -> Result<Stream> {
let stream = l4_connect(peer, bind_to).await?;
if peer.tls() {
Expand Down Expand Up @@ -383,12 +388,12 @@ fn test_reusable_stream(stream: &mut Stream) -> bool {
}

#[cfg(test)]
#[cfg(feature = "some_tls")]
#[cfg(feature = "any_tls")]
mod tests {
use pingora_error::ErrorType;
use tls::Connector;

use super::*;
use crate::tls::ssl::SslMethod;
use crate::upstreams::peer::BasicPeer;
use tokio::io::AsyncWriteExt;
#[cfg(unix)]
Expand Down Expand Up @@ -498,8 +503,8 @@ mod tests {
/// This assumes that the connection will fail to on the peer and returns
/// the decomposed error type and message
async fn get_do_connect_failure_with_peer(peer: &BasicPeer) -> (ErrorType, String) {
let ssl_connector = SslConnector::builder(SslMethod::tls()).unwrap().build();
let stream = do_connect(peer, None, None, &ssl_connector).await;
let tls_connector = Connector::new(None);
let stream = do_connect(peer, None, None, &tls_connector.ctx).await;
match stream {
Ok(_) => panic!("should throw an error"),
Err(e) => (
Expand Down
68 changes: 3 additions & 65 deletions pingora-core/src/connectors/tls/boringssl_openssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use log::debug;
use pingora_error::{Error, ErrorType::*, OrErr, Result};
use std::sync::{Arc, Once};

use crate::connectors::tls::replace_leftmost_underscore;
use crate::connectors::ConnectorOptions;
use crate::protocols::tls::client::handshake;
use crate::protocols::tls::SslStream;
Expand All @@ -31,6 +32,8 @@ use crate::tls::ssl::{SslConnector, SslFiletype, SslMethod, SslVerifyMode, SslVe
use crate::tls::x509::store::X509StoreBuilder;
use crate::upstreams::peer::{Peer, ALPN};

pub type TlsConnector = SslConnector;

const CIPHER_LIST: &str = "AES-128-GCM-SHA256\
:AES-256-GCM-SHA384\
:CHACHA20-POLY1305-SHA256\
Expand Down Expand Up @@ -147,33 +150,6 @@ impl Connector {
}
}

/*
OpenSSL considers underscores in hostnames non-compliant.
We replace the underscore in the leftmost label as we must support these
hostnames for wildcard matches and we have not patched OpenSSL.
https://github.com/openssl/openssl/issues/12566
> The labels must follow the rules for ARPANET host names. They must
> start with a letter, end with a letter or digit, and have as interior
> characters only letters, digits, and hyphen. There are also some
> restrictions on the length. Labels must be 63 characters or less.
- https://datatracker.ietf.org/doc/html/rfc1034#section-3.5
*/
fn replace_leftmost_underscore(sni: &str) -> Option<String> {
// wildcard is only leftmost label
if let Some((leftmost, rest)) = sni.split_once('.') {
// if not a subdomain or leftmost does not contain underscore return
if !rest.contains('.') || !leftmost.contains('_') {
return None;
}
// we have a subdomain, replace underscores
let leftmost = leftmost.replace('_', "-");
return Some(format!("{leftmost}.{rest}"));
}
None
}

pub(crate) async fn connect<T, P>(
stream: T,
peer: &P,
Expand Down Expand Up @@ -283,41 +259,3 @@ where
None => connect_future.await,
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_replace_leftmost_underscore() {
let none_cases = [
"",
"some",
"some.com",
"1.1.1.1:5050",
"dog.dot.com",
"dog.d_t.com",
"dog.dot.c_m",
"d_g.com",
"_",
"dog.c_m",
];

for case in none_cases {
assert!(replace_leftmost_underscore(case).is_none(), "{}", case);
}

assert_eq!(
Some("bb-b.some.com".to_string()),
replace_leftmost_underscore("bb_b.some.com")
);
assert_eq!(
Some("a-a-a.some.com".to_string()),
replace_leftmost_underscore("a_a_a.some.com")
);
assert_eq!(
Some("-.some.com".to_string()),
replace_leftmost_underscore("_.some.com")
);
}
}
83 changes: 81 additions & 2 deletions pingora-core/src/connectors/tls/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,84 @@
#[cfg(feature = "some_tls")]
// Copyright 2024 Cloudflare, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#[cfg(feature = "openssl_derived")]
mod boringssl_openssl;

#[cfg(feature = "some_tls")]
#[cfg(feature = "openssl_derived")]
pub use boringssl_openssl::*;

/// OpenSSL considers underscores in hostnames non-compliant.
/// We replace the underscore in the leftmost label as we must support these
/// hostnames for wildcard matches and we have not patched OpenSSL.
///
/// https://github.com/openssl/openssl/issues/12566
///
/// > The labels must follow the rules for ARPANET host names. They must
/// > start with a letter, end with a letter or digit, and have as interior
/// > characters only letters, digits, and hyphen. There are also some
/// > restrictions on the length. Labels must be 63 characters or less.
/// - https://datatracker.ietf.org/doc/html/rfc1034#section-3.5
#[cfg(feature = "any_tls")]
pub fn replace_leftmost_underscore(sni: &str) -> Option<String> {
// wildcard is only leftmost label
if let Some((leftmost, rest)) = sni.split_once('.') {
// if not a subdomain or leftmost does not contain underscore return
if !rest.contains('.') || !leftmost.contains('_') {
return None;
}
// we have a subdomain, replace underscores
let leftmost = leftmost.replace('_', "-");
return Some(format!("{leftmost}.{rest}"));
}
None
}

#[cfg(feature = "any_tls")]
#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_replace_leftmost_underscore() {
let none_cases = [
"",
"some",
"some.com",
"1.1.1.1:5050",
"dog.dot.com",
"dog.d_t.com",
"dog.dot.c_m",
"d_g.com",
"_",
"dog.c_m",
];

for case in none_cases {
assert!(replace_leftmost_underscore(case).is_none(), "{}", case);
}

assert_eq!(
Some("bb-b.some.com".to_string()),
replace_leftmost_underscore("bb_b.some.com")
);
assert_eq!(
Some("a-a-a.some.com".to_string()),
replace_leftmost_underscore("a_a_a.some.com")
);
assert_eq!(
Some("-.some.com".to_string()),
replace_leftmost_underscore("_.some.com")
);
}
}
9 changes: 6 additions & 3 deletions pingora-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,14 @@ pub use pingora_error::{ErrorType::*, *};
#[cfg(feature = "boringssl")]
pub use pingora_boringssl as tls;

#[cfg(all(not(feature = "boringssl"), feature = "openssl"))]
#[cfg(feature = "openssl")]
pub use pingora_openssl as tls;

#[cfg(not(feature = "some_tls"))]
pub use protocols::tls::dummy_tls as tls;
#[cfg(feature = "rustls")]
pub use pingora_rustls as tls;

#[cfg(not(feature = "any_tls"))]
pub use protocols::tls::noop_tls as tls;

pub mod prelude {
pub use crate::server::configuration::Opt;
Expand Down
Loading

0 comments on commit b939776

Please sign in to comment.