Skip to content

Commit

Permalink
feature: pivot renegotiation bindings to managed by s2n-tls
Browse files Browse the repository at this point in the history
... rather than managed by the application, like in the C library.
This is required for use with s2n-tls-tokio and s2n-tls-hyper.
  • Loading branch information
lrstewart committed Sep 11, 2024
1 parent 341a69e commit 2053284
Show file tree
Hide file tree
Showing 5 changed files with 639 additions and 128 deletions.
12 changes: 8 additions & 4 deletions .github/workflows/ci_rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,19 @@ jobs:
- name: Generate
run: ${{env.ROOT_PATH}}/generate.sh

- name: Tests
# Ensure that all tests pass with the default feature set
- name: Default Tests
working-directory: ${{env.ROOT_PATH}}
run: cargo test

- name: Feature Tests: Fingerprint, kTLS, QUIC, and PQ
working-directory: ${{env.ROOT_PATH}}
# Test all features except for FIPS, which is tested separately.
run: cargo test --features unstable-fingerprint,unstable-ktls,quic,pq

# Ensure that all tests pass with the default feature set
- name: Default Tests
- name: Feature Test: Renegotiate
working-directory: ${{env.ROOT_PATH}}
run: cargo test
run: cargo test --features unstable-renegotiate

- name: Test external build
# if this test is failing, make sure that api headers are appropriately
Expand Down
31 changes: 30 additions & 1 deletion api/unstable/renegotiate.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,31 @@
* s2n-tls clients support secure (compliant with RFC5746) renegotiation for compatibility reasons,
* but s2n-tls does NOT recommend its use. While s2n-tls addresses all currently known security concerns,
* renegotiation has appeared in many CVEs and was completely removed from TLS1.3.
*
* A basic renegotiation integration with s2n-tls would look like:
* 1. The application calls s2n_recv as part of normal IO.
* 2. s2n_recv receives a request for renegotiation (a HelloRequest message)
* instead of application data.
* 3. s2n_recv calls the configured s2n_renegotiate_request_cb.
* 4. The application's implementation of the s2n_renegotiate_request_cb should:
* 1. Set the `response` parameter to S2N_RENEGOTIATE_ACCEPT
* 2. Set some application state to indicate that renegotiation is required.
* s2n_connection_set_ctx can be used to associate application state with
* a specific connection.
* 3. Return success.
* 5. s2n_recv returns as part of normal IO.
* 6. The application should check the application state set in 4.2 to determine
* whether or not renegotiation is required.
* 7. The application should complete any in-progress IO. Failing to do this will
* cause s2n_negotiate_wipe to fail.
* 1. For sending, the application must retry any blocked calls to s2n_send
* until they return success.
* 2. For receiving, the application must call s2n_recv to handle any buffered
* decrypted application data. s2n_peek indicates how much data is buffered.
* 8. The application should call s2n_renegotiate_wipe.
* 9. The application should reconfigure the connection, if necessary.
* 10. The application should call s2n_renegotiate until it indicates success,
* while handling any application data encountered.
*/

