signalapp/boring
signalapp/boring
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rework SslMethod

Browse Source 
Instead of keeping around a flag on contexts to know
whether we are configured for RPK, we start from SslMethod
with a flag to know whether it is configured for X.509
certificates. We then propagate this flag to context builders
and contexts, defaulting to false, and introduce
`assume_x509` methods to inform the crate of X.509 support
for contexts created with other means than our own functions.

This improves the safety of the crate as any `SslContextBuilder`
configured with `SslMethod::tls_with_buffer` would crash if used
with functions involving X.509 certificates. This `SslMethod` is
made unsafe because we can't guarantee that we check for X.509
support from all FFI bindingsi (for example, BoringSSL crashes
if there is a mismatch in X.509 support in `SSL_set_SSL_CTX`).

Note that there is no point anyway in forbidding X.509 functions
on a context that supports RPK, as current BoringSSL is able
to negociate both raw public keys and X.509 certificates on the
same context.

Finally, I removed `SslMethod::tls_client` and other peer-specific
methods as they are just the same as there non-peer-specific
equivalent methods.
This commit is contained in:
Anthony Ramine 2025-12-20 12:27:10 +01:00 committed by Kornel
parent 93d9018774
commit c2f063cf47
5 changed files with 122 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
boring/src/ssl/connector.rs
View File

