Skip to content

WIP: remove patched openssl crate, use upstream API for TLS security profiles #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vparfonov wants to merge 1 commit into
base: v0.54.0-rh
Choose a base branch
from
Open

WIP: remove patched openssl crate, use upstream API for TLS security profiles #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ async-nats = { version = "0.42.0", default-features = false, optional = true, fe
nkeys = { version = "0.4.5", default-features = false, optional = true }
nom = { workspace = true, optional = true }
notify = { version = "8.1.0", default-features = false, features = ["macos_fsevent"] }
openssl = { version = "0.10.73", default-features = false}
openssl = { version = "0.10.75", default-features = false}
openssl-probe = { version = "0.1.6", default-features = false }
ordered-float.workspace = true
percent-encoding = { version = "2.3.1", default-features = false }
Expand Down Expand Up @@ -500,7 +500,6 @@ zstd = { version = "0.13.0", default-features = false }
# The `heim` crates depend on `ntapi` 0.3.7 on Windows, but that version has an
# unaligned access bug fixed in the following revision.
ntapi = { git = "https://github.com/MSxDOS/ntapi.git", rev = "24fc1e47677fc9f6e38e5f154e6011dc9b270da6" }
openssl = { path = "patch/openssl" }
hyper = { path = "patch/hyper" }

