Refactor a bit, remove question padding

src/crypto.rs

@@ -142,29 +142,45 @@ impl CryptKeyPair {
         kp
     }
 
-    pub fn decrypt(
+    pub fn compute_shared_key(&self, pk: &[u8]) -> Result<SharedKey, Error> {
-        &self,
+        let mut shared_key = SharedKey::default();
-        client_pk: &[u8],
+        let res = unsafe {
-        nonce: &[u8],
+            crypto_box_curve25519xchacha20poly1305_beforenm(
-        encrypted: &[u8],
+                shared_key.0.as_mut_ptr(),
-    ) -> Result<Vec<u8>, Error> {
+                pk.as_ptr(),
+                self.sk.0.as_ptr(),
+            )
+        };
+        ensure!(res == 0, "Weak public key");
+        Ok(shared_key)
+    }
+}
+
+#[derive(Debug, Default)]
+pub struct SharedKey([u8; crypto_box_curve25519xchacha20poly1305_BEFORENMBYTES as usize]);
+
+impl SharedKey {
+    pub fn decrypt(&self, nonce: &[u8], encrypted: &[u8]) -> Result<Vec<u8>, Error> {
         let encrypted_len = encrypted.len();
         let mut decrypted =
             vec![0u8; encrypted_len - crypto_box_curve25519xchacha20poly1305_MACBYTES as usize];
         let res = unsafe {
-            libsodium_sys::crypto_box_curve25519xchacha20poly1305_open_easy(
+            libsodium_sys::crypto_box_curve25519xchacha20poly1305_open_easy_afternm(
                 decrypted.as_mut_ptr(),
                 encrypted.as_ptr(),
                 encrypted_len as _,
                 nonce.as_ptr(),
-                client_pk.as_ptr(),
+                self.0.as_ptr(),
-                self.sk.as_bytes().as_ptr(),
             )
         };
-        match res {
+        ensure!(res == 0, "Unable to decrypt");
-            0 => Ok(decrypted),
+        let idx = decrypted
-            _ => bail!("Unable to decrypt"),
+            .iter()
-        }
+            .rposition(|x| *x != 0x00)
+            .ok_or_else(|| format_err!("Padding error"))?;
+        ensure!(decrypted[idx] == 0x80, "Padding error");
+        decrypted.truncate(idx);
+        Ok(decrypted)
     }
 }
 

src/dns.rs

@@ -295,6 +295,12 @@ pub fn serve_certificates<'t>(
     expected_qname: &str,
     dnscrypt_encryption_params_set: impl IntoIterator<Item = &'t DNSCryptEncryptionParams>,
 ) -> Result<Option<Vec<u8>>, Error> {
+    ensure!(client_packet.len() >= DNS_HEADER_SIZE, "Short packet");
+    ensure!(qdcount(&client_packet) == 1, "No question");
+    ensure!(
+        !is_response(&client_packet),
+        "Question expected, but got a response instead"
+    );
     let offset = skip_name(client_packet, DNS_HEADER_SIZE)?;
     ensure!(client_packet.len() - offset >= 4, "Short packet");
     let qtype = BigEndian::read_u16(&client_packet[offset..]);

src/dnscrypt.rs

@@ -11,61 +11,37 @@ pub const DNSCRYPT_CLIENT_MAGIC_SIZE: usize = 8;
 pub const DNSCRYPT_CLIENT_PK_SIZE: usize = 32;
 pub const DNSCRYPT_CLIENT_NONCE_SIZE: usize = 12;
 
-pub struct DNSCryptQuery<'t> {
+pub fn decrypt(
-    wrapped_packet: &'t [u8],
+    wrapped_packet: &[u8],
-}
+    dnscrypt_encryption_params_set: &[DNSCryptEncryptionParams],
-
+) -> Result<Vec<u8>, Error> {
-impl<'t> DNSCryptQuery<'t> {
+    ensure!(
-    pub fn new(
+        wrapped_packet.len()
-        wrapped_packet: &'t [u8],
+            >= DNSCRYPT_CLIENT_MAGIC_SIZE
-        dnscrypt_encryption_params_set: &[DNSCryptEncryptionParams],
+                + DNSCRYPT_CLIENT_PK_SIZE
-    ) -> Result<Self, Error> {
+                + DNSCRYPT_CLIENT_NONCE_SIZE
-        ensure!(
+                + DNS_HEADER_SIZE,
-            wrapped_packet.len()
+        "Short packet"
-                >= DNSCRYPT_CLIENT_MAGIC_SIZE
+    );
-                    + DNSCRYPT_CLIENT_PK_SIZE
+    let client_magic = &wrapped_packet[..DNSCRYPT_CLIENT_MAGIC_SIZE];
-                    + DNSCRYPT_CLIENT_NONCE_SIZE
+    let client_pk = &wrapped_packet
-                    + DNS_HEADER_SIZE,
+        [DNSCRYPT_CLIENT_MAGIC_SIZE..DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE];
-            "Short packet"
+    let client_nonce = &wrapped_packet[DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE
-        );
+        ..DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE + DNSCRYPT_CLIENT_NONCE_SIZE];
-        let dnscrypt_query = DNSCryptQuery { wrapped_packet };
+    let encrypted_packet = &wrapped_packet
-        let client_magic = dnscrypt_query.client_magic();
+        [DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE + DNSCRYPT_CLIENT_NONCE_SIZE..];
-        let dnscrypt_encryption_params = dnscrypt_encryption_params_set
+    let encrypted_packet_len = encrypted_packet.len();
-            .iter()
+
-            .find(|p| p.client_magic() == client_magic)
+    let dnscrypt_encryption_params = dnscrypt_encryption_params_set
-            .ok_or_else(|| format_err!("Client magic not found"))?;
+        .iter()
-
+        .find(|p| p.client_magic() == client_magic)
-        let encrypted_packet = dnscrypt_query.encrypted_packet();
+        .ok_or_else(|| format_err!("Client magic not found"))?;
-        let encrypted_packet_len = encrypted_packet.len();
+
-        let mut nonce = vec![0u8; crypto_box_curve25519xchacha20poly1305_NONCEBYTES as usize];
+    let mut nonce = vec![0u8; crypto_box_curve25519xchacha20poly1305_NONCEBYTES as usize];
-        &mut nonce[..crypto_box_curve25519xchacha20poly1305_HALFNONCEBYTES]
+    &mut nonce[..crypto_box_curve25519xchacha20poly1305_HALFNONCEBYTES]
-            .copy_from_slice(dnscrypt_query.client_nonce());
+        .copy_from_slice(client_nonce);
-        let resolver_kp = dnscrypt_encryption_params.resolver_kp();
+    let resolver_kp = dnscrypt_encryption_params.resolver_kp();
-        resolver_kp.decrypt(dnscrypt_query.client_pk(), &nonce, encrypted_packet)?;
+    let shared_secret = resolver_kp.compute_shared_key(client_pk)?;
-        dbg!("ok");
+    let packet = shared_secret.decrypt(&nonce, encrypted_packet)?;
-        Ok(dnscrypt_query)
+    Ok(packet)
-    }
-
-    pub fn client_magic(&self) -> &[u8] {
-        &self.wrapped_packet[..DNSCRYPT_CLIENT_MAGIC_SIZE]
-    }
-
-    pub fn client_pk(&self) -> &[u8] {
-        &self.wrapped_packet
-            [DNSCRYPT_CLIENT_MAGIC_SIZE..DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE]
-    }
-
-    pub fn client_nonce(&self) -> &[u8] {
-        &self.wrapped_packet[DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE
-            ..DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE + DNSCRYPT_CLIENT_NONCE_SIZE]
-    }
-
-    pub fn encrypted_packet(&self) -> &[u8] {
-        &self.wrapped_packet
-            [DNSCRYPT_CLIENT_MAGIC_SIZE + DNSCRYPT_CLIENT_PK_SIZE + DNSCRYPT_CLIENT_NONCE_SIZE..]
-    }
-
-    pub fn into_packet(self) -> Vec<u8> {
-        self.encrypted_packet().to_vec()
-    }
 }

src/main.rs

@@ -98,16 +98,11 @@ async fn handle_client_query(
     client_ctx: ClientCtx,
     encrypted_packet: Vec<u8>,
 ) -> Result<(), Error> {
-    let packet = DNSCryptQuery::new(&encrypted_packet, &globals.dnscrypt_encryption_params_set);
+    let packet = dnscrypt::decrypt(&encrypted_packet, &globals.dnscrypt_encryption_params_set);
     let mut packet = match packet {
+        Ok(packet) => packet,
         Err(_) => {
             let packet = encrypted_packet;
-            ensure!(packet.len() >= DNS_HEADER_SIZE, "Short packet");
-            ensure!(dns::qdcount(&packet) == 1, "No question");
-            ensure!(
-                !dns::is_response(&packet),
-                "Question expected, but got a response instead"
-            );
             if let Some(synth_packet) = serve_certificates(
                 &packet,
                 &globals.provider_name,
@@ -117,8 +112,13 @@ async fn handle_client_query(
             }
             bail!("Unencrypted query");
         }
-        Ok(packet) => packet.into_packet(),
     };
+    ensure!(packet.len() >= DNS_HEADER_SIZE, "Short packet");
+    ensure!(qdcount(&packet) == 1, "No question");
+    ensure!(
+        !dns::is_response(&packet),
+        "Question expected, but got a response instead"
+    );
 
     let original_tid = dns::tid(&packet);
     let tid = random();