From 808af2a671246af8f82c7badff554e7a3ce7dc6e Mon Sep 17 00:00:00 2001
From: Martin Hoffmann <martin@nlnetlabs.nl>
Date: Tue, 23 Jul 2024 15:00:14 +0200
Subject: [PATCH] Fix TSIG server sequence signing and switch test to dig.
 (#356)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR fixes an issue with signature generation in the TSIG server
sequence, i.e., when the answer to a request signed with a TSIG key has
multiple messages.

This issue wasn’t caught by the interop test since drill happily consumed to
wrong signatures. Consequently, this PR switches the test to use dig which
won’t error out but at least complain about them in the output.
---
 src/tsig/interop.rs | 37 ++++++++++++++++++++++---------------
 src/tsig/mod.rs     |  3 ++-
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/tsig/interop.rs b/src/tsig/interop.rs
index 67097c2d..be614e82 100644
--- a/src/tsig/interop.rs
+++ b/src/tsig/interop.rs
@@ -19,6 +19,7 @@ use std::io::{Read, Write};
 use std::net::{IpAddr, SocketAddr, TcpListener, TcpStream, UdpSocket};
 use std::process::Command;
 use std::str::FromStr;
+use std::string::String;
 use std::time::Duration;
 use std::vec::Vec;
 use std::{env, fs, io, path::PathBuf, thread};
@@ -124,10 +125,10 @@ fn tsig_client_nsd() {
     res.unwrap(); // Panic if the thread panicked.
 }
 
-/// Tests the TSIG server implementation against drill as a client.
+/// Tests the TSIG server implementation against dig as a client.
 #[test]
 #[ignore]
-fn tsig_server_drill() {
+fn tsig_server_dig() {
     let rng = SystemRandom::new();
     let (key, secret) = tsig::Key::generate(
         tsig::Algorithm::Sha1,
@@ -138,7 +139,7 @@ fn tsig_server_drill() {
     )
     .unwrap();
     let secret = base64::encode_string(&secret);
-    let secret = format!("test.key:{}:hmac-sha1", secret);
+    let secret = format!("hmac-sha1:test.key:{}", secret);
 
     let join = thread::spawn(move || {
         let sock = UdpSocket::bind("127.0.0.1:54322").unwrap();
@@ -180,12 +181,15 @@ fn tsig_server_drill() {
         }
     });
 
-    let status = Command::new("/usr/bin/drill")
+    let output = Command::new("/usr/bin/dig")
         .args(["-p", "54322", "-y", &secret, "example.com", "@127.0.0.1"])
-        .status()
-        .expect("failed to start drill");
+        .output()
+        .expect("failed to start dig");
     drop(join);
-    assert!(status.success());
+    assert!(output.status.success());
+    assert!(!String::from_utf8(output.stdout)
+        .unwrap()
+        .contains("tsig verify failure"));
 }
 
 /// Test the client sequence implementation against NSD.
@@ -274,10 +278,10 @@ fn tsig_client_sequence_nsd() {
     res.unwrap(); // Panic if the thread paniced.
 }
 
-/// Tests the TSIG server sequence implementation against drill.
+/// Tests the TSIG server sequence implementation against dig.
 #[test]
 #[ignore]
-fn tsig_server_sequence_drill() {
+fn tsig_server_sequence_dig() {
     let rng = SystemRandom::new();
     let (key, secret) = tsig::Key::generate(
         tsig::Algorithm::Sha1,
@@ -288,7 +292,7 @@ fn tsig_server_sequence_drill() {
     )
     .unwrap();
     let secret = base64::encode_string(&secret);
-    let secret = format!("test.key:{}:hmac-sha1", secret);
+    let secret = format!("hmac-sha1:test.key:{}", secret);
     let listener = TcpListener::bind("127.0.0.1:54324").unwrap();
     let port = listener.local_addr().unwrap().port();
 
@@ -327,21 +331,24 @@ fn tsig_server_sequence_drill() {
         }
     });
 
-    let status = Command::new("/usr/bin/drill")
+    let output = Command::new("/usr/bin/dig")
         .args([
             "-p",
             &format!("{}", port),
             "-y",
             &secret,
-            "-t",
             "example.com",
             "AXFR",
             "@127.0.0.1",
+            "+tcp",
         ])
-        .status()
-        .expect("failed to start drill");
+        .output()
+        .expect("failed to start dig");
     drop(join);
-    assert!(status.success());
+    assert!(output.status.success());
+    assert!(!String::from_utf8(output.stdout)
+        .unwrap()
+        .contains("tsig verify failure"));
 }
 
 //------------ Helpers ------------------------------------------------------
diff --git a/src/tsig/mod.rs b/src/tsig/mod.rs
index 50475fa1..89e7dd8c 100644
--- a/src/tsig/mod.rs
+++ b/src/tsig/mod.rs
@@ -897,7 +897,7 @@ impl<K: AsRef<Key>> ServerSequence<K> {
         SigningContext::server_request(store, message, now).map(|context| {
             context.map(|context| ServerSequence {
                 context,
-                first: false,
+                first: true,
             })
         })
     }
@@ -943,6 +943,7 @@ impl<K: AsRef<Key>> ServerSequence<K> {
                 &variables,
             )
         };
+        self.context.apply_signature(mac.as_ref());
         let mac = self.key().signature_slice(&mac);
         self.key().complete_message(message, &variables, mac)
     }