Skip to content
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.

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

Add support for SSL_group_to_name and SSL_get_negotiated_group #2201

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
5 changes: 5 additions & 0 deletions openssl-sys/src/handwritten/ssl.rs
Expand Up @@ -951,3 +951,8 @@ extern "C" {
#[cfg(any(ossl110, libressl360))]
pub fn SSL_get_security_level(s: *const SSL) -> c_int;
}

extern "C" {
#[cfg(ossl300)]
pub fn SSL_group_to_name(ssl: *mut SSL, id: c_int) -> *const c_char;
}
6 changes: 6 additions & 0 deletions openssl-sys/src/ssl.rs
Expand Up @@ -363,6 +363,8 @@ pub const SSL_CTRL_GET_MIN_PROTO_VERSION: c_int = 130;
pub const SSL_CTRL_GET_MAX_PROTO_VERSION: c_int = 131;
#[cfg(ossl300)]
pub const SSL_CTRL_GET_TMP_KEY: c_int = 133;
#[cfg(ossl300)]
pub const SSL_CTRL_GET_NEGOTIATED_GROUP: c_int = 134;

pub unsafe fn SSL_CTX_set_tmp_dh(ctx: *mut SSL_CTX, dh: *mut DH) -> c_long {
SSL_CTX_ctrl(ctx, SSL_CTRL_SET_TMP_DH, 0, dh as *mut c_void)
Expand Down Expand Up @@ -519,6 +521,10 @@ cfg_if! {
pub unsafe fn SSL_get_tmp_key(ssl: *mut SSL, key: *mut *mut EVP_PKEY) -> c_long {
SSL_ctrl(ssl, SSL_CTRL_GET_TMP_KEY, 0, key as *mut c_void)
}

pub unsafe fn SSL_get_negotiated_group(ssl: *mut SSL) -> c_int {
SSL_ctrl(ssl, SSL_CTRL_GET_NEGOTIATED_GROUP, 0, ptr::null_mut()) as c_int
}
}
}

Expand Down
43 changes: 42 additions & 1 deletion openssl/src/ssl/mod.rs
Expand Up @@ -82,7 +82,7 @@ use crate::x509::store::{X509Store, X509StoreBuilderRef, X509StoreRef};
#[cfg(any(ossl102, boringssl, libressl261))]
use crate::x509::verify::X509VerifyParamRef;
use crate::x509::{X509Name, X509Ref, X509StoreContextRef, X509VerifyResult, X509};
use crate::{cvt, cvt_n, cvt_p, init};
use crate::{cvt, cvt_n, cvt_p, cvt_p_const, init};
use bitflags::bitflags;
use cfg_if::cfg_if;
use foreign_types::{ForeignType, ForeignTypeRef, Opaque};
Expand Down Expand Up @@ -3484,6 +3484,47 @@ impl SslRef {
}
}
}

/// Returns the NID of the negotiated group used for the handshake key
/// exchange process.
/// For TLSv1.3 connections this typically reflects the state of the
/// current connection, though in the case of PSK-only resumption, the
/// returned value will be from a previous connection.
/// For earlier TLS versions, when a session has been resumed, it always
/// reflects the group used for key exchange during the initial handshake
/// (otherwise it is from the current, non-resumption, connection).
/// This can be called by either client or server.
/// If the NID for the shared group is unknown then the value is set to the
/// bitwise OR of TLSEXT_nid_unknown (0x1000000) and the id of the group.
#[corresponds(SSL_get_negotiated_group)]
#[cfg(ossl300)]
pub fn negotiated_group(&self) -> Result<c_int, ErrorStack> {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we shouldn't be exposing this as a raw c_int. Since their doing the weird unknown thing, I think we'd need to make a NegotiatedGroup type that provides a pub fn nid(self) -> Option<Nid> and pub fn unknown_group_id(self) -> Option<i32>.

Copy link
Author

Choose a reason for hiding this comment

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

I can do this change.

Then again in SSL::group_to_name which calls SSL_group_to_name(), we need the raw unaltered c_int.

Should the NegotiatedGroup type implement From/To trait to convert back to the raw c_int?
Suggestions on how to implement that?

Copy link
Owner

Choose a reason for hiding this comment

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

Yep, these wrapper types have an as_raw method to get the c_int out: https://docs.rs/openssl/latest/openssl/nid/struct.Nid.html#method.as_raw

Copy link
Author

Choose a reason for hiding this comment

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

I tried addressing this with 893ce6a

unsafe { cvt(ffi::SSL_get_negotiated_group(self.as_ptr())) }
}

/// Return retrieve the TLS group name associated with a given TLS
/// group ID, as registered via built-in or external providers and as
/// returned by a call to SSL_get1_groups() or SSL_get_shared_group().
///
/// If non-NULL, SSL_group_to_name() returns the TLS group name
/// corresponding to the given id as a NUL-terminated string.
/// If SSL_group_to_name() returns NULL, an error occurred; possibly no
/// corresponding tlsname was registered during provider initialisation.
///
/// Note that the return value is valid only during the lifetime of the
/// SSL object ssl.
#[corresponds(SSL_group_to_name)]
#[cfg(ossl300)]
pub fn group_to_name(&self, id: c_int) -> Result<&str, ErrorStack> {
unsafe {
match cvt_p_const(ffi::SSL_group_to_name(self.as_ptr(), id)) {
Ok(constp) => Ok(CStr::from_ptr(constp)
.to_str()
.expect("Invalid UTF8 in input")),
romen marked this conversation as resolved.
Show resolved Hide resolved
Err(e) => Err(e),
}
}
}
}

/// An SSL stream midway through the handshake process.
Expand Down