More on negotiation and dnssec record verification

Signed-off-by: Luca Fulchir <luca.fulchir@runesauth.com>

src/connection/handshake/mod.rs

@@ -11,6 +11,7 @@ use ::std::rc::Rc;
 
 /// Handshake errors
 #[derive(::thiserror::Error, Debug, Copy, Clone)]
+#[non_exhaustive]
 pub enum Error {
     /// Error while parsing the handshake packet
     /// TODO: more detailed parsing errors
@@ -25,6 +26,12 @@ pub enum Error {
     /// Not enough data
     #[error("not enough data")]
     NotEnoughData,
+    /// Could not find common cryptography
+    #[error("Negotiation of keys/hkdfs/ciphers failed")]
+    Negotiation,
+    /// Could not generate Keys
+    #[error("Key generation failed")]
+    KeyGeneration,
 }
 
 /// List of possible handshakes

src/dnssec/record.rs

@@ -46,7 +46,7 @@ use crate::{
     connection::handshake::HandshakeID,
     enc::{
         self,
-        asym::{KeyExchangeKind, KeyID, PubKey},
+        asym::{ExchangePubKey, KeyExchangeKind, KeyID, PubKey},
         hkdf::HkdfKind,
         sym::CipherKind,
     },
@@ -498,7 +498,7 @@ impl Record {
             let (public_key, bytes) = match PubKey::deserialize(
                 &raw[bytes_parsed..(bytes_parsed + pubkey_length)],
             ) {
-                Ok(public_key_and_bytes) => public_key_and_bytes,
+                Ok((public_key, bytes)) => (public_key, bytes),
                 Err(enc::Error::UnsupportedKey(_)) => {
                     // continue parsing. This could be a new pubkey type
                     // that is not supported by an older client
@@ -582,6 +582,14 @@ impl Record {
                 if idx.0 as usize >= result.public_keys.len() {
                     return Err(Error::Max16PublicKeys);
                 }
+                if !result.public_keys[idx.0 as usize]
+                    .1
+                    .kind()
+                    .capabilities()
+                    .has_exchange()
+                {
+                    return Err(Error::UnsupportedData(bytes_parsed));
+                }
             }
         }
         if bytes_parsed != raw.len() {

src/enc/asym.rs

@@ -41,6 +41,17 @@ pub enum KeyCapabilities {
     /// All: sign, encrypt, Key Exchange
     SignEncryptExchage,
 }
+impl KeyCapabilities {
+    /// Check if this key supports eky exchage
+    pub fn has_exchange(&self) -> bool {
+        match self {
+            KeyCapabilities::Exchange
+            | KeyCapabilities::SignExchange
+            | KeyCapabilities::SignEncryptExchage => true,
+            _ => false,
+        }
+    }
+}
 
 /// Kind of key used in the handshake
 #[derive(Debug, Copy, Clone, PartialEq, ::num_derive::FromPrimitive)]
@@ -253,7 +264,7 @@ impl ExchangePubKey {
     }
     /// serialize the key into the buffer
     /// NOTE: Assumes there is enough space
-    fn serialize_into(&self, out: &mut [u8]) {
+    pub fn serialize_into(&self, out: &mut [u8]) {
         match self {
             ExchangePubKey::X25519(pk) => {
                 let bytes = pk.as_bytes();

src/inner/worker.rs

@@ -5,6 +5,7 @@ use crate::{
     connection::{
         self,
         handshake::{
+            self,
             dirsync::{self, DirSync},
             Handshake, HandshakeData,
         },
@@ -13,7 +14,7 @@ use crate::{
     },
     dnssec,
     enc::{
-        asym::{self, PubKey},
+        asym::{self, PrivKey, PubKey},
         hkdf::{self, Hkdf, HkdfKind},
         sym::{self, Secret},
         Random,
@@ -181,6 +182,9 @@ impl Worker {
                                 return None;
                             }
 
+                            // make sure this server has a public key
+                            // that supports one of the key exchanges that
+                            // *we* support
                             for idx in addr.public_key_idx.iter() {
                                 let key_supported_k_x =
                                     dnssec_record.public_keys[idx.0 as usize]
@@ -198,7 +202,7 @@ impl Worker {
                                             addr,
                                             dnssec_record.public_keys
                                                 [idx.0 as usize],
-                                            exchange,
+                                            exchange.clone(),
                                         ))
                                     }
                                     None => return None,
@@ -217,25 +221,27 @@ impl Worker {
                             continue 'mainloop;
                         }
                     };
-                    let hkdf = match hkdf::client_select_hkdf(
+                    let hkdf_selected = match hkdf::client_select_hkdf(
                         &self.cfg,
                         &dnssec_record.hkdfs,
                     ) {
-                        Some(hkdf) => hkdf,
+                        Some(hkdf_selected) => hkdf_selected,
                         None => {
-                            let _ = send_res
+                            let _ = send_res.send(Err(
-                                .send(Err(crate::Error::HandshakeNegotiation));
+                                handshake::Error::Negotiation.into(),
+                            ));
                             continue 'mainloop;
                         }
                     };
-                    let cipher = match sym::client_select_cipher(
+                    let cipher_selected = match sym::client_select_cipher(
                         &self.cfg,
                         &dnssec_record.ciphers,
                     ) {
-                        Some(cipher) => cipher,
+                        Some(cipher_selected) => cipher_selected,
                         None => {
-                            let _ = send_res
+                            let _ = send_res.send(Err(
-                                .send(Err(crate::Error::HandshakeNegotiation));
+                                handshake::Error::Negotiation.into(),
+                            ));
                             continue 'mainloop;
                         }
                     };
@@ -243,8 +249,36 @@ impl Worker {
                     let (priv_key, pub_key) =
                         match exchange.new_keypair(&self.rand) {
                             Ok(pair) => pair,
-                            Err(_) => todo!(),
+                            Err(_) => {
+                                ::tracing::error!("Failed to generate keys");
+                                let _ = send_res.send(Err(
+                                    handshake::Error::KeyGeneration.into(),
+                                ));
+                                continue 'mainloop;
+                            }
                         };
+                    let hkdf;
+                    if let PubKey::Exchange(srv_pub) = key.1 {
+                        let secret =
+                            match priv_key.key_exchange(exchange, srv_pub) {
+                                Ok(secret) => secret,
+                                Err(_) => {
+                                    ::tracing::warn!(
+                                        "Could not run the key exchange"
+                                    );
+                                    let _ = send_res.send(Err(
+                                        handshake::Error::Negotiation.into(),
+                                    ));
+                                    continue 'mainloop;
+                                }
+                            };
+                        hkdf = Hkdf::new(hkdf_selected, b"fenrir", secret);
+                    } else {
+                        // crate::dnssec already verifies that the keys
+                        // listed in dnssec::Record.addresses.public_key_idx
+                        // are PubKey::Exchange
+                        unreachable!()
+                    }
                     // build request
                     /*
                     let req_data = dirsync::ReqData {

src/lib.rs

@@ -62,9 +62,6 @@ pub enum Error {
     /// Resolution problems. wrong or incomplete DNSSEC data
     #[error("DNSSEC resolution: {0}")]
     Resolution(String),
-    /// No common cryptographic primitives
-    #[error("No common cryptographic primitives")]
-    HandshakeNegotiation,
 }
 
 /// Instance of a fenrir endpoint