Skip to content

Remove SslCurve API #390

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

Merged
ghedo merged 3 commits into from
Sep 30, 2025
Merged

Remove SslCurve API #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df76125
Remove `SslCurve` API
cjpatton Sep 29, 2025
b1dcfbb
Remove outdated comments on FIPS API compatibility
cjpatton Sep 29, 2025
af1fec1
Add back the `curve()` method on `SslRef`
cjpatton Sep 30, 2025
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
126 changes: 3 additions & 123 deletions boring/src/ssl/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,86 +695,6 @@ impl From<u16> for SslSignatureAlgorithm {
}
}

/// Numeric identifier of a TLS curve.
#[repr(transparent)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct SslCurveNid(c_int);

/// A TLS Curve.
#[repr(transparent)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct SslCurve(c_int);

impl SslCurve {
pub const SECP224R1: SslCurve = SslCurve(ffi::SSL_GROUP_SECP224R1 as _);

pub const SECP256R1: SslCurve = SslCurve(ffi::SSL_GROUP_SECP256R1 as _);

pub const SECP384R1: SslCurve = SslCurve(ffi::SSL_GROUP_SECP384R1 as _);

pub const SECP521R1: SslCurve = SslCurve(ffi::SSL_GROUP_SECP521R1 as _);

pub const X25519: SslCurve = SslCurve(ffi::SSL_GROUP_X25519 as _);

pub const X25519_KYBER768_DRAFT00: SslCurve =
SslCurve(ffi::SSL_GROUP_X25519_KYBER768_DRAFT00 as _);

#[cfg(feature = "pq-experimental")]
pub const X25519_KYBER768_DRAFT00_OLD: SslCurve =
SslCurve(ffi::SSL_GROUP_X25519_KYBER768_DRAFT00_OLD as _);

#[cfg(feature = "pq-experimental")]
pub const X25519_KYBER512_DRAFT00: SslCurve =
SslCurve(ffi::SSL_GROUP_X25519_KYBER512_DRAFT00 as _);

#[cfg(feature = "pq-experimental")]
pub const P256_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::SSL_GROUP_P256_KYBER768_DRAFT00 as _);

#[cfg(feature = "pq-experimental")]
pub const X25519_MLKEM768: SslCurve = SslCurve(ffi::SSL_GROUP_X25519_MLKEM768 as _);

/// Returns the curve name
#[corresponds(SSL_get_curve_name)]
#[must_use]
pub fn name(&self) -> Option<&'static str> {
unsafe {
let ptr = ffi::SSL_get_curve_name(self.0 as u16);
if ptr.is_null() {
return None;
}

CStr::from_ptr(ptr).to_str().ok()
}
}

// **NOTE**: This function only exists because the version of boringssl we currently use does
// not expose SSL_CTX_set1_group_ids. Because `SslRef::curve()` returns the public SSL_GROUP id
// as opposed to the internal NID, but `SslContextBuilder::set_curves()` requires the internal
// NID, we need this mapping in place to avoid breaking changes to the public API. Once the
// underlying boringssl version is upgraded, this should be removed in favor of the new
// SSL_CTX_set1_group_ids API.
pub fn nid(&self) -> Option<SslCurveNid> {
match self.0 {
ffi::SSL_GROUP_SECP224R1 => Some(ffi::NID_secp224r1),
ffi::SSL_GROUP_SECP256R1 => Some(ffi::NID_X9_62_prime256v1),
ffi::SSL_GROUP_SECP384R1 => Some(ffi::NID_secp384r1),
ffi::SSL_GROUP_SECP521R1 => Some(ffi::NID_secp521r1),
ffi::SSL_GROUP_X25519 => Some(ffi::NID_X25519),
ffi::SSL_GROUP_X25519_KYBER768_DRAFT00 => Some(ffi::NID_X25519Kyber768Draft00),
#[cfg(feature = "pq-experimental")]
ffi::SSL_GROUP_X25519_KYBER768_DRAFT00_OLD => Some(ffi::NID_X25519Kyber768Draft00Old),
#[cfg(feature = "pq-experimental")]
ffi::SSL_GROUP_X25519_KYBER512_DRAFT00 => Some(ffi::NID_X25519Kyber512Draft00),
#[cfg(feature = "pq-experimental")]
ffi::SSL_GROUP_P256_KYBER768_DRAFT00 => Some(ffi::NID_P256Kyber768Draft00),
#[cfg(feature = "pq-experimental")]
ffi::SSL_GROUP_X25519_MLKEM768 => Some(ffi::NID_X25519MLKEM768),
_ => None,
}
.map(SslCurveNid)
}
}

/// A compliance policy.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct CompliancePolicy(ffi::ssl_compliance_policy_t);
Expand Down Expand Up @@ -1975,10 +1895,6 @@ impl SslContextBuilder {
}

