Skip to content

Commit

Permalink
Add SslCurve::to_nid() and remove SslCurveId
Browse files Browse the repository at this point in the history
We previously added an `SslCurveId` struct to house SSL_CURVE variants of
the internal NID constants, to allow `SslRef::curve()` to properly
instantiate `SslCurve` structures. This was done to ensure
`SslRef::set_curves()` did not break, as it expects the internal NID
constants instead of the public SSL_CURVE ones. In future versions of
boringssl, this problem is solved by virtue of the
SSL_CTX_set1_group_ids API. Since we don't have this yet, this commit
adds `SslCurve::nid()` so `SslRef::set_curves()` can convert the
SSL_CURVE constants to the NID representation internally
without breaking the public API.
  • Loading branch information
rushilmehra committed Aug 1, 2024
1 parent 07bfd55 commit 0e33475
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 54 deletions.
95 changes: 47 additions & 48 deletions boring/src/ssl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,63 +704,33 @@ impl From<u16> for SslSignatureAlgorithm {
pub struct SslCurve(c_int);

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

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

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

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

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

#[cfg(not(feature = "fips"))]
pub const X25519_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_X25519Kyber768Draft00);
pub const X25519_KYBER768_DRAFT00: SslCurve =
SslCurve(ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 as _);

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

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

#[cfg(feature = "pq-experimental")]
pub const P256_KYBER768_DRAFT00: SslCurve = SslCurve(ffi::NID_P256Kyber768Draft00);
}

/// A TLS Curve group ID.
#[repr(transparent)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct SslCurveId(u16);

impl SslCurveId {
pub const SECP224R1: SslCurveId = SslCurveId(ffi::SSL_CURVE_SECP224R1 as _);

pub const SECP256R1: SslCurveId = SslCurveId(ffi::SSL_CURVE_SECP256R1 as _);

pub const SECP384R1: SslCurveId = SslCurveId(ffi::SSL_CURVE_SECP384R1 as _);

pub const SECP521R1: SslCurveId = SslCurveId(ffi::SSL_CURVE_SECP521R1 as _);

pub const X25519: SslCurveId = SslCurveId(ffi::SSL_CURVE_X25519 as _);

#[cfg(not(feature = "fips"))]
pub const X25519_KYBER768_DRAFT00: SslCurveId =
SslCurveId(ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 as _);

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

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

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

#[cfg(feature = "pq-experimental")]
pub const IPD_WING: SslCurve = SslCurve(ffi::NID_IPDWing);
pub const IPD_WING: SslCurve = SslCurve(ffi::SSL_CURVE_IPDWING);

/// Returns the curve name
///
Expand All @@ -769,14 +739,38 @@ impl SslCurveId {
/// [`SSL_get_curve_name`]: https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_get_curve_name
pub fn name(&self) -> Option<&'static str> {
unsafe {
let ptr = ffi::SSL_get_curve_name(self.0);
let ptr = ffi::SSL_get_curve_name(self.0 as u16);
if ptr.is_null() {
return None;
}

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

// We need to allow dead_code here because `SslRef::set_curves` is conditionally compiled
// against the absence of the `kx-safe-default` feature and thus this function is never used.
#[allow(dead_code)]
fn nid(&self) -> Option<c_int> {
match self.0 {
ffi::SSL_CURVE_SECP224R1 => Some(ffi::NID_secp224r1),
ffi::SSL_CURVE_SECP256R1 => Some(ffi::NID_X9_62_prime256v1),
ffi::SSL_CURVE_SECP384R1 => Some(ffi::NID_secp384r1),
ffi::SSL_CURVE_SECP521R1 => Some(ffi::NID_secp521r1),
ffi::SSL_CURVE_X25519 => Some(ffi::NID_X25519),
#[cfg(not(feature = "fips"))]
ffi::SSL_CURVE_X25519_KYBER768_DRAFT00 => Some(ffi::NID_X25519Kyber768Draft00),
#[cfg(feature = "pq-experimental")]
ffi::SSL_CURVE_X25519_KYBER768_DRAFT00_OLD => Some(ffi::NID_X25519Kyber768Draft00Old),
#[cfg(feature = "pq-experimental")]
ffi::SSL_CURVE_X25519_KYBER512_DRAFT00 => Some(ffi::NID_X25519Kyber512Draft00),
#[cfg(feature = "pq-experimental")]
ffi::SSL_CURVE_P256_KYBER768_DRAFT00 => Some(ffi::NID_P256Kyber768Draft00),
#[cfg(feature = "pq-experimental")]
ffi::SSL_CURVE_IPDWING => Some(ffi::NID_IPDWing),
_ => None,
}
}
}

/// A compliance policy.
Expand Down Expand Up @@ -1964,11 +1958,16 @@ impl SslContextBuilder {
// when the flags are used, the preferences are set just before connecting or accepting.
#[cfg(not(feature = "kx-safe-default"))]
pub fn set_curves(&mut self, curves: &[SslCurve]) -> Result<(), ErrorStack> {
let mut nid_curves = Vec::with_capacity(curves.len());
for curve in curves {
nid_curves.push(curve.nid())
}

unsafe {
cvt_0i(ffi::SSL_CTX_set1_curves(
self.as_ptr(),
curves.as_ptr() as *const _,
curves.len(),
nid_curves.as_ptr() as *const _,
nid_curves.len(),
))
.map(|_| ())
}
Expand Down Expand Up @@ -2877,12 +2876,12 @@ impl SslRef {
/// This corresponds to [`SSL_get_curve_id`]
///
/// [`SSL_get_curve_id`]: https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_get_curve_id
pub fn curve(&self) -> Option<SslCurveId> {
pub fn curve(&self) -> Option<SslCurve> {
let curve_id = unsafe { ffi::SSL_get_curve_id(self.as_ptr()) };
if curve_id == 0 {
return None;
}
Some(SslCurveId(curve_id))
Some(SslCurve(curve_id.into()))
}

/// Returns an `ErrorCode` value for the most recent operation on this `SslRef`.
Expand Down
12 changes: 6 additions & 6 deletions boring/src/ssl/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::pkey::PKey;
use crate::srtp::SrtpProfileId;
use crate::ssl::test::server::Server;
use crate::ssl::SslVersion;
use crate::ssl::{self, SslCurveId};
use crate::ssl::{self, SslCurve};
use crate::ssl::{
ExtensionType, ShutdownResult, ShutdownState, Ssl, SslAcceptor, SslAcceptorBuilder,
SslConnector, SslContext, SslFiletype, SslMethod, SslOptions, SslStream, SslVerifyMode,
Expand Down Expand Up @@ -931,11 +931,11 @@ fn get_curve() {

#[test]
fn get_curve_name() {
assert_eq!(SslCurveId::SECP224R1.name(), Some("P-224"));
assert_eq!(SslCurveId::SECP256R1.name(), Some("P-256"));
assert_eq!(SslCurveId::SECP384R1.name(), Some("P-384"));
assert_eq!(SslCurveId::SECP521R1.name(), Some("P-521"));
assert_eq!(SslCurveId::X25519.name(), Some("X25519"));
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]
Expand Down

0 comments on commit 0e33475

Please sign in to comment.