[features]
Expand Down
53 changes: 52 additions & 1 deletion lib/vector-core/src/tls/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{fmt::Debug, net::SocketAddr, num::TryFromIntError, path::PathBuf, time

use openssl::{
error::ErrorStack,
ssl::{ConnectConfiguration, SslConnector, SslConnectorBuilder, SslMethod},
ssl::{ConnectConfiguration, SslConnector, SslConnectorBuilder, SslMethod, SslVersion},
};
use snafu::{ResultExt, Snafu};
use tokio::net::TcpStream;
Expand Down Expand Up @@ -131,6 +131,16 @@ pub enum TlsError {
NewCaStack { source: ErrorStack },
#[snafu(display("Could not push intermediate certificate onto stack"))]
CaStackPush { source: ErrorStack },
// BEGIN RED HAT - TLS security profile support (LOG-3398)
#[snafu(display("Invalid TLS version: {}", version))]
InvalidTlsVersion { version: String },
#[snafu(display("Invalid or empty ciphersuite string"))]
InvalidCiphersuite,
#[snafu(display("Could not set minimum TLS version: {}", source))]
SetMinTlsVersion { source: ErrorStack },
#[snafu(display("Could not set cipher list: {}", source))]
SetCipherList { source: ErrorStack },
// END RED HAT - TLS security profile support (LOG-3398)
}

impl MaybeTlsStream<TcpStream> {
Expand Down Expand Up @@ -175,6 +185,47 @@ impl MaybeTlsStream<TcpStream> {
}
}

// BEGIN RED HAT - TLS security profile support (LOG-3398)
/// Apply TLS security profile settings (min version, ciphersuites) to an SSL context builder.
///
/// Maps `OpenShift` TLS security profile version strings (`VersionTLS10`..`VersionTLS13`)
/// to OpenSSL protocol versions and configures ciphersuites accordingly.
pub fn apply_tls_security_profile(
ctx: &mut openssl::ssl::SslContextBuilder,
min_tls_version: &Option<String>,
ciphersuites: &Option<String>,
) -> Result<()> {
let mut resolved_version = SslVersion::TLS1;
if let Some(version_str) = min_tls_version {
resolved_version = match version_str.as_str() {
"VersionTLS10" => SslVersion::TLS1,
"VersionTLS11" => SslVersion::TLS1_1,
"VersionTLS12" => SslVersion::TLS1_2,
"VersionTLS13" => SslVersion::TLS1_3,
_ => {
return Err(TlsError::InvalidTlsVersion {
version: version_str.clone(),
});
}
};
ctx.set_min_proto_version(Some(resolved_version))
.context(SetMinTlsVersionSnafu)?;
}
if let Some(suites) = ciphersuites {
if suites.is_empty() {
return Err(TlsError::InvalidCiphersuite);
}
let suites = suites.replace(',', ":");
if resolved_version == SslVersion::TLS1_3 {
ctx.set_ciphersuites(&suites).context(SetCipherListSnafu)?;
} else {
ctx.set_cipher_list(&suites).context(SetCipherListSnafu)?;
}
}
Ok(())
}
// END RED HAT - TLS security profile support (LOG-3398)

pub fn tls_connector_builder(settings: &MaybeTlsSettings) -> Result<SslConnectorBuilder> {
let mut builder = SslConnector::builder(SslMethod::tls()).context(TlsBuildConnectorSnafu)?;
if let Some(settings) = settings.tls() {
Expand Down
176 changes: 88 additions & 88 deletions lib/vector-core/src/tls/settings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,25 @@ use std::{
};

use super::{
AddCertToStoreSnafu, AddExtraChainCertSnafu, CaStackPushSnafu, EncodeAlpnProtocolsSnafu,
FileOpenFailedSnafu, FileReadFailedSnafu, MaybeTls, NewCaStackSnafu, NewStoreBuilderSnafu,
ParsePkcs12Snafu, PrivateKeyParseSnafu, Result, SetAlpnProtocolsSnafu, SetCertificateSnafu,
SetPrivateKeySnafu, SetVerifyCertSnafu, TlsError, X509ParseSnafu,
AddCertToStoreSnafu,
AddExtraChainCertSnafu,
CaStackPushSnafu,
EncodeAlpnProtocolsSnafu,
FileOpenFailedSnafu,
FileReadFailedSnafu,
MaybeTls,
NewCaStackSnafu,
NewStoreBuilderSnafu,
ParsePkcs12Snafu,
PrivateKeyParseSnafu,
Result,
SetAlpnProtocolsSnafu,
SetCertificateSnafu,
SetPrivateKeySnafu,
SetVerifyCertSnafu,
TlsError,
X509ParseSnafu,
apply_tls_security_profile, // RED HAT - TLS security profile support (LOG-3398)
};
use cfg_if::cfg_if;
use lookup::lookup_v2::OptionalValuePath;
Expand Down Expand Up @@ -157,11 +172,13 @@ pub struct TlsConfig {
#[configurable(metadata(docs::human_name = "Server Name"))]
pub server_name: Option<String>,

// BEGIN RED HAT - TLS security profile support (LOG-3398)
/// Minimal enabled TLS version.
pub min_tls_version: Option<String>,

/// TLS ciphersuites to enable.
pub ciphersuites: Option<String>,
// END RED HAT - TLS security profile support (LOG-3398)
}

impl TlsConfig {
Expand All @@ -184,8 +201,10 @@ pub struct TlsSettings {
pub(super) identity: Option<IdentityStore>,
alpn_protocols: Option<Vec<u8>>,
server_name: Option<String>,
// BEGIN RED HAT - TLS security profile support (LOG-3398)
pub min_tls_version: Option<String>,
pub ciphersuites: Option<String>,
// END RED HAT - TLS security profile support (LOG-3398)
}

/// Identity store in PEM format
Expand Down Expand Up @@ -228,6 +247,7 @@ impl TlsSettings {
identity: options.load_identity()?,
alpn_protocols: options.parse_alpn_protocols()?,
server_name: options.server_name.clone(),
// RED HAT - TLS security profile support (LOG-3398)
min_tls_version: options.min_tls_version.clone(),
ciphersuites: options.ciphersuites.clone(),
})
Expand Down Expand Up @@ -347,6 +367,9 @@ impl TlsSettings {
}
}

// RED HAT - TLS security profile support (LOG-3398)
apply_tls_security_profile(context, &self.min_tls_version, &self.ciphersuites)?;

Ok(())
}

Expand Down Expand Up @@ -662,7 +685,7 @@ fn open_read(filename: &Path, note: &'static str) -> Result<(Vec<u8>, PathBuf)>

#[cfg(test)]
mod test {
use openssl::ssl::{ErrorEx, SslMethod, SslVersion};
use openssl::ssl::{SslMethod, SslVersion};

use super::*;

Expand Down Expand Up @@ -831,102 +854,79 @@ mod test {
assert!(config.is_tls());
}

// BEGIN RED HAT - TLS security profile tests (LOG-3398)
#[test]
fn from_min_tls_version() {
use std::result::Result;

struct TlsVersionTest {
text: Option<String>,
num: Option<SslVersion>,
want: Result<(), ErrorEx>,
use super::super::apply_tls_security_profile;

// Valid version strings set the correct min protocol version
let valid_cases = [
("VersionTLS10", SslVersion::TLS1),
("VersionTLS11", SslVersion::TLS1_1),
("VersionTLS12", SslVersion::TLS1_2),
("VersionTLS13", SslVersion::TLS1_3),
];
for (version_str, expected_version) in valid_cases {
let mut builder = SslContextBuilder::new(SslMethod::tls()).unwrap();
apply_tls_security_profile(&mut builder, &Some(version_str.to_string()), &None)
.unwrap_or_else(|_| panic!("should accept {version_str}"));
assert_eq!(builder.min_proto_version(), Some(expected_version));
}

// None leaves the default unchanged
let mut builder = SslContextBuilder::new(SslMethod::tls()).unwrap();
let orig_min_proto_version = builder.min_proto_version();
let tests = [
TlsVersionTest {
text: None,
num: orig_min_proto_version,
want: Ok(()),
},
TlsVersionTest {
text: Some(String::new()),
num: orig_min_proto_version,
want: Err(ErrorEx::InvalidTlsVersion),
},
TlsVersionTest {
text: Some("foobar".to_string()),
num: Some(SslVersion::TLS1),
want: Err(ErrorEx::InvalidTlsVersion),
},
TlsVersionTest {
text: Some("VersionTLS10".to_string()),
num: Some(SslVersion::TLS1),
want: Ok(()),
},
TlsVersionTest {
text: Some("VersionTLS11".to_string()),
num: Some(SslVersion::TLS1_1),
want: Ok(()),
},
TlsVersionTest {
text: Some("VersionTLS12".to_string()),
num: Some(SslVersion::TLS1_2),
want: Ok(()),
},
TlsVersionTest {
text: Some("VersionTLS13".to_string()),
num: Some(SslVersion::TLS1_3),
want: Ok(()),
},
];
for t in tests {
match builder.set_min_tls_version_and_ciphersuites(&t.text, &None) {
Ok(()) => {
assert!(t.want.is_ok());
assert_eq!(builder.min_proto_version(), t.num);
}
Err(e) => assert_eq!(t.want.err().unwrap(), e),
}
let orig = builder.min_proto_version();
apply_tls_security_profile(&mut builder, &None, &None).unwrap();
assert_eq!(builder.min_proto_version(), orig);

// Invalid version strings return InvalidTlsVersion
for bad in ["", "foobar"] {
let mut builder = SslContextBuilder::new(SslMethod::tls()).unwrap();
let err = apply_tls_security_profile(&mut builder, &Some(bad.to_string()), &None)
.expect_err(&format!("should reject {bad:?}"));
assert!(
matches!(err, TlsError::InvalidTlsVersion { .. }),
"expected InvalidTlsVersion for {bad:?}, got {err:?}"
);
}
}

#[test]
fn from_min_tls_version_and_ciphersuites() {
use std::result::Result;
use super::super::apply_tls_security_profile;

struct TlsCiphersuiteTest {
min_tls_version: Option<String>,
ciphersuite: Option<String>,
want: Result<(), ErrorEx>,
}
// Empty ciphersuite string returns InvalidCiphersuite
let mut builder = SslContextBuilder::new(SslMethod::tls()).unwrap();
let err = apply_tls_security_profile(
&mut builder,
&Some("VersionTLS10".to_string()),
&Some(String::new()),
)
.expect_err("should reject empty ciphersuite");
assert!(
matches!(err, TlsError::InvalidCiphersuite),
"expected InvalidCiphersuite, got {err:?}"
);

// TLS 1.2 cipher list is accepted
let mut builder = SslContextBuilder::new(SslMethod::tls()).unwrap();
let tests = [
TlsCiphersuiteTest {
min_tls_version: Some("VersionTLS10".to_string()),
ciphersuite: Some(String::new()),
want: Err(ErrorEx::InvalidCiphersuite),
},
TlsCiphersuiteTest {
min_tls_version: Some("VersionTLS12".to_string()),
ciphersuite: Some("AES128-SHA256".to_string()),
want: Ok(()),
},
TlsCiphersuiteTest {
min_tls_version: Some("VersionTLS13".to_string()),
ciphersuite: Some(
"TLS_CHACHA20_POLY1305_SHA256,TLS_AES_256_GCM_SHA384".to_string(),
),
want: Ok(()),
},
];
for t in tests {
match builder.set_min_tls_version_and_ciphersuites(&t.min_tls_version, &t.ciphersuite) {
Ok(()) => assert!(t.want.is_ok()),
Err(e) => assert_eq!(t.want.err().unwrap(), e),
}
}
apply_tls_security_profile(
&mut builder,
&Some("VersionTLS12".to_string()),
&Some("AES128-SHA256".to_string()),
)
.expect("should accept TLS 1.2 cipher list");

// TLS 1.3 ciphersuites are accepted
let mut builder = SslContextBuilder::new(SslMethod::tls()).unwrap();
apply_tls_security_profile(
&mut builder,
&Some("VersionTLS13".to_string()),
&Some("TLS_CHACHA20_POLY1305_SHA256,TLS_AES_256_GCM_SHA384".to_string()),
)
.expect("should accept TLS 1.3 ciphersuites");
}
// END RED HAT - TLS security profile tests (LOG-3398)

fn settings_from_config(
enabled: Option<bool>,
Expand Down
Loading