/**
Expand Down Expand Up @@ -92,7 +117,7 @@ S2N_API int s2n_config_set_renegotiate_request_cb(struct s2n_config *config, s2n
*
* The application MUST handle any incomplete IO before calling this method. The last call to `s2n_send` must
* have succeeded, and `s2n_peek` must return zero. If there is any data in the send or receive buffers,
* this method will fail.
* this method will fail. That means that this method cannot be called from inside s2n_renegotiate_request_cb.
*
* The application MUST repeat any connection-specific setup after calling this method. This method
* cannot distinguish between internal connection state and configuration state set by the application,
Expand Down Expand Up @@ -130,6 +155,10 @@ S2N_API int s2n_renegotiate_wipe(struct s2n_connection *conn);
* copy the data to `app_data_buf`, and set `app_data_size` to the size of the data.
* The application should handle the data in `app_data_buf` before calling s2n_renegotiate again.
*
* This method cannot be called from inside s2n_renegotiate_request_cb. The receive
* call that triggered s2n_renegotiate_request_cb must complete before either
* s2n_renegotiate_wipe or s2n_renegotiate can be called.
*
* @note s2n_renegotiate_wipe MUST be called before this method.
* @note Calling this method on a server connection will fail. s2n-tls servers do not support renegotiation.
*
Expand Down
46 changes: 42 additions & 4 deletions bindings/rust/s2n-tls/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#![allow(clippy::missing_safety_doc)] // TODO add safety docs

#[cfg(feature = "unstable-renegotiate")]
use crate::renegotiate::RenegotiateState;
use crate::{
callbacks::*,
cert_chain::CertificateChain,
Expand Down Expand Up @@ -579,23 +581,33 @@ impl Connection {
/// [negotiate](`Self::poll_negotiate`) has succeeded.
///
/// Returns the number of bytes written, and may indicate a partial write.
#[cfg(not(feature = "unstable-renegotiate"))]
pub fn poll_send(&mut self, buf: &[u8]) -> Poll<Result<usize, Error>> {
let mut blocked = s2n_blocked_status::NOT_BLOCKED;
let buf_len: isize = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?;
let buf_ptr = buf.as_ptr() as *const ::libc::c_void;
unsafe { s2n_send(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
}

#[cfg(not(feature = "unstable-renegotiate"))]
pub(crate) fn poll_recv_raw(
&mut self,
buf_ptr: *mut ::libc::c_void,
buf_len: isize,
) -> Poll<Result<usize, Error>> {
let mut blocked = s2n_blocked_status::NOT_BLOCKED;
unsafe { s2n_recv(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
}

/// Reads and decrypts data from a connection where
/// [negotiate](`Self::poll_negotiate`) has succeeded.
///
/// Returns the number of bytes read, and may indicate a partial read.
/// 0 bytes returned indicates EOF due to connection closure.
pub fn poll_recv(&mut self, buf: &mut [u8]) -> Poll<Result<usize, Error>> {
let mut blocked = s2n_blocked_status::NOT_BLOCKED;
let buf_len: isize = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?;
let buf_ptr = buf.as_ptr() as *mut ::libc::c_void;
unsafe { s2n_recv(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
self.poll_recv_raw(buf_ptr, buf_len)
}

/// Reads and decrypts data from a connection where
Expand All @@ -613,7 +625,6 @@ impl Connection {
&mut self,
buf: &mut [MaybeUninit<u8>],
) -> Poll<Result<usize, Error>> {
let mut blocked = s2n_blocked_status::NOT_BLOCKED;
let buf_len: isize = buf.len().try_into().map_err(|_| Error::INVALID_INPUT)?;
let buf_ptr = buf.as_ptr() as *mut ::libc::c_void;

Expand All @@ -622,7 +633,7 @@ impl Connection {
// 2. if s2n_recv returns `+n`, it guarantees that the first
// `n` bytes of `buf` have been initialized, which allows this
// function to return `Ok(n)`
unsafe { s2n_recv(self.connection.as_ptr(), buf_ptr, buf_len, &mut blocked).into_poll() }
self.poll_recv_raw(buf_ptr, buf_len)
}

/// Attempts to flush any data previously buffered by a call to [send](`Self::poll_send`).
Expand Down Expand Up @@ -910,6 +921,19 @@ impl Connection {
}
}

pub fn message_type(&self) -> Result<&str, Error> {
let message = unsafe {
s2n_connection_get_last_message_name(self.connection.as_ptr()).into_result()?
};
unsafe {
// SAFETY: Constructed strings have a null byte appended to them.
// SAFETY: The data has a 'static lifetime, because it resides in a
// static char array, and is never modified after its initial
// creation.
const_str!(message)
}
}

pub fn cipher_suite(&self) -> Result<&str, Error> {
let cipher = unsafe { s2n_connection_get_cipher(self.connection.as_ptr()).into_result()? };
unsafe {
Expand Down Expand Up @@ -1150,6 +1174,16 @@ impl Connection {
Some(app_context) => app_context.downcast_mut::<T>(),
}
}

#[cfg(feature = "unstable-renegotiate")]
pub(crate) fn renegotiate_state_mut(&mut self) -> &mut RenegotiateState {
&mut self.context_mut().renegotiate_state
}

#[cfg(feature = "unstable-renegotiate")]
pub(crate) fn renegotiate_state(&self) -> &RenegotiateState {
&self.context().renegotiate_state
}
}

struct Context {
Expand All @@ -1159,6 +1193,8 @@ struct Context {
verify_host_callback: Option<Box<dyn VerifyHostNameCallback>>,
connection_initialized: bool,
app_context: Option<Box<dyn Any + Send + Sync>>,
#[cfg(feature = "unstable-renegotiate")]
pub(crate) renegotiate_state: RenegotiateState,
}

impl Context {
Expand All @@ -1170,6 +1206,8 @@ impl Context {
verify_host_callback: None,
connection_initialized: false,
app_context: None,
#[cfg(feature = "unstable-renegotiate")]
renegotiate_state: RenegotiateState::default(),
}
}
}
Expand Down
Loading

0 comments on commit 2053284

Please sign in to comment.