Skip to content

Commit

Permalink
persist: avoid passing secrets in as Vec<u8>
Browse files Browse the repository at this point in the history
This avoids having them as 'loose' unzeroized type on the way to being moved
to their final home.

This is sufficient, because:

- tls12: the secret comes from
  `tls12::ConnectionSecrets::master_secret()` which borrows from its
  internal storage; `tls12::ConnectionSecrets::drop` zeroes this
  storage.
- tls13: the secret comes from
  `resumption_master_secret_and_derive_ticket_psk`, of type `hkdf::OkmBlock`,
  which we borrow from.  Only once the borrow finishes will that be
  zeroized.
  • Loading branch information
ctz committed Oct 20, 2023
1 parent 05bd018 commit 19d5eda
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 27 deletions.
13 changes: 2 additions & 11 deletions rustls/src/client/handy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ mod tests {
tls12_suite,
SessionId::empty(),
Vec::new(),
Vec::new(),
&[],
Vec::new(),
now,
0,
Expand All @@ -264,16 +264,7 @@ mod tests {
};
c.insert_tls13_ticket(
&name,
Tls13ClientSessionValue::new(
tls13_suite,
Vec::new(),
Vec::new(),
Vec::new(),
now,
0,
0,
0,
),
Tls13ClientSessionValue::new(tls13_suite, Vec::new(), &[], Vec::new(), now, 0, 0, 0),
);
assert!(c.take_tls13_ticket(&name).is_none());
}
Expand Down
2 changes: 1 addition & 1 deletion rustls/src/client/tls12.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,7 @@ impl ExpectFinished {
self.secrets.suite(),
self.session_id,
ticket,
self.secrets.get_master_secret(),
self.secrets.master_secret(),
cx.common
.peer_certificates
.clone()
Expand Down
2 changes: 1 addition & 1 deletion rustls/src/client/tls13.rs
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ impl ExpectTraffic {
let mut value = persist::Tls13ClientSessionValue::new(
self.suite,
nst.ticket.0.clone(),
secret.as_ref().to_vec(),
secret.as_ref(),
cx.common
.peer_certificates
.clone()
Expand Down
14 changes: 7 additions & 7 deletions rustls/src/msgs/persist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl Tls13ClientSessionValue {
pub(crate) fn new(
suite: &'static Tls13CipherSuite,
ticket: Vec<u8>,
secret: Vec<u8>,
secret: &[u8],
server_cert_chain: Vec<CertificateDer<'static>>,
time_now: UnixTime,
lifetime_secs: u32,
Expand Down Expand Up @@ -158,7 +158,7 @@ impl Tls12ClientSessionValue {
suite: &'static Tls12CipherSuite,
session_id: SessionId,
ticket: Vec<u8>,
master_secret: Vec<u8>,
master_secret: &[u8],
server_cert_chain: Vec<CertificateDer<'static>>,
time_now: UnixTime,
lifetime_secs: u32,
Expand Down Expand Up @@ -218,14 +218,14 @@ pub struct ClientSessionCommon {
impl ClientSessionCommon {
fn new(
ticket: Vec<u8>,
secret: Vec<u8>,
secret: &[u8],
time_now: UnixTime,
lifetime_secs: u32,
server_cert_chain: Vec<CertificateDer<'static>>,
) -> Self {
Self {
ticket: PayloadU16(ticket),
secret: Zeroizing::new(PayloadU8(secret)),
secret: Zeroizing::new(PayloadU8(secret.to_vec())),
epoch: time_now.as_secs(),
lifetime_secs: cmp::min(lifetime_secs, MAX_TICKET_LIFETIME),
server_cert_chain,
Expand Down Expand Up @@ -355,7 +355,7 @@ impl ServerSessionValue {
sni: Option<&DnsName>,
v: ProtocolVersion,
cs: CipherSuite,
ms: Vec<u8>,
ms: &[u8],
client_cert_chain: Option<CertificatePayload>,
alpn: Option<Vec<u8>>,
application_data: Vec<u8>,
Expand All @@ -366,7 +366,7 @@ impl ServerSessionValue {
sni: sni.cloned(),
version: v,
cipher_suite: cs,
master_secret: Zeroizing::new(PayloadU8::new(ms)),
master_secret: Zeroizing::new(PayloadU8::new(ms.to_vec())),
extended_ms: false,
client_cert_chain,
alpn: alpn.map(PayloadU8::new),
Expand Down Expand Up @@ -420,7 +420,7 @@ mod tests {
None,
ProtocolVersion::TLSv1_3,
CipherSuite::TLS13_AES_128_GCM_SHA256,
vec![1, 2, 3],
&[1, 2, 3],
None,
None,
vec![4, 5, 6],
Expand Down
3 changes: 1 addition & 2 deletions rustls/src/server/tls12.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,13 +751,12 @@ fn get_server_connection_value_tls12(
time_now: UnixTime,
) -> persist::ServerSessionValue {
let version = ProtocolVersion::TLSv1_2;
let secret = secrets.get_master_secret();

let mut v = persist::ServerSessionValue::new(
cx.data.sni.as_ref(),
version,
secrets.suite().common.suite,
secret,
secrets.master_secret(),
cx.common.peer_certificates.clone(),
cx.common.alpn_protocol.clone(),
cx.data.resumption_data.clone(),
Expand Down
2 changes: 1 addition & 1 deletion rustls/src/server/tls13.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ fn get_server_session_value(
cx.data.sni.as_ref(),
version,
suite.common.suite,
secret.as_ref().to_vec(),
secret.as_ref(),
cx.common.peer_certificates.clone(),
cx.common.alpn_protocol.clone(),
cx.data.resumption_data.clone(),
Expand Down
6 changes: 2 additions & 4 deletions rustls/src/tls12/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,8 @@ impl ConnectionSecrets {
self.suite
}

pub(crate) fn get_master_secret(&self) -> Vec<u8> {
let mut ret = Vec::new();
ret.extend_from_slice(&self.master_secret);
ret
pub(crate) fn master_secret(&self) -> &[u8] {
&self.master_secret[..]
}

fn make_verify_data(&self, handshake_hash: &hash::Output, label: &[u8]) -> Vec<u8> {
Expand Down

0 comments on commit 19d5eda

Please sign in to comment.