diff --git a/Cargo.lock b/Cargo.lock index c519976376..301e93c57e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5016,6 +5016,7 @@ dependencies = [ "rand", "serde", "serde_json", + "strum 0.26.3", "thiserror", "tokio", "tracing", diff --git a/common/client-core/gateways-storage/fs_gateways_migrations/20240916120000_add_aes256_gcm_siv_key.sql b/common/client-core/gateways-storage/fs_gateways_migrations/20240916120000_add_aes256_gcm_siv_key.sql new file mode 100644 index 0000000000..66a8632bb4 --- /dev/null +++ b/common/client-core/gateways-storage/fs_gateways_migrations/20240916120000_add_aes256_gcm_siv_key.sql @@ -0,0 +1,13 @@ +/* + * Copyright 2024 - Nym Technologies SA + * SPDX-License-Identifier: Apache-2.0 + */ + +-- make aes128 key column nullable and add aes256 column +ALTER TABLE remote_gateway_details RENAME COLUMN derived_aes128_ctr_blake3_hmac_keys_bs58 TO derived_aes128_ctr_blake3_hmac_keys_bs58_old; +ALTER TABLE remote_gateway_details ADD COLUMN derived_aes128_ctr_blake3_hmac_keys_bs58 TEXT; +ALTER TABLE remote_gateway_details ADD COLUMN derived_aes256_gcm_siv_key BLOB; + +UPDATE remote_gateway_details SET derived_aes128_ctr_blake3_hmac_keys_bs58 = derived_aes128_ctr_blake3_hmac_keys_bs58_old; + +ALTER TABLE remote_gateway_details DROP COLUMN derived_aes128_ctr_blake3_hmac_keys_bs58_old; \ No newline at end of file diff --git a/common/client-core/gateways-storage/src/error.rs b/common/client-core/gateways-storage/src/error.rs index c867557108..effac347fb 100644 --- a/common/client-core/gateways-storage/src/error.rs +++ b/common/client-core/gateways-storage/src/error.rs @@ -36,6 +36,9 @@ pub enum BadGateway { source: SharedKeyConversionError, }, + #[error("could not find any valid shared keys for gateway {gateway_id}")] + MissingSharedKey { gateway_id: String }, + #[error( "the listening address of gateway {gateway_id} ({raw_listener}) is malformed: {source}" )] diff --git a/common/client-core/gateways-storage/src/types.rs b/common/client-core/gateways-storage/src/types.rs index 94888faf9c..1b6cb35265 100644 --- a/common/client-core/gateways-storage/src/types.rs +++ b/common/client-core/gateways-storage/src/types.rs @@ -4,9 +4,12 @@ use crate::BadGateway; use cosmrs::AccountId; use nym_crypto::asymmetric::identity; -use nym_gateway_requests::registration::handshake::LegacySharedKeys; +use nym_gateway_requests::registration::handshake::{ + LegacySharedKeys, SharedGatewayKey, SharedSymmetricKey, +}; use serde::{Deserialize, Serialize}; use std::fmt::{Display, Formatter}; +use std::ops::Deref; use std::str::FromStr; use std::sync::Arc; use time::OffsetDateTime; @@ -64,13 +67,13 @@ impl From for GatewayRegistration { impl GatewayDetails { pub fn new_remote( gateway_id: identity::PublicKey, - derived_aes128_ctr_blake3_hmac_keys: Arc, + shared_key: Arc, gateway_owner_address: Option, gateway_listener: Url, ) -> Self { GatewayDetails::Remote(RemoteGatewayDetails { gateway_id, - derived_aes128_ctr_blake3_hmac_keys, + shared_key, gateway_owner_address, gateway_listener, }) @@ -87,9 +90,9 @@ impl GatewayDetails { } } - pub fn shared_key(&self) -> Option<&LegacySharedKeys> { + pub fn shared_key(&self) -> Option<&SharedGatewayKey> { match self { - GatewayDetails::Remote(details) => Some(&details.derived_aes128_ctr_blake3_hmac_keys), + GatewayDetails::Remote(details) => Some(&details.shared_key), GatewayDetails::Custom(_) => None, } } @@ -167,7 +170,8 @@ pub struct RegisteredGateway { #[cfg_attr(feature = "sqlx", derive(sqlx::FromRow))] pub struct RawRemoteGatewayDetails { pub gateway_id_bs58: String, - pub derived_aes128_ctr_blake3_hmac_keys_bs58: String, + pub derived_aes128_ctr_blake3_hmac_keys_bs58: Option, + pub derived_aes256_gcm_siv_key: Option>, pub gateway_owner_address: Option, pub gateway_listener: String, } @@ -184,15 +188,35 @@ impl TryFrom for RemoteGatewayDetails { } })?; - let derived_aes128_ctr_blake3_hmac_keys = Arc::new( - LegacySharedKeys::try_from_base58_string( + let shared_key = + match ( + &value.derived_aes256_gcm_siv_key, &value.derived_aes128_ctr_blake3_hmac_keys_bs58, - ) - .map_err(|source| BadGateway::MalformedSharedKeys { - gateway_id: value.gateway_id_bs58.clone(), - source, - })?, - ); + ) { + (None, None) => { + return Err(BadGateway::MissingSharedKey { + gateway_id: value.gateway_id_bs58.clone(), + }) + } + (Some(aes256gcm_siv), _) => { + let current_key = + SharedSymmetricKey::try_from_bytes(aes256gcm_siv).map_err(|source| { + BadGateway::MalformedSharedKeys { + gateway_id: value.gateway_id_bs58.clone(), + source, + } + })?; + SharedGatewayKey::Current(current_key) + } + (None, Some(aes128ctr_hmac)) => { + let legacy_key = LegacySharedKeys::try_from_base58_string(aes128ctr_hmac) + .map_err(|source| BadGateway::MalformedSharedKeys { + gateway_id: value.gateway_id_bs58.clone(), + source, + })?; + SharedGatewayKey::Legacy(legacy_key) + } + }; let gateway_owner_address = value .gateway_owner_address @@ -218,7 +242,7 @@ impl TryFrom for RemoteGatewayDetails { Ok(RemoteGatewayDetails { gateway_id, - derived_aes128_ctr_blake3_hmac_keys, + shared_key: Arc::new(shared_key), gateway_owner_address, gateway_listener, }) @@ -227,11 +251,21 @@ impl TryFrom for RemoteGatewayDetails { impl<'a> From<&'a RemoteGatewayDetails> for RawRemoteGatewayDetails { fn from(value: &'a RemoteGatewayDetails) -> Self { + /* + pub derived_aes128_ctr_blake3_hmac_keys_bs58: Option, + pub derived_aes256_gcm_siv_key: Option>, + */ + + let (derived_aes128_ctr_blake3_hmac_keys_bs58, derived_aes256_gcm_siv_key) = + match value.shared_key.deref() { + SharedGatewayKey::Current(key) => (None, Some(key.to_bytes())), + SharedGatewayKey::Legacy(key) => (Some(key.to_base58_string()), None), + }; + RawRemoteGatewayDetails { gateway_id_bs58: value.gateway_id.to_base58_string(), - derived_aes128_ctr_blake3_hmac_keys_bs58: value - .derived_aes128_ctr_blake3_hmac_keys - .to_base58_string(), + derived_aes128_ctr_blake3_hmac_keys_bs58, + derived_aes256_gcm_siv_key, gateway_owner_address: value.gateway_owner_address.as_ref().map(|o| o.to_string()), gateway_listener: value.gateway_listener.to_string(), } @@ -242,9 +276,7 @@ impl<'a> From<&'a RemoteGatewayDetails> for RawRemoteGatewayDetails { pub struct RemoteGatewayDetails { pub gateway_id: identity::PublicKey, - // note: `SharedKeys` implement ZeroizeOnDrop, meaning when `RemoteGatewayDetails` is dropped, - // the keys will be zeroized - pub derived_aes128_ctr_blake3_hmac_keys: Arc, + pub shared_key: Arc, pub gateway_owner_address: Option, diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 0579cc12a8..ba2b20c8b4 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -387,7 +387,7 @@ where ), cfg, managed_keys.identity_keypair(), - Some(details.derived_aes128_ctr_blake3_hmac_keys), + Some(details.shared_key), packet_router, bandwidth_controller, shutdown, diff --git a/common/client-core/src/client/base_client/storage/migration_helpers.rs b/common/client-core/src/client/base_client/storage/migration_helpers.rs index 91d7a0bf93..61c5f59f1d 100644 --- a/common/client-core/src/client/base_client/storage/migration_helpers.rs +++ b/common/client-core/src/client/base_client/storage/migration_helpers.rs @@ -91,7 +91,7 @@ pub mod v1_1_33 { .map_err(|err| ClientCoreError::UpgradeFailure { message: format!("the stored gateway id was malformed: {err}"), })?, - derived_aes128_ctr_blake3_hmac_keys: Arc::new(gateway_shared_key), + shared_key: Arc::new(gateway_shared_key.into()), gateway_owner_address: Some(gateway_owner.parse().map_err(|err| { ClientCoreError::UpgradeFailure { message: format!("the stored gateway owner address was malformed: {err}"), diff --git a/common/client-core/src/client/topology_control/mod.rs b/common/client-core/src/client/topology_control/mod.rs index ae50128942..bbf32f377c 100644 --- a/common/client-core/src/client/topology_control/mod.rs +++ b/common/client-core/src/client/topology_control/mod.rs @@ -102,6 +102,7 @@ impl TopologyRefresher { .current_topology() .await .ok_or(NymTopologyError::EmptyNetworkTopology)?; + if !topology.gateway_exists(gateway) { return Err(NymTopologyError::NonExistentGatewayError { identity_key: gateway.to_base58_string(), diff --git a/common/client-core/src/init/types.rs b/common/client-core/src/init/types.rs index d0744abf62..2acf91311f 100644 --- a/common/client-core/src/init/types.rs +++ b/common/client-core/src/init/types.rs @@ -11,7 +11,7 @@ use nym_client_core_gateways_storage::{ }; use nym_crypto::asymmetric::identity; use nym_gateway_client::client::InitGatewayClient; -use nym_gateway_requests::registration::handshake::LegacySharedKeys; +use nym_gateway_requests::registration::handshake::SharedGatewayKey; use nym_sphinx::addressing::clients::Recipient; use nym_topology::gateway; use nym_validator_client::client::IdentityKey; @@ -104,7 +104,7 @@ impl SelectedGateway { /// - shared keys derived between ourselves and the node /// - an authenticated handle of an ephemeral handle created for the purposes of registration pub struct RegistrationResult { - pub shared_keys: Arc, + pub shared_keys: Arc, pub authenticated_ephemeral_client: InitGatewayClient, } diff --git a/common/client-libs/gateway-client/src/client/mod.rs b/common/client-libs/gateway-client/src/client/mod.rs index 1a29bba376..6ed2b98ec9 100644 --- a/common/client-libs/gateway-client/src/client/mod.rs +++ b/common/client-libs/gateway-client/src/client/mod.rs @@ -17,9 +17,7 @@ use nym_credential_storage::ephemeral_storage::EphemeralStorage as EphemeralCred use nym_credential_storage::storage::Storage as CredentialStorage; use nym_credentials::CredentialSpendingData; use nym_crypto::asymmetric::identity; -use nym_gateway_requests::authentication::encrypted_address::EncryptedAddressBytes; -use nym_gateway_requests::iv::IV; -use nym_gateway_requests::registration::handshake::{client_handshake, LegacySharedKeys}; +use nym_gateway_requests::registration::handshake::{client_handshake, SharedGatewayKey}; use nym_gateway_requests::{ BinaryRequest, ClientControlRequest, ServerResponse, AES_GCM_SIV_PROTOCOL_VERSION, CREDENTIAL_UPDATE_V2_PROTOCOL_VERSION, CURRENT_PROTOCOL_VERSION, @@ -82,7 +80,7 @@ pub struct GatewayClient { gateway_address: String, gateway_identity: identity::PublicKey, local_identity: Arc, - shared_key: Option>, + shared_key: Option>, connection: SocketState, packet_router: PacketRouter, bandwidth_controller: Option>, @@ -100,7 +98,7 @@ impl GatewayClient { gateway_config: GatewayConfig, local_identity: Arc, // TODO: make it mandatory. if you don't want to pass it, use `new_init` - shared_key: Option>, + shared_key: Option>, packet_router: PacketRouter, bandwidth_controller: Option>, task_client: TaskClient, @@ -432,9 +430,7 @@ impl GatewayClient { .await .map_err(GatewayClientError::RegistrationFailure), _ => return Err(GatewayClientError::ConnectionInInvalidState), - }; - - println!("registration result: {shared_key:?}"); + }?; let (authentication_status, gateway_protocol) = match self.read_control_response().await? { ServerResponse::Register { @@ -450,54 +446,48 @@ impl GatewayClient { self.check_gateway_protocol(gateway_protocol)?; self.authenticated = authentication_status; - todo!() - // if self.authenticated { - // self.shared_key = Some(Arc::new(shared_key)); - // } - // - // // populate the negotiated protocol for future uses - // self.negotiated_protocol = gateway_protocol; - // - // Ok(()) + if self.authenticated { + self.shared_key = Some(Arc::new(shared_key)); + } + + // populate the negotiated protocol for future uses + self.negotiated_protocol = gateway_protocol; + + Ok(()) } - async fn authenticate( - &mut self, - shared_key: Option, - supports_aes_gcm_siv: bool, - ) -> Result<(), GatewayClientError> { - if shared_key.is_none() && self.shared_key.is_none() { + async fn upgrade_key_authenticated(&mut self) -> Result<(), GatewayClientError> { + let Some(shared_key) = self.shared_key.as_ref() else { return Err(GatewayClientError::NoSharedKeyAvailable); - } + }; + + assert!(shared_key.is_legacy()); + let legacy_key = shared_key.unwrap_legacy(); + + unimplemented!() + } + + async fn authenticate(&mut self) -> Result<(), GatewayClientError> { + let Some(shared_key) = self.shared_key.as_ref() else { + return Err(GatewayClientError::NoSharedKeyAvailable); + }; + if !self.connection.is_established() { return Err(GatewayClientError::ConnectionNotEstablished); } - log::debug!("Authenticating with gateway"); + debug!("authenticating with gateway"); - todo!("if using legacy key, check if we can upgrade"); - - // it's fine to instantiate it here as it's only used once (during authentication or registration) - // and putting it into the GatewayClient struct would be a hassle - let mut rng = OsRng; - - // because of the previous check one of the unwraps MUST succeed - let shared_key = shared_key - .as_ref() - .unwrap_or_else(|| self.shared_key.as_ref().unwrap()); - let iv = IV::new_random(&mut rng); let self_address = self .local_identity .as_ref() .public_key() .derive_destination_address(); - let encrypted_address = EncryptedAddressBytes::new(&self_address, shared_key, &iv); let msg = ClientControlRequest::new_authenticate( self_address, - encrypted_address, - iv, + shared_key, self.cfg.bandwidth.require_tickets, - ); + )?; match self.send_websocket_message(msg).await? { ServerResponse::Authenticate { @@ -511,6 +501,7 @@ impl GatewayClient { self.negotiated_protocol = protocol_version; log::debug!("authenticated: {status}, bandwidth remaining: {bandwidth_remaining}"); + self.task_client.send_status_msg(Box::new( BandwidthStatusMessage::RemainingBandwidth(bandwidth_remaining), )); @@ -530,7 +521,7 @@ impl GatewayClient { )] pub async fn perform_initial_authentication( &mut self, - ) -> Result, GatewayClientError> { + ) -> Result, GatewayClientError> { // 1. check gateway's protocol version let supports_aes_gcm_siv = match self.get_gateway_protocol().await { Ok(protocol) => protocol >= AES_GCM_SIV_PROTOCOL_VERSION, @@ -546,11 +537,6 @@ impl GatewayClient { warn!("this gateway is on an old version that doesn't support AES256-GCM-SIV"); } - // 2. if error or new handshake unsupported => fallback to the old registration/authentication - // 3. otherwise continue with the updated key derivation - - // ?. if new protocol is supported and we have an old key, upgrade it to aes-gcm-siv - if self.authenticated { debug!("Already authenticated"); return if let Some(shared_key) = &self.shared_key { @@ -561,10 +547,25 @@ impl GatewayClient { } if self.shared_key.is_some() { - self.authenticate(None, supports_aes_gcm_siv).await?; + self.authenticate().await?; + + if self.authenticated { + // if we managed to authenticate with an old key, see if we can upgrade it + if self + .shared_key + .as_ref() + .map(|k| k.is_legacy()) + .unwrap_or_default() + && supports_aes_gcm_siv + { + info!("using a legacy shared key and the gateway supports the updated format - attempting to upgrade"); + self.upgrade_key_authenticated().await?; + } + } } else { self.register(supports_aes_gcm_siv).await?; } + if self.authenticated { // if we are authenticated it means we MUST have an associated shared_key Ok(Arc::clone(self.shared_key.as_ref().unwrap())) @@ -592,14 +593,10 @@ impl GatewayClient { &mut self, credential: CredentialSpendingData, ) -> Result<(), GatewayClientError> { - let mut rng = OsRng; - let iv = IV::new_random(&mut rng); - let msg = ClientControlRequest::new_enc_ecash_credential( credential, self.shared_key.as_ref().unwrap(), - iv, - ); + )?; let bandwidth_remaining = match self.send_websocket_message(msg).await? { ServerResponse::Bandwidth { available_total } => Ok(available_total), ServerResponse::Error { message } => Err(GatewayClientError::GatewayError(message)), @@ -720,10 +717,10 @@ impl GatewayClient { return Err(GatewayClientError::ConnectionNotEstablished); } - let messages: Vec<_> = packets + let messages: Result, _> = packets .into_iter() .map(|mix_packet| { - BinaryRequest::new_forward_request(mix_packet).into_ws_message( + BinaryRequest::ForwardSphinx { packet: mix_packet }.into_ws_message( self.shared_key .as_ref() .expect("no shared key present even though we're authenticated!"), @@ -732,7 +729,7 @@ impl GatewayClient { .collect(); if let Err(err) = self - .batch_send_websocket_messages_without_response(messages) + .batch_send_websocket_messages_without_response(messages?) .await { if err.is_closed_connection() && self.cfg.connection.should_reconnect_on_failure { @@ -796,11 +793,11 @@ impl GatewayClient { } // note: into_ws_message encrypts the requests and adds a MAC on it. Perhaps it should // be more explicit in the naming? - let msg = BinaryRequest::new_forward_request(mix_packet).into_ws_message( + let msg = BinaryRequest::ForwardSphinx { packet: mix_packet }.into_ws_message( self.shared_key .as_ref() .expect("no shared key present even though we're authenticated!"), - ); + )?; self.send_with_reconnection_on_failure(msg).await } @@ -877,7 +874,7 @@ impl GatewayClient { pub async fn authenticate_and_start( &mut self, - ) -> Result, GatewayClientError> + ) -> Result, GatewayClientError> where C: DkgQueryClient + Send + Sync, St: CredentialStorage, diff --git a/common/client-libs/gateway-client/src/error.rs b/common/client-libs/gateway-client/src/error.rs index 0e7757a46c..d796b6df71 100644 --- a/common/client-libs/gateway-client/src/error.rs +++ b/common/client-libs/gateway-client/src/error.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use nym_gateway_requests::registration::handshake::error::HandshakeError; -use nym_gateway_requests::SimpleGatewayRequestsError; +use nym_gateway_requests::{GatewayRequestsError, SimpleGatewayRequestsError}; use std::io; use thiserror::Error; use tungstenite::Error as WsError; @@ -21,6 +21,9 @@ pub enum GatewayClientError { #[error("gateway returned an error response: {0}")] TypedGatewayError(SimpleGatewayRequestsError), + #[error("request error: {0}")] + RequestError(#[from] GatewayRequestsError), + #[error("There was a network error: {0}")] NetworkError(#[from] WsError), diff --git a/common/client-libs/gateway-client/src/lib.rs b/common/client-libs/gateway-client/src/lib.rs index 319c98b23f..57ac4f2714 100644 --- a/common/client-libs/gateway-client/src/lib.rs +++ b/common/client-libs/gateway-client/src/lib.rs @@ -8,6 +8,7 @@ use tungstenite::{protocol::Message, Error as WsError}; pub use client::{config::GatewayClientConfig, GatewayClient, GatewayConfig}; pub use nym_gateway_requests::registration::handshake::LegacySharedKeys; +use nym_gateway_requests::registration::handshake::SharedGatewayKey; pub use packet_router::{ AcknowledgementReceiver, AcknowledgementSender, MixnetMessageReceiver, MixnetMessageSender, PacketRouter, @@ -45,11 +46,11 @@ pub(crate) fn cleanup_socket_messages( pub(crate) fn try_decrypt_binary_message( bin_msg: Vec, - shared_keys: &LegacySharedKeys, + shared_keys: &SharedGatewayKey, ) -> Option> { match BinaryResponse::try_from_encrypted_tagged_bytes(bin_msg, shared_keys) { Ok(bin_response) => match bin_response { - BinaryResponse::PushedMixMessage(plaintext) => Some(plaintext), + BinaryResponse::PushedMixMessage { message } => Some(message), }, Err(err) => { warn!("message received from the gateway was malformed! - {err}",); diff --git a/common/client-libs/gateway-client/src/socket_state.rs b/common/client-libs/gateway-client/src/socket_state.rs index 46d81cb895..b32f85d3f3 100644 --- a/common/client-libs/gateway-client/src/socket_state.rs +++ b/common/client-libs/gateway-client/src/socket_state.rs @@ -9,7 +9,7 @@ use crate::{cleanup_socket_messages, try_decrypt_binary_message}; use futures::channel::oneshot; use futures::stream::{SplitSink, SplitStream}; use futures::{SinkExt, StreamExt}; -use nym_gateway_requests::registration::handshake::LegacySharedKeys; +use nym_gateway_requests::registration::handshake::SharedGatewayKey; use nym_gateway_requests::{ServerResponse, SimpleGatewayRequestsError}; use nym_task::TaskClient; use si_scale::helpers::bibytes2; @@ -62,7 +62,7 @@ pub(crate) struct PartiallyDelegatedHandle { struct PartiallyDelegatedRouter { packet_router: PacketRouter, - shared_key: Arc, + shared_key: Arc, client_bandwidth: ClientBandwidth, stream_return: SplitStreamSender, @@ -72,7 +72,7 @@ struct PartiallyDelegatedRouter { impl PartiallyDelegatedRouter { fn new( packet_router: PacketRouter, - shared_key: Arc, + shared_key: Arc, client_bandwidth: ClientBandwidth, stream_return: SplitStreamSender, stream_return_requester: oneshot::Receiver<()>, @@ -247,7 +247,7 @@ impl PartiallyDelegatedHandle { pub(crate) fn split_and_listen_for_mixnet_messages( conn: WsConn, packet_router: PacketRouter, - shared_key: Arc, + shared_key: Arc, client_bandwidth: ClientBandwidth, shutdown: TaskClient, ) -> Self { diff --git a/common/gateway-requests/Cargo.toml b/common/gateway-requests/Cargo.toml index 3517df78b2..2b8a106794 100644 --- a/common/gateway-requests/Cargo.toml +++ b/common/gateway-requests/Cargo.toml @@ -17,6 +17,7 @@ generic-array = { workspace = true, features = ["serde"] } rand = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +strum = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true, features = ["log"] } zeroize = { workspace = true } diff --git a/common/gateway-requests/src/authentication/encrypted_address.rs b/common/gateway-requests/src/authentication/encrypted_address.rs index f061f170a4..f4a2f3e3a8 100644 --- a/common/gateway-requests/src/authentication/encrypted_address.rs +++ b/common/gateway-requests/src/authentication/encrypted_address.rs @@ -1,60 +1,47 @@ -// Copyright 2020 - Nym Technologies SA +// Copyright 2020-2024 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use crate::iv::IV; -use crate::registration::handshake::LegacySharedKeys; -use nym_crypto::symmetric::stream_cipher; -use nym_sphinx::params::LegacyGatewayEncryptionAlgorithm; -use nym_sphinx::{DestinationAddressBytes, DESTINATION_ADDRESS_LENGTH}; +use crate::registration::handshake::{SharedGatewayKey, SharedKeyUsageError}; +use nym_sphinx::DestinationAddressBytes; use thiserror::Error; -pub const ENCRYPTED_ADDRESS_SIZE: usize = DESTINATION_ADDRESS_LENGTH; - /// Replacement for what used to be an `AuthToken`. /// /// Replacement for what used to be an `AuthToken`. We used to be generating an `AuthToken` based on /// local secret and remote address in order to allow for authentication. Due to changes in registration /// and the fact we are deriving a shared key, we are encrypting remote's address with the previously /// derived shared key. If the value is as expected, then authentication is successful. -#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)] -pub struct EncryptedAddressBytes([u8; ENCRYPTED_ADDRESS_SIZE]); +#[derive(Debug, PartialEq, Eq, Hash, Clone)] +// this is no longer constant size due to the differences in ciphertext between aes128ctr and aes256gcm-siv (inclusion of tag) +pub struct EncryptedAddressBytes(Vec); #[derive(Debug, Error)] pub enum EncryptedAddressConversionError { #[error("Failed to decode the encrypted address - {0}")] DecodeError(#[from] bs58::decode::Error), - #[error("The decoded address has invalid length")] - StringOfInvalidLengthError, } impl EncryptedAddressBytes { - pub fn new(address: &DestinationAddressBytes, key: &LegacySharedKeys, iv: &IV) -> Self { - let ciphertext = stream_cipher::encrypt::( - key.encryption_key(), - iv.inner(), - address.as_bytes_ref(), - ); + pub fn new( + address: &DestinationAddressBytes, + key: &SharedGatewayKey, + nonce: &[u8], + ) -> Result { + let ciphertext = key.encrypt_naive(address.as_bytes_ref(), Some(nonce))?; - let mut enc_address = [0u8; ENCRYPTED_ADDRESS_SIZE]; - enc_address.copy_from_slice(&ciphertext[..]); - EncryptedAddressBytes(enc_address) + Ok(EncryptedAddressBytes(ciphertext)) } pub fn verify( &self, address: &DestinationAddressBytes, - key: &LegacySharedKeys, - iv: &IV, + key: &SharedGatewayKey, + nonce: &[u8], ) -> bool { - self == &Self::new(address, key, iv) - } - - pub fn from_bytes(bytes: [u8; ENCRYPTED_ADDRESS_SIZE]) -> Self { - EncryptedAddressBytes(bytes) - } - - pub fn to_bytes(self) -> [u8; ENCRYPTED_ADDRESS_SIZE] { - self.0 + let Ok(reconstructed) = Self::new(address, key, nonce) else { + return false; + }; + self == &reconstructed } pub fn as_bytes(&self) -> &[u8] { @@ -65,14 +52,7 @@ impl EncryptedAddressBytes { val: S, ) -> Result { let decoded = bs58::decode(val.into()).into_vec()?; - - if decoded.len() != ENCRYPTED_ADDRESS_SIZE { - return Err(EncryptedAddressConversionError::StringOfInvalidLengthError); - } - - let mut enc_address = [0u8; ENCRYPTED_ADDRESS_SIZE]; - enc_address.copy_from_slice(&decoded[..]); - Ok(EncryptedAddressBytes(enc_address)) + Ok(EncryptedAddressBytes(decoded)) } pub fn to_base58_string(self) -> String { diff --git a/common/gateway-requests/src/iv.rs b/common/gateway-requests/src/iv.rs deleted file mode 100644 index 5db5b320bf..0000000000 --- a/common/gateway-requests/src/iv.rs +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright 2020 - Nym Technologies SA -// SPDX-License-Identifier: Apache-2.0 - -use nym_crypto::generic_array::{typenum::Unsigned, GenericArray}; -use nym_crypto::symmetric::stream_cipher::{random_iv, IvSizeUser, IV as CryptoIV}; -use nym_sphinx::params::LegacyGatewayEncryptionAlgorithm; -use rand::{CryptoRng, RngCore}; -use thiserror::Error; - -type NonceSize = ::IvSize; - -// I think 'IV' looks better than 'Iv', feel free to change that. -#[allow(clippy::upper_case_acronyms)] -pub struct IV(CryptoIV); - -#[derive(Error, Debug)] -// I think 'IV' looks better than 'Iv', feel free to change that. -#[allow(clippy::upper_case_acronyms)] -pub enum IVConversionError { - #[error("Failed to decode the iv - {0}")] - DecodeError(#[from] bs58::decode::Error), - - #[error("The decoded bytes iv has invalid length")] - BytesOfInvalidLengthError, - - #[error("The decoded string iv has invalid length")] - StringOfInvalidLengthError, -} - -impl IV { - pub fn new_random(rng: &mut R) -> Self { - IV(random_iv::(rng)) - } - - pub fn try_from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != NonceSize::to_usize() { - return Err(IVConversionError::BytesOfInvalidLengthError); - } - - Ok(IV(GenericArray::clone_from_slice(bytes))) - } - - pub fn to_bytes(&self) -> Vec { - self.0.to_vec() - } - - pub fn as_bytes(&self) -> &[u8] { - self.0.as_ref() - } - - pub fn inner(&self) -> &CryptoIV { - &self.0 - } - - pub fn try_from_base58_string>(val: S) -> Result { - let decoded = bs58::decode(val.into()).into_vec()?; - - if decoded.len() != NonceSize::to_usize() { - return Err(IVConversionError::StringOfInvalidLengthError); - } - - Ok(IV( - GenericArray::from_exact_iter(decoded).expect("Invalid vector length!") - )) - } - - pub fn to_base58_string(&self) -> String { - bs58::encode(self.to_bytes()).into_string() - } -} - -impl From for String { - fn from(iv: IV) -> Self { - iv.to_base58_string() - } -} diff --git a/common/gateway-requests/src/lib.rs b/common/gateway-requests/src/lib.rs index 47e30e2758..4b83be8c10 100644 --- a/common/gateway-requests/src/lib.rs +++ b/common/gateway-requests/src/lib.rs @@ -2,13 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 pub use nym_crypto::generic_array; -use nym_crypto::hmac::HmacOutput; use nym_crypto::OutputSizeUser; use nym_sphinx::params::GatewayIntegrityHmacAlgorithm; + pub use types::*; pub mod authentication; -pub mod iv; pub mod models; pub mod registration; pub mod types; @@ -25,8 +24,6 @@ pub const INITIAL_PROTOCOL_VERSION: u8 = 1; pub const CREDENTIAL_UPDATE_V2_PROTOCOL_VERSION: u8 = 2; pub const AES_GCM_SIV_PROTOCOL_VERSION: u8 = 3; -pub type GatewayMac = HmacOutput; - // TODO: could using `Mac` trait here for OutputSize backfire? // Should hmac itself be exposed, imported and used instead? -pub type GatewayMacSize = ::OutputSize; +pub type LegacyGatewayMacSize = ::OutputSize; diff --git a/common/gateway-requests/src/registration/handshake/client.rs b/common/gateway-requests/src/registration/handshake/client.rs index 0acb6bff43..aebda07551 100644 --- a/common/gateway-requests/src/registration/handshake/client.rs +++ b/common/gateway-requests/src/registration/handshake/client.rs @@ -6,7 +6,6 @@ use crate::registration::handshake::state::State; use crate::registration::handshake::SharedGatewayKey; use crate::registration::handshake::{error::HandshakeError, WsItem}; use futures::{Sink, Stream}; -use tracing::info; use tungstenite::Message as WsMessage; impl<'a, S> State<'a, S> { diff --git a/common/gateway-requests/src/registration/handshake/shared_key/legacy.rs b/common/gateway-requests/src/registration/handshake/shared_key/legacy.rs index 7b60a737b1..3d6bb905f0 100644 --- a/common/gateway-requests/src/registration/handshake/shared_key/legacy.rs +++ b/common/gateway-requests/src/registration/handshake/shared_key/legacy.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::registration::handshake::shared_key::{SharedKeyConversionError, SharedKeyUsageError}; -use crate::GatewayMacSize; +use crate::LegacyGatewayMacSize; use nym_crypto::generic_array::{ typenum::{Sum, Unsigned, U16}, GenericArray, @@ -110,7 +110,7 @@ impl LegacySharedKeys { enc_data: &[u8], iv: Option<&IV>, ) -> Result, SharedKeyUsageError> { - let mac_size = GatewayMacSize::to_usize(); + let mac_size = LegacyGatewayMacSize::to_usize(); if enc_data.len() < mac_size { return Err(SharedKeyUsageError::TooShortRequest); } @@ -128,16 +128,16 @@ impl LegacySharedKeys { // couldn't have made the first borrow mutable as you can't have an immutable borrow // together with a mutable one - let message_bytes_mut = &mut enc_data.to_vec()[mac_size..]; + let mut message_bytes_mut = message_bytes.to_vec(); let zero_iv = stream_cipher::zero_iv::(); let iv = iv.unwrap_or(&zero_iv); stream_cipher::decrypt_in_place::( self.encryption_key(), iv, - message_bytes_mut, + &mut message_bytes_mut, ); - Ok(message_bytes_mut.to_vec()) + Ok(message_bytes_mut) } pub fn encryption_key(&self) -> &CipherKey { diff --git a/common/gateway-requests/src/registration/handshake/shared_key/mod.rs b/common/gateway-requests/src/registration/handshake/shared_key/mod.rs index b199e09752..4fee47b6cc 100644 --- a/common/gateway-requests/src/registration/handshake/shared_key/mod.rs +++ b/common/gateway-requests/src/registration/handshake/shared_key/mod.rs @@ -3,10 +3,13 @@ use crate::registration::handshake::LegacySharedKeys; use nym_crypto::generic_array::{typenum::Unsigned, GenericArray}; -use nym_crypto::symmetric::aead::{self, nonce_size, AeadError, AeadKey, KeySizeUser, Nonce}; -use nym_crypto::symmetric::stream_cipher::{iv_size, IV}; +use nym_crypto::symmetric::aead::{ + self, nonce_size, random_nonce, AeadError, AeadKey, KeySizeUser, Nonce, +}; +use nym_crypto::symmetric::stream_cipher::{iv_size, random_iv, IV}; use nym_pemstore::traits::PemStorableKey; use nym_sphinx::params::{GatewayEncryptionAlgorithm, LegacyGatewayEncryptionAlgorithm}; +use rand::thread_rng; use serde::{Deserialize, Serialize}; use thiserror::Error; use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; @@ -15,12 +18,77 @@ pub mod legacy; pub type SharedKeySize = ::KeySize; -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Zeroize)] pub enum SharedGatewayKey { Current(SharedSymmetricKey), Legacy(LegacySharedKeys), } +impl SharedGatewayKey { + pub fn is_legacy(&self) -> bool { + matches!(self, SharedGatewayKey::Legacy(..)) + } + + pub fn aes128_ctr_hmac_bs58(&self) -> Option> { + match self { + SharedGatewayKey::Current(_) => None, + SharedGatewayKey::Legacy(key) => Some(Zeroizing::new(key.to_base58_string())), + } + } + + pub fn aes256_gcm_siv(&self) -> Option>> { + match self { + SharedGatewayKey::Current(key) => Some(Zeroizing::new(key.to_bytes())), + SharedGatewayKey::Legacy(_) => None, + } + } + + pub fn unwrap_legacy(&self) -> &LegacySharedKeys { + match self { + SharedGatewayKey::Current(_) => panic!("expected legacy key"), + SharedGatewayKey::Legacy(key) => key, + } + } + + pub fn random_nonce_or_iv(&self) -> Vec { + let mut rng = thread_rng(); + + if self.is_legacy() { + random_iv::(&mut rng).to_vec() + } else { + random_nonce::(&mut rng).to_vec() + } + } + + pub fn random_nonce_or_zero_iv(&self) -> Option> { + if self.is_legacy() { + None + } else { + let mut rng = thread_rng(); + Some(random_nonce::(&mut rng).to_vec()) + } + } + + pub fn nonce_size(&self) -> usize { + match self { + SharedGatewayKey::Current(_) => nonce_size::(), + SharedGatewayKey::Legacy(_) => iv_size::(), + } + } +} + +impl From for SharedGatewayKey { + fn from(keys: LegacySharedKeys) -> Self { + SharedGatewayKey::Legacy(keys) + } +} + +impl From for SharedGatewayKey { + fn from(keys: SharedSymmetricKey) -> Self { + SharedGatewayKey::Current(keys) + } +} + #[derive(Debug, Error)] pub enum SharedKeyUsageError { #[error("the request is too short")] diff --git a/common/gateway-requests/src/registration/handshake/state.rs b/common/gateway-requests/src/registration/handshake/state.rs index 0522ae6631..c1b5e1056a 100644 --- a/common/gateway-requests/src/registration/handshake/state.rs +++ b/common/gateway-requests/src/registration/handshake/state.rs @@ -170,8 +170,6 @@ impl<'a, S> State<'a, S> { .collect(); let signature = self.identity.private_key().sign(plaintext); - println!("signature len: {}", signature.to_bytes().len()); - let nonce = if self.derive_aes256_gcm_siv_key { let mut rng = thread_rng(); Some(random_nonce::(&mut rng).to_vec()) @@ -179,8 +177,6 @@ impl<'a, S> State<'a, S> { None }; - println!("key types: {:?}", self.derived_shared_keys); - // SAFETY: this function is only called after the local key has already been derived let signature_ciphertext = self .derived_shared_keys diff --git a/common/gateway-requests/src/types.rs b/common/gateway-requests/src/types.rs index 82dab71853..b223502b39 100644 --- a/common/gateway-requests/src/types.rs +++ b/common/gateway-requests/src/types.rs @@ -1,24 +1,22 @@ // Copyright 2020-2024 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use crate::authentication::encrypted_address::EncryptedAddressBytes; -use crate::iv::{IVConversionError, IV}; use crate::models::CredentialSpendingRequest; -use crate::registration::handshake::{LegacySharedKeys, SharedKeyUsageError}; -use crate::{GatewayMacSize, CURRENT_PROTOCOL_VERSION, INITIAL_PROTOCOL_VERSION}; +use crate::registration::handshake::{SharedGatewayKey, SharedKeyUsageError}; +use crate::{ + AES_GCM_SIV_PROTOCOL_VERSION, CREDENTIAL_UPDATE_V2_PROTOCOL_VERSION, INITIAL_PROTOCOL_VERSION, +}; use nym_credentials::ecash::bandwidth::CredentialSpendingData; use nym_credentials_interface::CompactEcashError; -use nym_crypto::generic_array::typenum::Unsigned; -use nym_crypto::hmac::recompute_keyed_hmac_and_verify_tag; -use nym_crypto::symmetric::stream_cipher; use nym_sphinx::addressing::nodes::NymNodeRoutingAddressError; use nym_sphinx::forwarding::packet::{MixPacket, MixPacketFormattingError}; use nym_sphinx::params::packet_sizes::PacketSize; -use nym_sphinx::params::{GatewayIntegrityHmacAlgorithm, LegacyGatewayEncryptionAlgorithm}; use nym_sphinx::DestinationAddressBytes; use serde::{Deserialize, Serialize}; +use std::iter::once; use std::str::FromStr; use std::string::FromUtf8Error; +use strum::FromRepr; use thiserror::Error; use tracing::log::error; use tungstenite::protocol::Message; @@ -97,15 +95,21 @@ pub enum GatewayRequestsError { #[error(transparent)] KeyUsageFailure(#[from] SharedKeyUsageError), + #[error("received request with an unknown kind: {kind}")] + UnknownRequestKind { kind: u8 }, + + #[error("received response with an unknown kind: {kind}")] + UnknownResponseKind { kind: u8 }, + + #[error("the encryption flag had an unexpected value")] + InvalidEncryptionFlag, + #[error("the request is too short")] TooShortRequest, #[error("provided MAC is invalid")] InvalidMac, - #[error("Provided bandwidth IV is malformed: {0}")] - MalformedIV(#[from] IVConversionError), - #[error("address field was incorrectly encoded: {source}")] IncorrectlyEncodedAddress { #[from] @@ -124,18 +128,15 @@ pub enum GatewayRequestsError { #[error("received sphinx packet was malformed")] MalformedSphinxPacket, + #[error("failed to serialise created sphinx packet: {0}")] + SphinxSerialisationFailure(#[from] MixPacketFormattingError), + #[error("the received encrypted data was malformed")] MalformedEncryption, #[error("provided packet mode is invalid")] InvalidPacketMode, - #[error("provided mix packet was malformed: {source}")] - InvalidMixPacket { - #[from] - source: MixPacketFormattingError, - }, - #[error("failed to deserialize provided credential: {0}")] EcashCredentialDeserializationFailure(#[from] CompactEcashError), @@ -190,24 +191,29 @@ pub enum ClientControlRequest { impl ClientControlRequest { pub fn new_authenticate( address: DestinationAddressBytes, - enc_address: EncryptedAddressBytes, - iv: IV, + shared_key: &SharedGatewayKey, uses_credentials: bool, - ) -> Self { - // if we're not going to be using credentials, advertise lower protocol version to allow connection - // to wider range of gateways - let protocol_version = if uses_credentials { - Some(CURRENT_PROTOCOL_VERSION) + ) -> Result { + // if we're encrypting with non-legacy key, the remote must support AES256-GCM-SIV + let protocol_version = if !shared_key.is_legacy() { + Some(AES_GCM_SIV_PROTOCOL_VERSION) + } else if uses_credentials { + Some(CREDENTIAL_UPDATE_V2_PROTOCOL_VERSION) } else { + // if we're not going to be using credentials, advertise lower protocol version to allow connection + // to wider range of gateways Some(INITIAL_PROTOCOL_VERSION) }; - ClientControlRequest::Authenticate { + let nonce = shared_key.random_nonce_or_iv(); + let ciphertext = shared_key.encrypt_naive(address.as_bytes_ref(), Some(&nonce))?; + + Ok(ClientControlRequest::Authenticate { protocol_version, address: address.as_base58_string(), - enc_address: enc_address.to_base58_string(), - iv: iv.to_base58_string(), - } + enc_address: bs58::encode(&ciphertext).into_string(), + iv: bs58::encode(&nonce).into_string(), + }) } pub fn name(&self) -> String { @@ -230,26 +236,26 @@ impl ClientControlRequest { pub fn new_enc_ecash_credential( credential: CredentialSpendingData, - shared_key: &LegacySharedKeys, - iv: IV, - ) -> Self { + shared_key: &SharedGatewayKey, + ) -> Result { let cred = CredentialSpendingRequest::new(credential); let serialized_credential = cred.to_bytes(); - let enc_credential = shared_key.encrypt_and_tag(&serialized_credential, Some(iv.inner())); - ClientControlRequest::EcashCredential { + let nonce = shared_key.random_nonce_or_iv(); + let enc_credential = shared_key.encrypt(&serialized_credential, Some(&nonce))?; + + Ok(ClientControlRequest::EcashCredential { enc_credential, - iv: iv.to_bytes(), - } + iv: nonce, + }) } pub fn try_from_enc_ecash_credential( enc_credential: Vec, - shared_key: &LegacySharedKeys, + shared_key: &SharedGatewayKey, iv: Vec, ) -> Result { - let iv = IV::try_from_bytes(&iv)?; - let credential_bytes = shared_key.decrypt_tagged(&enc_credential, Some(iv.inner()))?; + let credential_bytes = shared_key.decrypt(&enc_credential, Some(&iv))?; CredentialSpendingRequest::try_from_bytes(credential_bytes.as_slice()) .map_err(|_| GatewayRequestsError::MalformedEncryption) } @@ -370,8 +376,141 @@ impl TryFrom for ServerResponse { } } +// each binary message consists of the following structure (for non-legacy messages) +// KIND || ENC_FLAG || MAYBE_NONCE || CIPHERTEXT/PLAINTEXT +// first byte is the kind of data to influence further serialisation/deseralisation +// second byte is a flag indicating whether the content is encrypted +// then it's followed by a pseudorandom nonce, assuming encryption is used +// finally, the rest of the message is the associated ciphertext or just plaintext (if message wasn't encrypted) +pub struct BinaryData<'a> { + kind: u8, + encrypted: bool, + maybe_nonce: Option<&'a [u8]>, + data: &'a [u8], +} + +impl<'a> BinaryData<'a> { + // serialises possibly encrypted data into bytes to be put on the wire + pub fn into_raw(self, legacy: bool) -> Vec { + if legacy { + return self.data.to_vec(); + } + + let i = once(self.kind).chain(once(if self.encrypted { 1 } else { 0 })); + if let Some(nonce) = self.maybe_nonce { + i.chain(nonce.iter().copied()) + .chain(self.data.iter().copied()) + .collect() + } else { + i.chain(self.data.iter().copied()).collect() + } + } + + // attempts to perform basic parsing on bytes received from the wire + pub fn from_raw( + raw: &'a [u8], + available_key: &SharedGatewayKey, + ) -> Result { + // if we're using legacy key, it's quite simple: + // it's always encrypted with no nonce and the request/response kind is always 1 + if available_key.is_legacy() { + return Ok(BinaryData { + kind: 1, + encrypted: true, + maybe_nonce: None, + data: raw, + }); + } + + if raw.len() < 2 { + return Err(GatewayRequestsError::TooShortRequest); + } + + let kind = raw[0]; + let encrypted = if raw[1] == 1 { + true + } else if raw[1] == 0 { + false + } else { + return Err(GatewayRequestsError::InvalidEncryptionFlag); + }; + + // if data is encrypted, there MUST be a nonce present for non-legacy keys + if encrypted && raw.len() < available_key.nonce_size() + 2 { + return Err(GatewayRequestsError::TooShortRequest); + } + + Ok(BinaryData { + kind, + encrypted, + maybe_nonce: Some(&raw[2..2 + available_key.nonce_size()]), + data: &raw[2 + available_key.nonce_size()..], + }) + } + + // attempt to encrypt plaintext of provided response/request and serialise it into wire format + pub fn make_encrypted_blob( + kind: u8, + plaintext: &[u8], + key: &SharedGatewayKey, + ) -> Result, GatewayRequestsError> { + let maybe_nonce = key.random_nonce_or_zero_iv(); + + let ciphertext = key.encrypt(plaintext, maybe_nonce.as_deref())?; + Ok(BinaryData { + kind, + encrypted: true, + maybe_nonce: maybe_nonce.as_deref(), + data: &ciphertext, + } + .into_raw(key.is_legacy())) + } + + // attempts to parse previously recovered bytes into a [`BinaryRequest`] + pub fn into_request( + self, + key: &SharedGatewayKey, + ) -> Result { + let kind = BinaryRequestKind::from_repr(self.kind) + .ok_or(GatewayRequestsError::UnknownRequestKind { kind: self.kind })?; + + let plaintext = if self.encrypted { + &*key.decrypt(self.data, self.maybe_nonce)? + } else { + self.data + }; + + BinaryRequest::from_plaintext(kind, plaintext) + } + + // attempts to parse previously recovered bytes into a [`BinaryResponse`] + pub fn into_response( + self, + key: &SharedGatewayKey, + ) -> Result { + let kind = BinaryResponseKind::from_repr(self.kind) + .ok_or(GatewayRequestsError::UnknownResponseKind { kind: self.kind })?; + + let plaintext = if self.encrypted { + &*key.decrypt(self.data, self.maybe_nonce)? + } else { + self.data + }; + + BinaryResponse::from_plaintext(kind, plaintext) + } +} + +// in legacy mode requests use zero IV without pub enum BinaryRequest { - ForwardSphinx(MixPacket), + ForwardSphinx { packet: MixPacket }, +} + +#[repr(u8)] +#[derive(Debug, Clone, Copy, FromRepr, PartialEq)] +#[non_exhaustive] +pub enum BinaryRequestKind { + ForwardSphinx = 1, } // Right now the only valid `BinaryRequest` is a request to forward a sphinx packet. @@ -380,96 +519,118 @@ pub enum BinaryRequest { // HOWEVER, NOTE: If we introduced another 'BinaryRequest', we must carefully examine if a 0s IV // would work there. impl BinaryRequest { - pub fn try_from_encrypted_tagged_bytes( - raw_req: Vec, - shared_keys: &LegacySharedKeys, - ) -> Result { - let message_bytes = &shared_keys.decrypt_tagged(&raw_req, None)?; - - // right now there's only a single option possible which significantly simplifies the logic - // if we decided to allow for more 'binary' messages, the API wouldn't need to change. - let mix_packet = MixPacket::try_from_bytes(message_bytes)?; - Ok(BinaryRequest::ForwardSphinx(mix_packet)) + pub fn kind(&self) -> BinaryRequestKind { + match self { + BinaryRequest::ForwardSphinx { .. } => BinaryRequestKind::ForwardSphinx, + } } - pub fn into_encrypted_tagged_bytes(self, shared_key: &LegacySharedKeys) -> Vec { - match self { - BinaryRequest::ForwardSphinx(mix_packet) => { - let forwarding_data = match mix_packet.into_bytes() { - Ok(mix_packet) => mix_packet, - Err(e) => { - error!("Could not convert packet to bytes: {e}"); - return vec![]; - } - }; - - // TODO: it could be theoretically slightly more efficient if the data wasn't taken - // by reference because then it makes a copy for encryption rather than do it in place - shared_key.encrypt_and_tag(&forwarding_data, None) + pub fn from_plaintext( + kind: BinaryRequestKind, + plaintext: &[u8], + ) -> Result { + match kind { + BinaryRequestKind::ForwardSphinx => { + let packet = MixPacket::try_from_bytes(plaintext)?; + Ok(BinaryRequest::ForwardSphinx { packet }) } } } - // TODO: this will be encrypted, etc. - pub fn new_forward_request(mix_packet: MixPacket) -> BinaryRequest { - BinaryRequest::ForwardSphinx(mix_packet) + pub fn try_from_encrypted_tagged_bytes( + bytes: Vec, + shared_key: &SharedGatewayKey, + ) -> Result { + BinaryData::from_raw(&bytes, shared_key)?.into_request(shared_key) } - pub fn into_ws_message(self, shared_key: &LegacySharedKeys) -> Message { - Message::Binary(self.into_encrypted_tagged_bytes(shared_key)) + pub fn into_encrypted_tagged_bytes( + self, + shared_key: &SharedGatewayKey, + ) -> Result, GatewayRequestsError> { + let kind = self.kind(); + + let plaintext = match self { + BinaryRequest::ForwardSphinx { packet } => packet.into_bytes()?, + }; + + BinaryData::make_encrypted_blob(kind as u8, &plaintext, shared_key) + } + + pub fn into_ws_message( + self, + shared_key: &SharedGatewayKey, + ) -> Result { + // all variants are currently encrypted + let blob = match self { + BinaryRequest::ForwardSphinx { .. } => self.into_encrypted_tagged_bytes(shared_key)?, + }; + + Ok(Message::Binary(blob)) } } -// Introduced for consistency sake pub enum BinaryResponse { - PushedMixMessage(Vec), + PushedMixMessage { message: Vec }, +} + +#[repr(u8)] +#[derive(Debug, Clone, Copy, FromRepr, PartialEq)] +#[non_exhaustive] +pub enum BinaryResponseKind { + PushedMixMessage = 1, } impl BinaryResponse { - pub fn try_from_encrypted_tagged_bytes( - raw_req: Vec, - shared_keys: &LegacySharedKeys, - ) -> Result { - let mac_size = GatewayMacSize::to_usize(); - if raw_req.len() < mac_size { - return Err(GatewayRequestsError::TooShortRequest); - } - - let mac_tag = &raw_req[..mac_size]; - let message_bytes = &raw_req[mac_size..]; - - if !recompute_keyed_hmac_and_verify_tag::( - shared_keys.mac_key().as_slice(), - message_bytes, - mac_tag, - ) { - return Err(GatewayRequestsError::InvalidMac); - } - - let zero_iv = stream_cipher::zero_iv::(); - let plaintext = stream_cipher::decrypt::( - shared_keys.encryption_key(), - &zero_iv, - message_bytes, - ); - - Ok(BinaryResponse::PushedMixMessage(plaintext)) - } - - pub fn into_encrypted_tagged_bytes(self, shared_key: &LegacySharedKeys) -> Vec { + pub fn kind(&self) -> BinaryResponseKind { match self { - // TODO: it could be theoretically slightly more efficient if the data wasn't taken - // by reference because then it makes a copy for encryption rather than do it in place - BinaryResponse::PushedMixMessage(message) => shared_key.encrypt_and_tag(&message, None), + BinaryResponse::PushedMixMessage { .. } => BinaryResponseKind::PushedMixMessage, } } - pub fn new_pushed_mix_message(msg: Vec) -> Self { - BinaryResponse::PushedMixMessage(msg) + pub fn from_plaintext( + kind: BinaryResponseKind, + plaintext: &[u8], + ) -> Result { + match kind { + BinaryResponseKind::PushedMixMessage => Ok(BinaryResponse::PushedMixMessage { + message: plaintext.to_vec(), + }), + } } - pub fn into_ws_message(self, shared_key: &LegacySharedKeys) -> Message { - Message::Binary(self.into_encrypted_tagged_bytes(shared_key)) + pub fn try_from_encrypted_tagged_bytes( + bytes: Vec, + shared_key: &SharedGatewayKey, + ) -> Result { + BinaryData::from_raw(&bytes, shared_key)?.into_response(shared_key) + } + + pub fn into_encrypted_tagged_bytes( + self, + shared_key: &SharedGatewayKey, + ) -> Result, GatewayRequestsError> { + let kind = self.kind(); + + let plaintext = match self { + BinaryResponse::PushedMixMessage { message } => message, + }; + + BinaryData::make_encrypted_blob(kind as u8, &plaintext, shared_key) + } + + pub fn into_ws_message( + self, + shared_key: &SharedGatewayKey, + ) -> Result { + // all variants are currently encrypted + let blob = match self { + BinaryResponse::PushedMixMessage { .. } => { + self.into_encrypted_tagged_bytes(shared_key)? + } + }; + + Ok(Message::Binary(blob)) } } diff --git a/common/gateway-storage/Cargo.toml b/common/gateway-storage/Cargo.toml index 26b2be56a1..d89bac7639 100644 --- a/common/gateway-storage/Cargo.toml +++ b/common/gateway-storage/Cargo.toml @@ -19,6 +19,7 @@ sqlx = { workspace = true, features = [ "macros", "migrate", "time", + "chrono" ] } time = { workspace = true } thiserror = { workspace = true } diff --git a/common/gateway-storage/migrations/20240916120000_add_aes256_gcm_siv_key.sql b/common/gateway-storage/migrations/20240916120000_add_aes256_gcm_siv_key.sql new file mode 100644 index 0000000000..6327832639 --- /dev/null +++ b/common/gateway-storage/migrations/20240916120000_add_aes256_gcm_siv_key.sql @@ -0,0 +1,13 @@ +/* + * Copyright 2024 - Nym Technologies SA + * SPDX-License-Identifier: GPL-3.0-only + */ + +-- make aes128 key column nullable and add aes256 column +ALTER TABLE shared_keys RENAME COLUMN derived_aes128_ctr_blake3_hmac_keys_bs58 TO derived_aes128_ctr_blake3_hmac_keys_bs58_old; +ALTER TABLE shared_keys ADD COLUMN derived_aes128_ctr_blake3_hmac_keys_bs58 TEXT; +ALTER TABLE shared_keys ADD COLUMN derived_aes256_gcm_siv_key BLOB; + +UPDATE shared_keys SET derived_aes128_ctr_blake3_hmac_keys_bs58 = derived_aes128_ctr_blake3_hmac_keys_bs58_old; + +ALTER TABLE shared_keys DROP COLUMN derived_aes128_ctr_blake3_hmac_keys_bs58_old; diff --git a/common/gateway-storage/src/error.rs b/common/gateway-storage/src/error.rs index 178d630c10..408ec245d6 100644 --- a/common/gateway-storage/src/error.rs +++ b/common/gateway-storage/src/error.rs @@ -11,6 +11,9 @@ pub enum StorageError { #[error("Failed to perform database migration: {0}")] MigrationError(#[from] sqlx::migrate::MigrateError), + #[error("could not find any valid shared keys for under id {id}")] + MissingSharedKey { id: i64 }, + #[error("Somehow stored data is incorrect: {0}")] DataCorruption(String), diff --git a/common/gateway-storage/src/lib.rs b/common/gateway-storage/src/lib.rs index 24fd0ee7b1..4bae9568aa 100644 --- a/common/gateway-storage/src/lib.rs +++ b/common/gateway-storage/src/lib.rs @@ -11,7 +11,7 @@ use models::{ VerifiedTicket, WireguardPeer, }; use nym_credentials_interface::ClientTicket; -use nym_gateway_requests::registration::handshake::LegacySharedKeys; +use nym_gateway_requests::registration::handshake::SharedGatewayKey; use nym_sphinx::DestinationAddressBytes; use shared_keys::SharedKeysManager; use sqlx::ConnectOptions; @@ -42,11 +42,13 @@ pub trait Storage: Send + Sync { /// # Arguments /// /// * `client_address`: base58-encoded address of the client - /// * `shared_keys`: shared encryption (AES128CTR) and mac (hmac-blake3) derived shared keys to store. + /// * `shared_keys`: + /// - legacy: shared encryption (AES128CTR) and mac (hmac-blake3) derived shared keys to store. + /// - current: shared AES256-GCM-SIV keys async fn insert_shared_keys( &self, client_address: DestinationAddressBytes, - shared_keys: &LegacySharedKeys, + shared_keys: &SharedGatewayKey, ) -> Result; /// Tries to retrieve shared keys stored for the particular client. @@ -330,7 +332,7 @@ impl Storage for PersistentStorage { async fn insert_shared_keys( &self, client_address: DestinationAddressBytes, - shared_keys: &LegacySharedKeys, + shared_keys: &SharedGatewayKey, ) -> Result { let client_id = self .client_manager @@ -340,7 +342,8 @@ impl Storage for PersistentStorage { .insert_shared_keys( client_id, client_address.as_base58_string(), - shared_keys.to_base58_string(), + shared_keys.aes128_ctr_hmac_bs58().as_deref(), + shared_keys.aes256_gcm_siv().as_deref(), ) .await?; Ok(client_id) diff --git a/common/gateway-storage/src/models.rs b/common/gateway-storage/src/models.rs index 09df516e9b..4b6afef89a 100644 --- a/common/gateway-storage/src/models.rs +++ b/common/gateway-storage/src/models.rs @@ -3,6 +3,9 @@ use crate::error::StorageError; use nym_credentials_interface::{AvailableBandwidth, ClientTicket, CredentialSpendingData}; +use nym_gateway_requests::registration::handshake::{ + LegacySharedKeys, SharedGatewayKey, SharedSymmetricKey, +}; use sqlx::FromRow; use time::OffsetDateTime; @@ -11,13 +14,38 @@ pub struct Client { pub client_type: crate::clients::ClientType, } +#[derive(FromRow)] pub struct PersistedSharedKeys { #[allow(dead_code)] pub client_id: i64, #[allow(dead_code)] pub client_address_bs58: String, - pub derived_aes128_ctr_blake3_hmac_keys_bs58: String, + pub derived_aes128_ctr_blake3_hmac_keys_bs58: Option, + pub derived_aes256_gcm_siv_key: Option>, +} + +impl TryFrom for SharedGatewayKey { + type Error = StorageError; + + fn try_from(value: PersistedSharedKeys) -> Result { + match ( + &value.derived_aes256_gcm_siv_key, + &value.derived_aes128_ctr_blake3_hmac_keys_bs58, + ) { + (None, None) => Err(StorageError::MissingSharedKey { id: value.id }), + (Some(aes256gcm_siv), _) => { + let current_key = SharedSymmetricKey::try_from_bytes(aes256gcm_siv) + .map_err(|source| StorageError::DataCorruption(source.to_string()))?; + Ok(SharedGatewayKey::Current(current_key)) + } + (None, Some(aes128ctr_hmac)) => { + let legacy_key = LegacySharedKeys::try_from_base58_string(aes128ctr_hmac) + .map_err(|source| StorageError::DataCorruption(source.to_string()))?; + Ok(SharedGatewayKey::Legacy(legacy_key)) + } + } + } } pub struct StoredMessage { diff --git a/common/gateway-storage/src/shared_keys.rs b/common/gateway-storage/src/shared_keys.rs index 4144dee0b7..ef7fac2eef 100644 --- a/common/gateway-storage/src/shared_keys.rs +++ b/common/gateway-storage/src/shared_keys.rs @@ -41,19 +41,27 @@ impl SharedKeysManager { &self, client_id: i64, client_address_bs58: String, - derived_aes128_ctr_blake3_hmac_keys_bs58: String, + derived_aes128_ctr_blake3_hmac_keys_bs58: Option<&String>, + derived_aes256_gcm_siv_key: Option<&Vec>, ) -> Result<(), sqlx::Error> { // https://stackoverflow.com/a/20310838 // we don't want to be using `INSERT OR REPLACE INTO` due to the foreign key on `available_bandwidth` if the entry already exists sqlx::query!( r#" - INSERT OR IGNORE INTO shared_keys(client_id, client_address_bs58, derived_aes128_ctr_blake3_hmac_keys_bs58) VALUES (?, ?, ?); - UPDATE shared_keys SET derived_aes128_ctr_blake3_hmac_keys_bs58 = ? WHERE client_address_bs58 = ? + INSERT OR IGNORE INTO shared_keys(client_id, client_address_bs58, derived_aes128_ctr_blake3_hmac_keys_bs58, derived_aes256_gcm_siv_key) VALUES (?, ?, ?); + + UPDATE shared_keys + SET + derived_aes128_ctr_blake3_hmac_keys_bs58 = ?, + derived_aes256_gcm_siv_key = ? + WHERE client_address_bs58 = ? "#, client_id, client_address_bs58, derived_aes128_ctr_blake3_hmac_keys_bs58, + derived_aes256_gcm_siv_key, derived_aes128_ctr_blake3_hmac_keys_bs58, + derived_aes256_gcm_siv_key, client_address_bs58, ).execute(&self.connection_pool).await?; diff --git a/gateway/src/node/client_handling/websocket/connection_handler/authenticated.rs b/gateway/src/node/client_handling/websocket/connection_handler/authenticated.rs index 18efea6588..8c6cb2ef29 100644 --- a/gateway/src/node/client_handling/websocket/connection_handler/authenticated.rs +++ b/gateway/src/node/client_handling/websocket/connection_handler/authenticated.rs @@ -286,10 +286,9 @@ where } Ok(request) => match request { // currently only a single type exists - BinaryRequest::ForwardSphinx(mix_packet) => self - .handle_forward_sphinx(mix_packet) - .await - .into_ws_message(), + BinaryRequest::ForwardSphinx { packet } => { + self.handle_forward_sphinx(packet).await.into_ws_message() + } }, } } diff --git a/gateway/src/node/client_handling/websocket/connection_handler/fresh.rs b/gateway/src/node/client_handling/websocket/connection_handler/fresh.rs index 7d2d41fa7d..34a313dd0d 100644 --- a/gateway/src/node/client_handling/websocket/connection_handler/fresh.rs +++ b/gateway/src/node/client_handling/websocket/connection_handler/fresh.rs @@ -21,12 +21,8 @@ use nym_crypto::asymmetric::identity; use nym_gateway_requests::authentication::encrypted_address::{ EncryptedAddressBytes, EncryptedAddressConversionError, }; -use nym_gateway_requests::registration::handshake::SharedGatewayKey; use nym_gateway_requests::{ - iv::{IVConversionError, IV}, - registration::handshake::{ - error::HandshakeError, gateway_handshake, LegacySharedKeys, SharedKeyConversionError, - }, + registration::handshake::{error::HandshakeError, gateway_handshake, SharedGatewayKey}, types::{ClientControlRequest, ServerResponse}, BinaryResponse, CURRENT_PROTOCOL_VERSION, INITIAL_PROTOCOL_VERSION, }; @@ -54,7 +50,7 @@ pub(crate) enum InitialAuthenticationError { MalformedStoredSharedKey { client_id: String, #[source] - source: SharedKeyConversionError, + source: StorageError, }, #[error("Failed to perform registration handshake: {0}")] @@ -70,8 +66,8 @@ pub(crate) enum InitialAuthenticationError { #[error("There is already an open connection to this client")] DuplicateConnection, - #[error("Provided authentication IV is malformed: {0}")] - MalformedIV(#[from] IVConversionError), + #[error("provided authentication IV is malformed: {0}")] + MalformedIV(bs58::decode::Error), #[error("Only 'Register' or 'Authenticate' requests are allowed")] InvalidRequest, @@ -260,7 +256,7 @@ where /// * `packets`: unwrapped packets that are to be pushed back to the client. pub(crate) async fn push_packets_to_client( &mut self, - shared_keys: &LegacySharedKeys, + shared_keys: &SharedGatewayKey, packets: Vec>, ) -> Result<(), WsError> where @@ -270,10 +266,13 @@ where // be more explicit in the naming? let messages: Vec> = packets .into_iter() - .map(|received_message| { - Ok(BinaryResponse::new_pushed_mix_message(received_message) - .into_ws_message(shared_keys)) + .filter_map(|message| { + BinaryResponse::PushedMixMessage { message } + .into_ws_message(shared_keys) + .inspect_err(|err| error!("failed to encrypt client message: {err}")) + .ok() }) + .map(|msg| Ok(msg)) .collect(); let mut send_stream = futures::stream::iter(messages); match self.socket_connection { @@ -315,7 +314,7 @@ where async fn push_stored_messages_to_client( &mut self, client_address: DestinationAddressBytes, - shared_keys: &LegacySharedKeys, + shared_keys: &SharedGatewayKey, ) -> Result<(), InitialAuthenticationError> where S: AsyncRead + AsyncWrite + Unpin, @@ -363,44 +362,33 @@ where /// /// * `client_address`: address of the client. /// * `encrypted_address`: encrypted address of the client, presumably encrypted using the shared keys. - /// * `iv`: iv created for this particular encryption. + /// * `iv`: nonce/iv created for this particular encryption. async fn verify_stored_shared_key( &self, client_address: DestinationAddressBytes, encrypted_address: EncryptedAddressBytes, - iv: IV, - ) -> Result, InitialAuthenticationError> { + nonce: &[u8], + ) -> Result, InitialAuthenticationError> { let shared_keys = self .shared_state .storage .get_shared_keys(client_address) .await?; - if let Some(shared_keys) = shared_keys { - // this should never fail as we only ever construct persisted shared keys ourselves when inserting - // data to the storage. The only way it could fail is if we somehow changed implementation without - // performing proper migration - let keys = LegacySharedKeys::try_from_base58_string( - shared_keys.derived_aes128_ctr_blake3_hmac_keys_bs58, - ) - .map_err(|source| { - InitialAuthenticationError::MalformedStoredSharedKey { - client_id: client_address.as_base58_string(), - source, - } - })?; + let Some(stored_shared_keys) = shared_keys else { + return Ok(None); + }; - // TODO: SECURITY: - // this is actually what we have been doing in the past, however, - // after looking deeper into implementation it seems that only checks the encryption - // key part of the shared keys. the MAC key might still be wrong - // (though I don't see how could this happen unless client messed with himself - // and I don't think it could lead to any attacks, but somebody smarter should take a look) - if encrypted_address.verify(&client_address, &keys, &iv) { - Ok(Some(keys)) - } else { - Ok(None) + let keys = SharedGatewayKey::try_from(stored_shared_keys).map_err(|source| { + InitialAuthenticationError::MalformedStoredSharedKey { + client_id: client_address.as_base58_string(), + source, } + })?; + + // LEGACY ISSUE: we're not verifying HMAC key + if encrypted_address.verify(&client_address, &keys, nonce) { + Ok(Some(keys)) } else { Ok(None) } @@ -452,13 +440,13 @@ where /// /// * `client_address`: address of the client wishing to authenticate. /// * `encrypted_address`: ciphertext of the address of the client wishing to authenticate. - /// * `iv`: fresh IV received with the request. + /// * `iv`: fresh nonce/IV received with the request. async fn authenticate_client( &mut self, client_address: DestinationAddressBytes, encrypted_address: EncryptedAddressBytes, - iv: IV, - ) -> Result, InitialAuthenticationError> + nonce: &[u8], + ) -> Result, InitialAuthenticationError> where S: AsyncRead + AsyncWrite + Unpin, { @@ -468,7 +456,7 @@ where ); let shared_keys = self - .verify_stored_shared_key(client_address, encrypted_address, iv) + .verify_stored_shared_key(client_address, encrypted_address, nonce) .await?; if let Some(shared_keys) = shared_keys { @@ -550,7 +538,7 @@ where client_protocol_version: Option, address: String, enc_address: String, - iv: String, + raw_nonce: String, ) -> Result where S: AsyncRead + AsyncWrite + Unpin, @@ -564,7 +552,9 @@ where let address = DestinationAddressBytes::try_from_base58_string(address) .map_err(|err| InitialAuthenticationError::MalformedClientAddress(err.to_string()))?; let encrypted_address = EncryptedAddressBytes::try_from_base58_string(enc_address)?; - let iv = IV::try_from_base58_string(iv)?; + let nonce = bs58::decode(&raw_nonce) + .into_vec() + .map_err(InitialAuthenticationError::MalformedIV)?; // Check for duplicate clients if let Some(client_tx) = self.active_clients_store.get_remote_client(address) { @@ -574,7 +564,7 @@ where } let Some(shared_keys) = self - .authenticate_client(address, encrypted_address, iv) + .authenticate_client(address, encrypted_address, &nonce) .await? else { // it feels weird to be returning an 'Ok' here, but I didn't want to change the existing behaviour @@ -623,7 +613,7 @@ where async fn register_client( &mut self, client_address: DestinationAddressBytes, - client_shared_keys: &LegacySharedKeys, + client_shared_keys: &SharedGatewayKey, ) -> Result where S: AsyncRead + AsyncWrite + Unpin, diff --git a/gateway/src/node/client_handling/websocket/connection_handler/mod.rs b/gateway/src/node/client_handling/websocket/connection_handler/mod.rs index fa78d8fe8b..bc9f47f4b8 100644 --- a/gateway/src/node/client_handling/websocket/connection_handler/mod.rs +++ b/gateway/src/node/client_handling/websocket/connection_handler/mod.rs @@ -3,7 +3,7 @@ use crate::config::Config; use nym_credential_verification::BandwidthFlushingBehaviourConfig; -use nym_gateway_requests::registration::handshake::LegacySharedKeys; +use nym_gateway_requests::registration::handshake::SharedGatewayKey; use nym_gateway_requests::ServerResponse; use nym_gateway_storage::Storage; use nym_sphinx::DestinationAddressBytes; @@ -46,14 +46,14 @@ pub(crate) struct ClientDetails { #[zeroize(skip)] pub(crate) address: DestinationAddressBytes, pub(crate) id: i64, - pub(crate) shared_keys: LegacySharedKeys, + pub(crate) shared_keys: SharedGatewayKey, } impl ClientDetails { pub(crate) fn new( id: i64, address: DestinationAddressBytes, - shared_keys: LegacySharedKeys, + shared_keys: SharedGatewayKey, ) -> Self { ClientDetails { address,