From 3e922e5ededfc9c7915407cf30171342078cc30e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 20 Sep 2022 09:36:49 -0400 Subject: [PATCH] Intoduce1: Use a constant-time check for all-zero RsaIdentity As a matter of good crypto practice, we shouldn't use short-circuiting checks to compare keys or key-like objects, since the amount of time taken by those checks can leak information about their inputs. I don't think it's actually _necessary_ to use a constant-time operation in this case, but let's establish the precedent. This is a follow-up to !724. --- crates/tor-cell/src/relaycell/onion_service.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/tor-cell/src/relaycell/onion_service.rs b/crates/tor-cell/src/relaycell/onion_service.rs index c39396d0c..796bdde6a 100644 --- a/crates/tor-cell/src/relaycell/onion_service.rs +++ b/crates/tor-cell/src/relaycell/onion_service.rs @@ -7,6 +7,7 @@ use super::msg; use caret::caret_int; use tor_bytes::{EncodeError, EncodeResult, Error as BytesError, Readable, Result, Writeable}; use tor_bytes::{Reader, Writer}; +use tor_llcrypto::pk::rsa::RsaIdentity; use tor_units::BoundedInt32; caret_int! { @@ -269,8 +270,8 @@ impl msg::Body for Introduce1 { msg::RelayMsg::Introduce1(self) } fn decode_from_reader(r: &mut Reader<'_>) -> Result { - let legacy_key_id: [u8; 20] = r.extract()?; - if legacy_key_id.iter().any(|b| *b != 0_u8) { + let legacy_key_id: RsaIdentity = r.extract()?; + if !legacy_key_id.is_zero() { return Err(BytesError::BadMessage("legacy key id in Introduce1.")); } let auth_key_type = r.take_u8()?.into();