/// Configures whether ClientHello extensions should be permuted.
///
/// Note: This is gated to non-fips because the fips feature builds with a separate
/// version of BoringSSL which doesn't yet include these APIs.
/// Once the submoduled fips commit is upgraded, these gates can be removed.
#[corresponds(SSL_CTX_set_permute_extensions)]
pub fn set_permute_extensions(&mut self, enabled: bool) {
unsafe { ffi::SSL_CTX_set_permute_extensions(self.as_ptr(), enabled as _) }
Expand Down Expand Up @@ -2025,24 +1941,6 @@ impl SslContextBuilder {
}
}

/// Sets the context's supported curves.
#[corresponds(SSL_CTX_set1_curves)]
pub fn set_curves(&mut self, curves: &[SslCurve]) -> Result<(), ErrorStack> {
let curves: Vec<i32> = curves
.iter()
.filter_map(|curve| curve.nid().map(|nid| nid.0))
.collect();

unsafe {
cvt_0i(ffi::SSL_CTX_set1_curves(
self.as_ptr(),
curves.as_ptr() as *const _,
curves.len(),
))
.map(|_| ())
}
}

/// Sets the context's compliance policy.
///
/// This feature isn't available in the certified version of BoringSSL.
Expand Down Expand Up @@ -2887,29 +2785,15 @@ impl SslRef {
}
}

/// Sets the ongoing session's supported groups by their named identifiers
/// (formerly referred to as curves).
#[corresponds(SSL_set1_groups)]
pub fn set_group_nids(&mut self, group_nids: &[SslCurveNid]) -> Result<(), ErrorStack> {
unsafe {
cvt_0i(ffi::SSL_set1_curves(
self.as_ptr(),
group_nids.as_ptr() as *const _,
group_nids.len(),
))
.map(|_| ())
}
}

/// Returns the [`SslCurve`] used for this `SslRef`.
/// Returns the curve ID (aka group ID) used for this `SslRef`.
Copy link
Collaborator Author

@cjpatton cjpatton Sep 30, 2025

Choose a reason for hiding this comment

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

If upstream BoringSSL is moving to replace "curve" with "group" in their API, then we might want do so here and for set_curves_list as well.

Copy link
Member

@ghedo ghedo Sep 30, 2025

Choose a reason for hiding this comment

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

It's not clear they are going to replace them since they still provide both names and the "group" APIs were added more than 2 years ago already. "group" is also not really a much better name given PQ... I think it was just for OpenSSL "compatibility".

#[corresponds(SSL_get_curve_id)]
#[must_use]
pub fn curve(&self) -> Option<SslCurve> {
pub fn curve(&self) -> Option<u16> {
let curve_id = unsafe { ffi::SSL_get_curve_id(self.as_ptr()) };
if curve_id == 0 {
return None;
}
Some(SslCurve(curve_id.into()))
Some(curve_id)
}

/// Returns an `ErrorCode` value for the most recent operation on this `SslRef`.
Expand Down Expand Up @@ -3061,10 +2945,6 @@ impl SslRef {

/// Configures whether ClientHello extensions should be permuted.
#[corresponds(SSL_set_permute_extensions)]
///
/// Note: This is gated to non-fips because the fips feature builds with a separate
/// version of BoringSSL which doesn't yet include these APIs.
/// Once the submoduled fips commit is upgraded, these gates can be removed.
pub fn set_permute_extensions(&mut self, enabled: bool) {
unsafe { ffi::SSL_set_permute_extensions(self.as_ptr(), enabled as _) }
}
Expand Down
28 changes: 3 additions & 25 deletions boring/src/ssl/test/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ use crate::pkey::PKey;
use crate::srtp::SrtpProfileId;
use crate::ssl::test::server::Server;
use crate::ssl::SslVersion;
use crate::ssl::{self, SslCurve};
use crate::ssl::{
ExtensionType, ShutdownResult, ShutdownState, Ssl, SslAcceptor, SslAcceptorBuilder,
self, ExtensionType, ShutdownResult, ShutdownState, Ssl, SslAcceptor, SslAcceptorBuilder,
SslConnector, SslContext, SslFiletype, SslMethod, SslOptions, SslStream, SslVerifyMode,
};
use crate::x509::store::X509StoreBuilder;
Expand Down Expand Up @@ -957,29 +956,8 @@ fn get_curve() {
let server = Server::builder().build();
let client = server.client_with_root_ca();
let client_stream = client.connect();
let curve = client_stream.ssl().curve().expect("curve");
assert!(curve.name().is_some());
}

#[test]
fn get_curve_name() {
assert_eq!(SslCurve::SECP224R1.name(), Some("P-224"));
assert_eq!(SslCurve::SECP256R1.name(), Some("P-256"));
assert_eq!(SslCurve::SECP384R1.name(), Some("P-384"));
assert_eq!(SslCurve::SECP521R1.name(), Some("P-521"));
assert_eq!(SslCurve::X25519.name(), Some("X25519"));
}

#[test]
fn set_curves() {
let mut ctx = SslContext::builder(SslMethod::tls()).unwrap();
ctx.set_curves(&[
SslCurve::SECP224R1,
SslCurve::SECP256R1,
SslCurve::SECP384R1,
SslCurve::X25519,
])
.expect("Failed to set curves");
let curve = client_stream.ssl().curve();
assert!(curve.is_some());
}

#[test]
Expand Down
Loading