Merge pull request #614 from rustdesk/kx-v2

Kx v2

protos/rendezvous.proto

@@ -8,6 +8,8 @@ message IdPk {
   // a WebRTC peer's DTLS channel can be bound to its verified identity (defeats a rendezvous/relay
   // that swaps SDP fingerprints). Empty for non-WebRTC handshakes.
   string dtls_fingerprint = 3;
+  // Newest key exchange version the signer speaks; absent, i.e. 0, is the original scheme.
+  uint32 kx_version = 4;
 }
 
 message RegisterPeer {
@@ -232,6 +234,19 @@ message OnlineResponse {
 
 message KeyExchange {
   repeated bytes keys = 1; 
+  // From the server: the newest key exchange version it speaks. From the client: the version it
+  // picked, the highest both speak. Absent, i.e. 0, is the original scheme.
+  uint32 version = 2;
+  // Server signature of KxParams. Bit 255 of the signed X25519 public key requires this field;
+  // legacy keys leave it clear.
+  bytes signed_params = 3;
+}
+
+// Signed by the server beside its ephemeral public key: that key as it went out, so the signature
+// belongs to this exchange alone, and the newest key exchange version the server speaks.
+message KxParams {
+  bytes pk = 1;
+  uint32 version = 2;
 }
 
 message HealthCheck {

src/stream.rs

@@ -1,4 +1,4 @@
-use crate::{config, tcp, websocket, ResultType};
+use crate::{bail, config, tcp, websocket, ResultType};
 #[cfg(feature = "webrtc")]
 use crate::webrtc;
 use sodiumoxide::crypto::secretbox::Key;
@@ -80,6 +80,31 @@ impl Stream {
         }
     }
 
+    /// The stream key for the key exchange version in `t.picked`: version 0 is `set_key`, and
+    /// version 1 derives one key per direction from the transcript. Any other version is refused
+    /// rather than run as one of these. `is_initiator` is the side that sent the sealed key.
+    #[inline]
+    pub fn set_negotiated_key(
+        &mut self,
+        key: Key,
+        is_initiator: bool,
+        t: &tcp::KxTranscript,
+    ) -> ResultType<()> {
+        match t.picked {
+            0 => {
+                self.set_key(key);
+                Ok(())
+            }
+            1 => match self {
+                #[cfg(feature = "webrtc")]
+                Stream::WebRTC(_) => bail!("key exchange version 1 does not apply to WebRTC"),
+                Stream::WebSocket(s) => s.set_key_split(key, is_initiator, t),
+                Stream::Tcp(s) => s.set_key_split(key, is_initiator, t),
+            },
+            v => bail!("unsupported key exchange version {}", v),
+        }
+    }
+
     /// An opaque token that changes whenever bytes have arrived from the peer, a fragment of a
     /// message that is still incomplete included. Compare successive samples; the value itself
     /// means nothing. `None` where the transport reports no such thing.
@@ -288,3 +313,24 @@ impl Drop for Stream {
         self.close_webrtc();
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[tokio::test]
+    async fn test_set_negotiated_key_refuses_unknown_versions() {
+        let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        let mut stream = Stream::from(TcpStream::connect(addr).await.unwrap(), addr);
+        let t = |picked| tcp::KxTranscript {
+            initiator_pk: &[1u8; 32],
+            responder_pk: &[2u8; 32],
+            advertised: 1,
+            picked,
+        };
+        let mut set = |picked| stream.set_negotiated_key(Key([0u8; 32]), true, &t(picked));
+        assert!(set(2).is_err());
+        assert!(set(1).is_ok());
+    }
+}

src/tcp.rs

@@ -4,7 +4,7 @@ use bytes::{BufMut, Bytes, BytesMut};
 use futures::{SinkExt, StreamExt};
 use protobuf::Message;
 use sodiumoxide::crypto::{
-    box_,
+    box_, generichash,
     secretbox::{self, Key, Nonce},
 };
 use std::{
@@ -24,8 +24,43 @@ use tokio_util::codec::Framed;
 pub trait TcpStreamTrait: AsyncRead + AsyncWrite + Unpin {}
 pub struct DynTcpStream(pub Box<dyn TcpStreamTrait + Send + Sync>);
 
+/// The newest key exchange version this build speaks. Version 0 is the original scheme, and
+/// what an absent field means: one key for both directions. Version 1 splits the exchanged key
+/// into one per direction, the way Noise splits its cipher states after the handshake, binds
+/// the handshake transcript into both, and keeps the nonce layout.
+pub const KX_VERSION_LATEST: u32 = 1;
+
+/// Prefix of what the server signs as `KxParams`. The same key signs other messages, and a
+/// protobuf carries no type of its own, so the prefix is what keeps them apart.
+pub const KX_PARAMS_DOMAIN: &[u8] = b"rdkx-params";
+
+/// The version to run against a peer that advertised `advertised`: the highest both sides
+/// support.
+#[inline]
+pub fn kx_version_for(advertised: u32) -> u32 {
+    advertised.min(KX_VERSION_LATEST)
+}
+
+const KX_SPLIT_CONTEXT: &[u8] = b"rdkx-spl";
+const KX_SPLIT_INITIATOR: u8 = 1;
+const KX_SPLIT_RESPONDER: u8 = 2;
+
+/// What both sides saw during the key exchange, each from its own side of the wire, mixed
+/// into the split keys.
+pub struct KxTranscript<'a> {
+    /// The ephemeral public key of the side that sent the sealed key.
+    pub initiator_pk: &'a [u8],
+    /// The ephemeral public key the sealed key was sealed to.
+    pub responder_pk: &'a [u8],
+    /// The version the responder advertised: what it sent, what the initiator received.
+    pub advertised: u32,
+    /// The version the initiator picked.
+    pub picked: u32,
+}
+
+/// The sending key, the send and receive counters, and the receiving key where it differs.
 #[derive(Clone)]
-pub struct Encrypt(pub Key, pub u64, pub u64);
+pub struct Encrypt(pub Key, pub u64, pub u64, Option<Key>);
 
 pub struct FramedStream(
     pub Framed<DynTcpStream, BytesCodec>,
@@ -200,6 +235,16 @@ impl FramedStream {
         self.2 = Some(Encrypt::new(key));
     }
 
+    pub fn set_key_split(
+        &mut self,
+        key: Key,
+        is_initiator: bool,
+        t: &KxTranscript,
+    ) -> ResultType<()> {
+        self.2 = Some(Encrypt::new_split(key, is_initiator, t)?);
+        Ok(())
+    }
+
     fn get_nonce(seqnum: u64) -> Nonce {
         let mut nonce = Nonce([0u8; secretbox::NONCEBYTES]);
         nonce.0[..std::mem::size_of_val(&seqnum)].copy_from_slice(&seqnum.to_le_bytes());
@@ -295,7 +340,43 @@ impl<R: AsyncRead + AsyncWrite + Unpin> TcpStreamTrait for R {}
 
 impl Encrypt {
     pub fn new(key: Key) -> Self {
-        Self(key, 0, 0)
+        Self(key, 0, 0, None)
+    }
+
+    /// Version 1: the initiator, the side that sent the sealed key, sends under one subkey and
+    /// receives under the other; the responder the reverse.
+    pub fn new_split(key: Key, is_initiator: bool, t: &KxTranscript) -> ResultType<Self> {
+        let initiator = Self::derive_subkey(&key, KX_SPLIT_INITIATOR, t)?;
+        let responder = Self::derive_subkey(&key, KX_SPLIT_RESPONDER, t)?;
+        Ok(if is_initiator {
+            Self(initiator, 0, 0, Some(responder))
+        } else {
+            Self(responder, 0, 0, Some(initiator))
+        })
+    }
+
+    fn derive_subkey(key: &Key, direction: u8, t: &KxTranscript) -> ResultType<Key> {
+        if t.initiator_pk.len() != box_::PUBLICKEYBYTES
+            || t.responder_pk.len() != box_::PUBLICKEYBYTES
+        {
+            bail!("key exchange transcript has a public key of the wrong length");
+        }
+        let failed = |_| anyhow::anyhow!("key derivation failed");
+        let mut h =
+            generichash::State::new(Some(secretbox::KEYBYTES), Some(&key.0)).map_err(failed)?;
+        for part in [
+            KX_SPLIT_CONTEXT,
+            &[direction],
+            &t.advertised.to_le_bytes(),
+            &t.picked.to_le_bytes(),
+            t.initiator_pk,
+            t.responder_pk,
+        ] {
+            h.update(part).map_err(failed)?;
+        }
+        let mut subkey = [0u8; secretbox::KEYBYTES];
+        subkey.copy_from_slice(h.finalize().map_err(failed)?.as_ref());
+        Ok(Key(subkey))
     }
 
     pub fn dec(&mut self, bytes: &mut BytesMut) -> Result<(), Error> {
@@ -304,7 +385,7 @@ impl Encrypt {
         }
         self.2 += 1;
         let nonce = FramedStream::get_nonce(self.2);
-        match secretbox::open(bytes, &nonce, &self.0) {
+        match secretbox::open(bytes, &nonce, self.3.as_ref().unwrap_or(&self.0)) {
             Ok(res) => {
                 bytes.clear();
                 bytes.put_slice(&res);
@@ -342,3 +423,133 @@ impl Encrypt {
         Ok(Key(key))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const INITIATOR_PK: [u8; 32] = [1u8; 32];
+    const RESPONDER_PK: [u8; 32] = [2u8; 32];
+
+    fn transcript(advertised: u32) -> KxTranscript<'static> {
+        KxTranscript {
+            initiator_pk: &INITIATOR_PK,
+            responder_pk: &RESPONDER_PK,
+            advertised,
+            picked: KX_VERSION_LATEST,
+        }
+    }
+
+    fn seal_and_open(a: &mut Encrypt, b: &mut Encrypt, msg: &[u8]) -> Vec<u8> {
+        let sealed = a.enc(msg);
+        let mut buf = BytesMut::from(&sealed[..]);
+        b.dec(&mut buf).unwrap();
+        assert_eq!(&buf[..], msg);
+        sealed
+    }
+
+    #[test]
+    fn test_v0_shares_one_key_across_directions() {
+        let key = secretbox::gen_key();
+        let (mut initiator, mut responder) = (Encrypt::new(key.clone()), Encrypt::new(key));
+        let sent = seal_and_open(&mut initiator, &mut responder, b"hello");
+        let back = seal_and_open(&mut responder, &mut initiator, b"hello");
+        assert_eq!(sent, back);
+    }
+
+    // Fixed bytes: the version 0 frame is what released peers send, and the subkeys are the ones
+    // hbbs's own copy of this code pins, so a change here breaks the wire, not just this test.
+    #[test]
+    fn test_wire_vectors() {
+        let hex = |b: &[u8]| b.iter().map(|b| format!("{:02x}", b)).collect::<String>();
+        let key = Key([0x11u8; 32]);
+        assert_eq!(
+            hex(&Encrypt::new(key.clone()).enc(b"hello")),
+            "3b768f827fdcc4af555b9e42533be6611e2c333481"
+        );
+        let mut t = KxTranscript {
+            initiator_pk: &[0x22u8; 32],
+            responder_pk: &[0x33u8; 32],
+            advertised: 1,
+            picked: 1,
+        };
+        let Encrypt(send, _, _, recv) = Encrypt::new_split(key.clone(), true, &t).unwrap();
+        assert_eq!(
+            hex(&send.0),
+            "c189ec6e1935c0751cfc85a0f075405cae507d7925bb19687caf56cbb54e0ecf"
+        );
+        assert_eq!(
+            hex(&recv.unwrap().0),
+            "8a62648e9195cb10ea900c0a24d2a166c1e982347a2dc6833c3e756bf5715d36"
+        );
+        // With the two versions equal, swapping their order in the derivation would go unseen.
+        t.advertised = 3;
+        let Encrypt(send, _, _, recv) = Encrypt::new_split(key, true, &t).unwrap();
+        assert_eq!(
+            hex(&send.0),
+            "25b0778ae600f9731c7fa457d1cc5d40071b0aa3d34c90b53e1d5344b0e7123a"
+        );
+        assert_eq!(
+            hex(&recv.unwrap().0),
+            "5bbde0ac6a708d4cd1ff807125d79cb6030bde62f7a66b6cc1a79e31bd657756"
+        );
+    }
+
+    #[test]
+    fn test_split_directions_use_different_keys() {
+        let key = secretbox::gen_key();
+        let t = transcript(KX_VERSION_LATEST);
+        let mut initiator = Encrypt::new_split(key.clone(), true, &t).unwrap();
+        let mut responder = Encrypt::new_split(key.clone(), false, &t).unwrap();
+        let sent = seal_and_open(&mut initiator, &mut responder, b"hello");
+        let back = seal_and_open(&mut responder, &mut initiator, b"hello");
+        assert_ne!(sent, back);
+        // Neither direction's key is the exchanged key itself, so a version 0 peer cannot be
+        // paired with a split one by accident.
+        let mut v0 = Encrypt::new(key);
+        let mut buf = BytesMut::from(&sent[..]);
+        assert!(v0.dec(&mut buf).is_err());
+    }
+
+    #[test]
+    fn test_split_roles_must_differ() {
+        let key = secretbox::gen_key();
+        let t = transcript(KX_VERSION_LATEST);
+        let mut a = Encrypt::new_split(key.clone(), true, &t).unwrap();
+        let mut b = Encrypt::new_split(key, true, &t).unwrap();
+        let sealed = a.enc(b"hello");
+        let mut buf = BytesMut::from(&sealed[..]);
+        assert!(b.dec(&mut buf).is_err());
+    }
+
+    #[test]
+    fn test_kx_version_for_picks_the_highest_shared() {
+        assert_eq!(kx_version_for(0), 0);
+        assert_eq!(kx_version_for(1), 1);
+        assert_eq!(kx_version_for(KX_VERSION_LATEST), KX_VERSION_LATEST);
+        assert_eq!(kx_version_for(KX_VERSION_LATEST + 7), KX_VERSION_LATEST);
+    }
+
+    #[test]
+    fn test_split_binds_the_transcript() {
+        let key = secretbox::gen_key();
+        // The two sides put different advertisements into the transcript.
+        let mut initiator =
+            Encrypt::new_split(key.clone(), true, &transcript(KX_VERSION_LATEST + 1)).unwrap();
+        let mut responder = Encrypt::new_split(key, false, &transcript(KX_VERSION_LATEST)).unwrap();
+        let sealed = initiator.enc(b"hello");
+        let mut buf = BytesMut::from(&sealed[..]);
+        assert!(responder.dec(&mut buf).is_err());
+    }
+
+    #[test]
+    fn test_split_rejects_a_short_public_key() {
+        let t = KxTranscript {
+            initiator_pk: &[1u8; 31],
+            responder_pk: &RESPONDER_PK,
+            advertised: KX_VERSION_LATEST,
+            picked: KX_VERSION_LATEST,
+        };
+        assert!(Encrypt::new_split(secretbox::gen_key(), true, &t).is_err());
+    }
+}

src/websocket.rs

@@ -5,7 +5,7 @@ use crate::{
     protobuf::Message,
     socket_client::split_host_port,
     sodiumoxide::crypto::secretbox::Key,
-    tcp::Encrypt,
+    tcp::{Encrypt, KxTranscript},
     tls::{get_cached_tls_accept_invalid_cert, get_cached_tls_type, upsert_tls_cache, TlsType},
     ResultType,
 };
@@ -257,6 +257,17 @@ impl WsFramedStream {
         self.encrypt = Some(Encrypt::new(key));
     }
 
+    #[inline]
+    pub fn set_key_split(
+        &mut self,
+        key: Key,
+        is_initiator: bool,
+        t: &KxTranscript,
+    ) -> ResultType<()> {
+        self.encrypt = Some(Encrypt::new_split(key, is_initiator, t)?);
+        Ok(())
+    }
+
     #[inline]
     pub fn is_secured(&self) -> bool {
         self.encrypt.is_some()