@ -23,19 +23,9 @@ ssbzSibBsu/6iGtCOGEoXJf//////////wIBAg==
-----END DH PARAMETERS-----
";
enum ContextType {
WithMethod(SslMethod),
#[cfg(feature = "rpk")]
Rpk,
}
#[allow(clippy::inconsistent_digit_grouping)]
fn ctx(ty: ContextType) -> Result<SslContextBuilder, ErrorStack> {
let mut ctx = match ty {
ContextType::WithMethod(method) => SslContextBuilder::new(method),
#[cfg(feature = "rpk")]
ContextType::Rpk => SslContextBuilder::new_rpk(),
}?;
fn ctx(method: SslMethod) -> Result<SslContextBuilder, ErrorStack> {
let mut ctx = SslContextBuilder::new(method)?;
let mut opts = SslOptions::ALL
| SslOptions::NO_COMPRESSION
@ -77,7 +67,7 @@ impl SslConnector {
///
/// The default configuration is subject to change, and is currently derived from Python.
pub fn builder(method: SslMethod) -> Result<SslConnectorBuilder, ErrorStack> {
let mut ctx = ctx(ContextType::WithMethod(method))?;
let mut ctx = ctx(method)?;
ctx.set_default_verify_paths()?;
ctx.set_cipher_list(
"DEFAULT:!aNULL:!eNULL:!MD5:!3DES:!DES:!RC4:!IDEA:!SEED:!aDSS:!SRP:!PSK",
@ -87,17 +77,6 @@ impl SslConnector {
Ok(SslConnectorBuilder(ctx))
}
/// Creates a new builder for TLS connections with raw public key.
#[cfg(feature = "rpk")]
pub fn rpk_builder() -> Result<SslConnectorBuilder, ErrorStack> {
let mut ctx = ctx(ContextType::Rpk)?;
ctx.set_cipher_list(
"DEFAULT:!aNULL:!eNULL:!MD5:!3DES:!DES:!RC4:!IDEA:!SEED:!aDSS:!SRP:!PSK",
)?;
Ok(SslConnectorBuilder(ctx))
}
/// Initiates a client-side TLS session on a stream.
///
/// The domain is used for SNI and hostname verification.
@ -224,13 +203,7 @@ impl ConnectConfiguration {
self.ssl.set_hostname(domain)?;
}
#[cfg(feature = "rpk")]
let verify_hostname = self.ssl.ssl_context().has_x509_support() && self.verify_hostname;
#[cfg(not(feature = "rpk"))]
let verify_hostname = self.verify_hostname;
if verify_hostname {
if self.verify_hostname {
setup_verify_hostname(&mut self.ssl, domain)?;
}
@ -292,21 +265,6 @@ impl DerefMut for ConnectConfiguration {
pub struct SslAcceptor(SslContext);
impl SslAcceptor {
/// Creates a new builder configured to connect to clients that support Raw Public Keys.
#[cfg(feature = "rpk")]
pub fn rpk() -> Result<SslAcceptorBuilder, ErrorStack> {
let mut ctx = ctx(ContextType::Rpk)?;
ctx.set_options(SslOptions::NO_TLSV1 | SslOptions::NO_TLSV1_1);
let dh = Dh::params_from_pem(FFDHE_2048.as_bytes())?;
ctx.set_tmp_dh(&dh)?;
ctx.set_cipher_list(
"ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:\
ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:\
DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384"
)?;
Ok(SslAcceptorBuilder(ctx))
}
/// Creates a new builder configured to connect to non-legacy clients. This should generally be
/// considered a reasonable default choice.
///
@ -315,7 +273,7 @@ impl SslAcceptor {
///
/// [docs]: https://wiki.mozilla.org/Security/Server_Side_TLS
pub fn mozilla_intermediate_v5(method: SslMethod) -> Result<SslAcceptorBuilder, ErrorStack> {
let mut ctx = ctx(ContextType::WithMethod(method))?;
let mut ctx = ctx(method)?;
ctx.set_options(SslOptions::NO_TLSV1 | SslOptions::NO_TLSV1_1);
let dh = Dh::params_from_pem(FFDHE_2048.as_bytes())?;
ctx.set_tmp_dh(&dh)?;
@ -336,7 +294,7 @@ impl SslAcceptor {
/// [docs]: https://wiki.mozilla.org/Security/Server_Side_TLS
// FIXME remove in next major version
pub fn mozilla_intermediate(method: SslMethod) -> Result<SslAcceptorBuilder, ErrorStack> {
let mut ctx = ctx(ContextType::WithMethod(method))?;
let mut ctx = ctx(method)?;
ctx.set_options(SslOptions::CIPHER_SERVER_PREFERENCE);
ctx.set_options(SslOptions::NO_TLSV1_3);
let dh = Dh::params_from_pem(FFDHE_2048.as_bytes())?;
@ -362,7 +320,7 @@ impl SslAcceptor {
/// [docs]: https://wiki.mozilla.org/Security/Server_Side_TLS
// FIXME remove in next major version
pub fn mozilla_modern(method: SslMethod) -> Result<SslAcceptorBuilder, ErrorStack> {
let mut ctx = ctx(ContextType::WithMethod(method))?;
let mut ctx = ctx(method)?;
ctx.set_options(
SslOptions::CIPHER_SERVER_PREFERENCE | SslOptions::NO_TLSV1 | SslOptions::NO_TLSV1_1,
);

1
boring/src/ssl/error.rs
View File

@ -249,7 +249,6 @@ fn fmt_mid_handshake_error(
f: &mut fmt::Formatter,
prefix: &str,
) -> fmt::Result {
#[cfg(feature = "rpk")]
if !s.ssl().ssl_context().has_x509_support() {
write!(f, "{}", prefix)?;
return write!(f, " {}", s.error());

199
boring/src/ssl/mod.rs
View File

@ -245,59 +245,87 @@ bitflags! {
/// A type specifying the kind of protocol an `SslContext` will speak.
#[derive(Copy, Clone)]
pub struct SslMethod(*const ffi::SSL_METHOD);
pub struct SslMethod {
ptr: *const ffi::SSL_METHOD,
is_x509_method: bool,
}
impl SslMethod {
/// Support all versions of the TLS protocol.
#[corresponds(TLS_method)]
#[must_use]
pub fn tls() -> SslMethod {
unsafe { SslMethod(TLS_method()) }
pub fn tls() -> Self {
unsafe {
Self {
ptr: ffi::TLS_method(),
is_x509_method: true,
}
}
}
/// Same as `tls`, but doesn't create X509 for certificates.
#[cfg(feature = "rpk")]
pub fn tls_with_buffer() -> SslMethod {
unsafe { SslMethod(ffi::TLS_with_buffers_method()) }
/// Same as `tls`, but doesn't create X.509 for certificates.
///
/// # Safety
///
/// BoringSSL will crash if the user calls a function that involves
/// X.509 certificates with an object configured with this method.
/// You most probably don't need it.
#[must_use]
pub unsafe fn tls_with_buffer() -> Self {
unsafe {
Self {
ptr: ffi::TLS_with_buffers_method(),
is_x509_method: false,
}
}
}
/// Support all versions of the DTLS protocol.
#[corresponds(DTLS_method)]
#[must_use]
pub fn dtls() -> SslMethod {
unsafe { SslMethod(DTLS_method()) }
}
/// Support all versions of the TLS protocol, explicitly as a client.
#[corresponds(TLS_client_method)]
#[must_use]
pub fn tls_client() -> SslMethod {
unsafe { SslMethod(TLS_client_method()) }
}
/// Support all versions of the TLS protocol, explicitly as a server.
#[corresponds(TLS_server_method)]
#[must_use]
pub fn tls_server() -> SslMethod {
unsafe { SslMethod(TLS_server_method()) }
pub fn dtls() -> Self {
unsafe {
Self {
ptr: ffi::DTLS_method(),
is_x509_method: true,
}
}
}
/// Constructs an `SslMethod` from a pointer to the underlying OpenSSL value.
///
/// This method assumes that the `SslMethod` is not configured for X.509
/// certificates. The user can call `SslMethod::assume_x509_method`
/// to change that.
///
/// # Safety
///
/// The caller must ensure the pointer is valid.
#[corresponds(TLS_server_method)]
#[must_use]
pub unsafe fn from_ptr(ptr: *const ffi::SSL_METHOD) -> SslMethod {
SslMethod(ptr)
SslMethod {
ptr,
is_x509_method: false,
}
}
/// Assumes that this `SslMethod` is configured for X.509 certificates.
///
/// # Safety
///
/// BoringSSL will crash if the user calls a function that involves
/// X.509 certificates with an object configured with this method.
/// You most probably don't need it.
pub unsafe fn assume_x509(&mut self) {
self.is_x509_method = true;
}
/// Returns a pointer to the underlying OpenSSL value.
#[allow(clippy::trivially_copy_pass_by_ref)]
#[must_use]
pub fn as_ptr(&self) -> *const ffi::SSL_METHOD {
self.0
self.ptr
}
}
@ -451,8 +479,7 @@ static SESSION_CTX_INDEX: LazyLock<Index<Ssl, SslContext>> =
LazyLock::new(|| Ssl::new_ex_index().unwrap());
static SSL_CREDENTIAL_INDEXES: LazyLock<Mutex<HashMap<TypeId, c_int>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
#[cfg(feature = "rpk")]
static RPK_FLAG_INDEX: LazyLock<Index<SslContext, bool>> =
static X509_FLAG_INDEX: LazyLock<Index<SslContext, bool>> =
LazyLock::new(|| SslContext::new_ex_index().unwrap());
/// An error returned from the SNI callback.
@ -884,40 +911,11 @@ impl Ssl3AlertLevel {
pub const FATAL: Ssl3AlertLevel = Self(ffi::SSL3_AL_FATAL);
}
#[cfg(feature = "rpk")]
extern "C" fn rpk_verify_failure_callback(
_ssl: *mut ffi::SSL,
_out_alert: *mut u8,
) -> ffi::ssl_verify_result_t {
// Always verify the peer.
ffi::ssl_verify_result_t::ssl_verify_invalid
}
/// A builder for `SslContext`s.
pub struct SslContextBuilder {
ctx: SslContext,
/// If it's not shared, it can be exposed as mutable
has_shared_cert_store: bool,
#[cfg(feature = "rpk")]
is_rpk: bool,
}
#[cfg(feature = "rpk")]
impl SslContextBuilder {
/// Creates a new `SslContextBuilder` to be used with Raw Public Key.
#[corresponds(SSL_CTX_new)]
pub fn new_rpk() -> Result<SslContextBuilder, ErrorStack> {
unsafe {
init();
let ctx = cvt_p(ffi::SSL_CTX_new(SslMethod::tls_with_buffer().as_ptr()))?;
let mut builder = SslContextBuilder::from_ptr(ctx);
builder.is_rpk = true;
builder.set_ex_data(*RPK_FLAG_INDEX, true);
Ok(builder)
}
}
}
impl SslContextBuilder {
@ -927,31 +925,45 @@ impl SslContextBuilder {
unsafe {
init();
let ctx = cvt_p(ffi::SSL_CTX_new(method.as_ptr()))?;
Ok(SslContextBuilder::from_ptr(ctx))
let mut builder = SslContextBuilder::from_ptr(ctx);
if method.is_x509_method {
builder.ctx.assume_x509();
}
Ok(builder)
}
}
/// Creates an `SslContextBuilder` from a pointer to a raw OpenSSL value.
///
#[cfg_attr(
feature = "rpk",
doc = "Keeps previous RPK state. Use `new_rpk()` to enable RPK."
)]
/// This method can find out whether `ctx` is configured for X.509 certificates
/// if `ctx` was itself a context created by this crate. If it was created by
/// other means and it supports X.509 certificates, the use can call
/// `SslContextBuilder::assume_x509`.
///
/// # Safety
///
/// The caller must ensure that the pointer is valid and uniquely owned by the builder.
/// The context must own its cert store exclusively.
pub unsafe fn from_ptr(ctx: *mut ffi::SSL_CTX) -> SslContextBuilder {
let ctx = SslContext::from_ptr(ctx);
SslContextBuilder {
#[cfg(feature = "rpk")]
is_rpk: !ctx.has_x509_support(),
pub unsafe fn from_ptr(ctx: *mut ffi::SSL_CTX) -> Self {
Self {
ctx: SslContext::from_ptr(ctx),
has_shared_cert_store: false,
ctx,
}
}
/// Assumes that this `SslContextBuilder` is configured for X.509 certificates.
///
/// # Safety
///
/// BoringSSL will crash if the user calls a function that involves
/// X.509 certificates with an object configured with this method.
/// You most probably don't need it.
pub unsafe fn assume_x509(&mut self) {
self.ctx.assume_x509();
}
/// Returns a pointer to the raw OpenSSL value.
#[must_use]
pub fn as_ptr(&self) -> *mut ffi::SSL_CTX {
@ -2303,12 +2315,20 @@ impl SslContextRef {
SslVerifyMode::from_bits(mode).expect("SSL_CTX_get_verify_mode returned invalid mode")
}
/// Returns `true` if context was NOT created for Raw Public Key verification
/// Assumes that this `SslContext` is configured for X.509 certificates.
///
/// # Safety
///
/// BoringSSL will crash if the user calls a function that involves
/// X.509 certificates with an object configured with this method.
/// You most probably don't need it.
pub unsafe fn assume_x509(&mut self) {
self.replace_ex_data(*X509_FLAG_INDEX, true);
}
/// Returns `true` if context is configured for X.509 certificates.
pub fn has_x509_support(&self) -> bool {
#[cfg(feature = "rpk")]
return !self.ex_data(*RPK_FLAG_INDEX).copied().unwrap_or_default();
#[cfg(not(feature = "rpk"))]
return true;
self.ex_data(*X509_FLAG_INDEX).copied().unwrap_or_default()
}
#[track_caller]
@ -2810,20 +2830,20 @@ impl Ssl {
where
S: Read + Write,
{
#[cfg(feature = "rpk")]
{
let ctx = self.ssl_context();
// #[cfg(feature = "rpk")]
// {
// let ctx = self.ssl_context();
if !ctx.has_x509_support() {
unsafe {
ffi::SSL_CTX_set_custom_verify(
ctx.as_ptr(),
SslVerifyMode::PEER.bits(),
Some(rpk_verify_failure_callback),
);
}
}
}
// if !ctx.has_x509_support() {
// unsafe {
// ffi::SSL_CTX_set_custom_verify(
// ctx.as_ptr(),
// SslVerifyMode::PEER.bits(),
// Some(rpk_verify_failure_callback),
// );
// }
// }
// }
SslStreamBuilder::new(self, stream).setup_accept()
}
@ -2853,7 +2873,6 @@ impl fmt::Debug for SslRef {
builder.field("state", &self.state_string_long());
#[cfg(feature = "rpk")]
if self.ssl_context().has_x509_support() {
builder.field("verify_result", &self.verify_result());
}
@ -3427,6 +3446,12 @@ impl SslRef {
/// It is most commonly used in the Server Name Indication (SNI) callback.
#[corresponds(SSL_set_SSL_CTX)]
pub fn set_ssl_context(&mut self, ctx: &SslContextRef) -> Result<(), ErrorStack> {
assert_eq!(
self.ssl_context().has_x509_support(),
ctx.has_x509_support(),
"X.509 certificate support in old and new contexts doesn't match",
);
unsafe { cvt_p(ffi::SSL_set_SSL_CTX(self.as_ptr(), ctx.as_ptr())).map(|_| ()) }
}
@ -4827,8 +4852,6 @@ pub trait CertificateCompressor: Send + Sync + 'static {
use crate::ffi::{SSL_CTX_up_ref, SSL_SESSION_get_master_key, SSL_SESSION_up_ref, SSL_is_server};
use crate::ffi::{DTLS_method, TLS_client_method, TLS_method, TLS_server_method};
use std::sync::Once;
unsafe fn get_new_idx(f: ffi::CRYPTO_EX_free) -> c_int {

2
tokio-boring/examples/simple-async.rs
View File

@ -6,7 +6,7 @@ async fn main() -> anyhow::Result<()> {
let listener = TcpListener::bind("127.0.0.1:8080").await?;
let (tcp_stream, _addr) = listener.accept().await?;
let server = ssl::SslMethod::tls_server();
let server = ssl::SslMethod::tls();
let mut ssl_builder = boring::ssl::SslAcceptor::mozilla_modern(server)?;
ssl_builder.set_default_verify_paths()?;
ssl_builder.set_verify(ssl::SslVerifyMode::PEER);

6
tokio-boring/tests/rpk.rs
View File

@ -2,7 +2,7 @@
use boring::pkey::PKey;
use boring::ssl::{
CertificateType, SslAcceptor, SslAlert, SslConnector, SslCredential, SslVerifyError,
CertificateType, SslAcceptor, SslAlert, SslConnector, SslCredential, SslMethod, SslVerifyError,
SslVerifyMode,
};
use futures::future;
@ -27,7 +27,7 @@ fn create_server() -> (
let addr = listener.local_addr().unwrap();
let server = async move {
let mut acceptor = SslAcceptor::rpk().unwrap();
let mut acceptor = SslAcceptor::mozilla_intermediate_v5(SslMethod::tls()).unwrap();
let private_key =
PKey::private_key_from_pem(&std::fs::read("tests/key.pem").unwrap()).unwrap();
let spki = std::fs::read("tests/pubkey.der").unwrap();
@ -58,7 +58,7 @@ async fn connect(
spki_path: &str,
is_ok_cell: &Arc<OnceLock<bool>>,
) -> Result<SslStream<TcpStream>, HandshakeError<TcpStream>> {
let mut connector = SslConnector::rpk_builder().unwrap();
let mut connector = SslConnector::builder(SslMethod::tls()).unwrap();
let spki = PKey::public_key_from_der(&std::fs::read(spki_path).unwrap()).unwrap();
let is_ok_cell = Arc::clone(is_ok_cell);