Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
222 changes: 218 additions & 4 deletions common/infrastructure/src/app/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ use actix_web::{
use anyhow::{Context, anyhow};
use bytesize::ByteSize;
use clap::{Arg, ArgMatches, Args, Command, Error, FromArgMatches, value_parser};
use openssl::ssl::SslFiletype;
use openssl::ssl::{SslFiletype, SslVersion};
use opentelemetry_instrumentation_actix_web::{RequestMetrics, RequestTracing, RouteFormatter};
use std::{
fmt::Debug,
fmt::{self, Debug},
marker::PhantomData,
net::{IpAddr, Ipv6Addr, SocketAddr, TcpListener},
ops::Deref,
Expand Down Expand Up @@ -110,6 +110,70 @@ impl<E: Endpoint> FromArgMatches for BindPort<E> {
}
}

/// TLS security profile, aligned with OpenShift TLS Security Profiles.
#[derive(clap::ValueEnum, Clone, Copy, Debug, PartialEq, Default)]
pub enum TlsSecurityProfile {
/// TLS 1.0+, broadest compatibility.
#[clap(name = "old")]
Old,
/// TLS 1.2+, broad compatibility.
#[clap(name = "intermediate")]
Intermediate,
/// TLS 1.3 only, most secure.
#[default]
#[clap(name = "modern")]
Modern,
/// User-defined min TLS version and cipher suites.
#[clap(name = "custom")]
Custom,
}

impl fmt::Display for TlsSecurityProfile {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
TlsSecurityProfile::Old => write!(f, "old"),
TlsSecurityProfile::Intermediate => write!(f, "intermediate"),
TlsSecurityProfile::Modern => write!(f, "modern"),
TlsSecurityProfile::Custom => write!(f, "custom"),
}
}
}

/// Minimum TLS protocol version.
#[derive(clap::ValueEnum, Clone, Copy, Debug, PartialEq)]
pub enum TlsMinVersion {
#[clap(name = "1.0")]
Tls1_0,
#[clap(name = "1.1")]
Tls1_1,
#[clap(name = "1.2")]
Tls1_2,
#[clap(name = "1.3")]
Tls1_3,
}

impl fmt::Display for TlsMinVersion {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
TlsMinVersion::Tls1_0 => write!(f, "1.0"),
TlsMinVersion::Tls1_1 => write!(f, "1.1"),
TlsMinVersion::Tls1_2 => write!(f, "1.2"),
TlsMinVersion::Tls1_3 => write!(f, "1.3"),
}
}
}

impl TlsMinVersion {
fn to_ssl_version(self) -> SslVersion {
match self {
TlsMinVersion::Tls1_0 => SslVersion::TLS1,
TlsMinVersion::Tls1_1 => SslVersion::TLS1_1,
TlsMinVersion::Tls1_2 => SslVersion::TLS1_2,
TlsMinVersion::Tls1_3 => SslVersion::TLS1_3,
}
}
}

#[derive(Clone, Debug, clap::Args)]
#[command(
rename_all_env = "SCREAMING_SNAKE_CASE",
Expand Down Expand Up @@ -186,6 +250,35 @@ where
)]
pub tls_certificate_file: Option<PathBuf>,

/// TLS security profile (old, intermediate, modern, custom)
#[arg(
id = "http-server-tls-security-profile",
long,
env = "HTTP_SERVER_TLS_SECURITY_PROFILE",
default_value_t = TlsSecurityProfile::Modern,
)]
pub tls_security_profile: TlsSecurityProfile,

/// Minimum TLS version (only used with custom profile)
#[arg(
id = "http-server-tls-min-version",
long,
env = "HTTP_SERVER_TLS_MIN_VERSION"
)]
pub tls_min_version: Option<TlsMinVersion>,

/// TLS 1.2 cipher list in OpenSSL format (only used with custom profile)
#[arg(id = "http-server-tls-ciphers", long, env = "HTTP_SERVER_TLS_CIPHERS")]
pub tls_ciphers: Option<String>,

/// TLS 1.3 ciphersuites in OpenSSL format (only used with custom profile)
#[arg(
id = "http-server-tls-ciphersuites",
long,
env = "HTTP_SERVER_TLS_CIPHERSUITES"
)]
Comment on lines +270 to +279

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Enforce or surface misuse when ciphers/ciphersuites are set without a Custom profile

Currently, when security_profile is not Custom, any values provided for tls_ciphers / tls_ciphersuites are silently ignored, despite the CLI docs stating they are "only used with custom profile". This can lead to unnoticed misconfigurations. Consider either rejecting configurations that set these fields without Custom, or emitting a clear warning that they are ignored unless tls_security_profile=custom is selected.

Suggested implementation:

    /// Minimum TLS version (ignored unless `tls_security_profile=custom`)
    /// TLS 1.2 cipher list in OpenSSL format (ignored unless `tls_security_profile=custom`)
    /// TLS 1.3 ciphersuites in OpenSSL format (ignored unless `tls_security_profile=custom`)

`).

Here are the code changes to adjust the CLI docs:

<file_operations>
<file_operation operation="edit" file_path="common/infrastructure/src/app/http.rs">
<<<<<<< SEARCH
/// Minimum TLS version (only used with custom profile)

/// Minimum TLS version (ignored unless `tls_security_profile=custom`)

REPLACE

<<<<<<< SEARCH
/// TLS 1.2 cipher list in OpenSSL format (only used with custom profile)

/// TLS 1.2 cipher list in OpenSSL format (ignored unless `tls_security_profile=custom`)

REPLACE

<<<<<<< SEARCH
/// TLS 1.3 ciphersuites in OpenSSL format (only used with custom profile)

/// TLS 1.3 ciphersuites in OpenSSL format (ignored unless `tls_security_profile=custom`)

REPLACE
</file_operation>
</file_operations>

<additional_changes>
To fully implement your suggestion of enforcing or warning on misuse at runtime, you will also need to:

  1. Identify the struct that owns these fields (e.g., the CLI args/config struct in this file) and add a validation method such as:
    impl HttpServerConfig {
        pub fn validate_tls_profile(&self) {
            if self.tls_security_profile != TlsSecurityProfile::Custom {
                if self.tls_min_version.is_some()
                    || self.tls_ciphers.is_some()
                    || self.tls_ciphersuites.is_some()
                {
                    tracing::warn!(
                        "TLS ciphers/ciphersuites/tls_min_version are configured but ignored \
                         because tls_security_profile is not `custom`"
                    );
                }
            }
        }
    }
    (Adjust the struct name and logging crate to match your codebase.)
  2. Call this validation method right after parsing CLI arguments / building this config, so that the warning is emitted on startup whenever these options are set but tls_security_profile != Custom. If you prefer hard failure instead of a warning, have this method return a Result and bail!/Err in that branch according to your existing error-handling conventions.

pub tls_ciphersuites: Option<String>,

/// Disable the request log
#[arg(id = "http-disable-log", long, env = "HTTP_SERVER_DISABLE_LOG")]
pub disable_log: bool,
Expand Down Expand Up @@ -224,6 +317,10 @@ where
tls_enabled: false,
tls_key_file: None,
tls_certificate_file: None,
tls_security_profile: TlsSecurityProfile::Modern,
tls_min_version: None,
tls_ciphers: None,
tls_ciphersuites: None,
disable_log: false,
_marker: Default::default(),
}
Expand Down Expand Up @@ -264,13 +361,24 @@ where
.json_limit(value.json_limit.0.0 as _);

if value.tls_enabled {
if value.tls_security_profile == TlsSecurityProfile::Custom

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use clap's support for this?

&& value.tls_min_version.is_none()
{
return Err(anyhow!(
"Custom TLS profile requires --http-server-tls-min-version"
Comment on lines +364 to +368

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 suggestion (security): Consider validating Custom profile invariants closer to build_ssl_acceptor as well

Right now try_from enforces that Custom profiles provide tls_min_version, but build_ssl_acceptor still accepts a Custom profile with min_version = None and silently falls back to the mozilla_intermediate_v5 default. If a TlsConfiguration is constructed and passed directly, this invariant can be violated. Please add a guard in build_ssl_acceptor (e.g., return an error when security_profile == Custom && min_version.is_none()) so this misconfiguration is caught at the point of use as well.

Suggested implementation:

pub fn build_ssl_acceptor(
    config: &TlsConfiguration,
) -> anyhow::Result<SslAcceptorBuilder> {
    if config.tls_security_profile == TlsSecurityProfile::Custom
        && config.tls_min_version.is_none()
    {
        anyhow::bail!(
            "Custom TLS profile requires --http-server-tls-min-version (tls_min_version must be set)"
        );
    }

Because I only see part of the file, you may need to adjust:

  1. Function signature / names: If build_ssl_acceptor has a slightly different name or signature (e.g., takes &HttpConfig instead of &TlsConfiguration), update the config type/name in the guard to match.
  2. Error type / crate: If anyhow::Result or anyhow::bail! is not used in this file, adapt the error handling to whatever is standard here (e.g., Result<_, Error> with return Err(Error::Config(...))).
  3. Field names: Ensure the field names align with your actual config struct (tls_security_profile vs security_profile, tls_min_version vs min_version). The guard should check the Custom variant and the min-version option field actually used in your struct.
  4. Import: If TlsSecurityProfile is not in scope in this module, add an appropriate use statement at the top of the file (e.g., use crate::app::http::TlsSecurityProfile; or wherever it is defined).

));
}
result = result.tls(TlsConfiguration {
key: value.tls_key_file.ok_or_else(|| {
anyhow!("TLS enabled but no key file configured (use --http-server-tls-key-file)")
})?,
certificate: value.tls_certificate_file.ok_or_else(|| {
anyhow!("TLS enabled but no certificate file configured (use --http-server-tls-certificate-file)")
})?,
security_profile: value.tls_security_profile,
min_version: value.tls_min_version,
ciphers: value.tls_ciphers,
ciphersuites: value.tls_ciphersuites,
});
}

Expand Down Expand Up @@ -309,6 +417,51 @@ pub struct HttpServerBuilder {
pub struct TlsConfiguration {
certificate: PathBuf,
key: PathBuf,
security_profile: TlsSecurityProfile,
min_version: Option<TlsMinVersion>,
ciphers: Option<String>,
ciphersuites: Option<String>,
}

/// Build an SSL acceptor configured according to the given TLS profile.
fn build_ssl_acceptor(tls: &TlsConfiguration) -> anyhow::Result<openssl::ssl::SslAcceptorBuilder> {
let acceptor = match tls.security_profile {
TlsSecurityProfile::Old => {
let mut builder = SslAcceptor::mozilla_intermediate_v5(SslMethod::tls_server())
.context("creating Old TLS profile acceptor")?;
builder
Comment on lines +429 to +432

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Clarify or tighten min TLS version for the Old profile to avoid enabling too-old protocols

Here you’re clearing the minimum protocol version after starting from mozilla_intermediate_v5. With some OpenSSL builds, set_min_proto_version(None) can re-enable SSLv3 or other pre–TLS 1.0 protocols, making the runtime behavior weaker than the documented TLS 1.0+. Consider explicitly setting TLS1 (or the intended minimum) instead of None, and/or adjust the comment so it accurately describes the minimum protocol version actually enforced.

.set_min_proto_version(None)
.context("clearing min TLS version for Old profile")?;
builder
}
TlsSecurityProfile::Intermediate => {
SslAcceptor::mozilla_intermediate_v5(SslMethod::tls_server())
.context("creating Intermediate TLS profile acceptor")?
}
TlsSecurityProfile::Modern => SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())
.context("creating Modern TLS profile acceptor")?,
TlsSecurityProfile::Custom => {
let mut builder = SslAcceptor::mozilla_intermediate_v5(SslMethod::tls_server())
.context("creating Custom TLS profile acceptor")?;
if let Some(min_version) = tls.min_version {
builder
.set_min_proto_version(Some(min_version.to_ssl_version()))
.context("setting custom min TLS version")?;
}
if let Some(ref ciphers) = tls.ciphers {
builder
.set_cipher_list(ciphers)
.context("setting custom TLS 1.2 cipher list")?;
}
if let Some(ref ciphersuites) = tls.ciphersuites {
builder
.set_ciphersuites(ciphersuites)
.context("setting custom TLS 1.3 ciphersuites")?;
}
builder
}
};
Ok(acceptor)
}

pub enum Bind {
Expand Down Expand Up @@ -535,8 +688,8 @@ impl HttpServerBuilder {

let tls = match self.tls {
Some(tls) => {
log::info!("Enabling TLS support");
let mut acceptor = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?;
log::info!("Enabling TLS support (profile: {})", tls.security_profile);
let mut acceptor = build_ssl_acceptor(&tls)?;
acceptor
.set_certificate_chain_file(tls.certificate)
.context("setting certificate chain")?;
Expand Down Expand Up @@ -641,4 +794,65 @@ mod test {
fn default_config_converts() {
HttpServerBuilder::try_from(HttpServerConfig::<MockEndpoint>::default()).unwrap();
}

/// Verifies that the default config uses the Modern TLS profile.
#[test]
fn default_tls_profile_is_modern() {
let config = HttpServerConfig::<MockEndpoint>::default();
assert_eq!(config.tls_security_profile, TlsSecurityProfile::Modern);
}

/// Verifies that a Custom profile without min_version is rejected.
#[test]
fn custom_profile_requires_min_version() {
let config = HttpServerConfig::<MockEndpoint> {
tls_enabled: true,
tls_key_file: Some(PathBuf::from("/tmp/key.pem")),
tls_certificate_file: Some(PathBuf::from("/tmp/cert.pem")),
tls_security_profile: TlsSecurityProfile::Custom,
tls_min_version: None,
..Default::default()
};
let err = match HttpServerBuilder::try_from(config) {
Err(e) => e,
Ok(_) => panic!("expected error for Custom profile without min_version"),
};
assert!(
err.to_string().contains("--http-server-tls-min-version"),
"error should mention --http-server-tls-min-version, got: {err}"
);
}

/// Verifies that all named profiles are accepted during config conversion.
#[test]
fn all_profiles_accepted() {
for profile in [
TlsSecurityProfile::Old,
TlsSecurityProfile::Intermediate,
TlsSecurityProfile::Modern,
] {
let config = HttpServerConfig::<MockEndpoint> {
tls_enabled: true,
tls_key_file: Some(PathBuf::from("/tmp/key.pem")),
tls_certificate_file: Some(PathBuf::from("/tmp/cert.pem")),
tls_security_profile: profile,
..Default::default()
};
HttpServerBuilder::try_from(config).unwrap();
}
}

/// Verifies that Custom profile with min_version is accepted.
#[test]
fn custom_profile_with_min_version_accepted() {
let config = HttpServerConfig::<MockEndpoint> {
tls_enabled: true,
tls_key_file: Some(PathBuf::from("/tmp/key.pem")),
tls_certificate_file: Some(PathBuf::from("/tmp/cert.pem")),
tls_security_profile: TlsSecurityProfile::Custom,
tls_min_version: Some(TlsMinVersion::Tls1_2),
..Default::default()
};
HttpServerBuilder::try_from(config).unwrap();
}
}
4 changes: 4 additions & 0 deletions docs/env-vars.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
| `HTTP_SERVER_JSON_LIMIT` | JSON request limit | `2 MiB` |
| `HTTP_SERVER_REQUEST_LIMIT` | Overall request limit | `256 KiB` |
| `HTTP_SERVER_TLS_CERTIFICATE_FILE` | Path to the TLS certificate in PEM format | |
| `HTTP_SERVER_TLS_CIPHERS` | TLS 1.2 cipher list in OpenSSL format (custom profile only) | |
| `HTTP_SERVER_TLS_CIPHERSUITES` | TLS 1.3 ciphersuites in OpenSSL format (custom profile only) | |
| `HTTP_SERVER_TLS_ENABLED` | Enable TLS | `false` |
| `HTTP_SERVER_TLS_KEY_FILE` | Path to the TLS key file in PEM format | |
| `HTTP_SERVER_TLS_MIN_VERSION` | Minimum TLS version for custom profile: 1.0, 1.1, 1.2, 1.3 | |
| `HTTP_SERVER_TLS_SECURITY_PROFILE` | TLS security profile: old, intermediate, modern, custom | `modern` |
| `HTTP_SERVER_WORKERS` | Number of worker threads, defaults to zero, which falls back to the number of cores | `0` |
| `IMPORTER_CONCURRENCY` | The maximum number of jobs run simultaneously by the importer | `1` |
| `IMPORTER_WORKING_DIR` | Where the importer downloads documents prior to ingesting them | `tempdir` |
Expand Down
Loading