dnsjava · ibauersachs · May 8, 2020 · May 6, 2020 · May 6, 2020 · May 6, 2020
diff --git a/src/main/java/org/xbill/DNS/Message.java b/src/main/java/org/xbill/DNS/Message.java
@@ -115,6 +115,9 @@ public static Message newUpdate(Name zone) {
           if (i == Section.ADDITIONAL) {
             if (rec.getType() == Type.TSIG) {
               tsigstart = pos;
+              if (j != count - 1) {
+                throw new WireParseException("TSIG is not the last record in the message");
+              }
             }
             if (rec.getType() == Type.SIG) {
               SIGRecord sig = (SIGRecord) rec;
@@ -519,7 +522,12 @@ private void toWire(DNSOutput out, int maxLength) {
     }
   }
 
-  /** Returns an array containing the wire format representation of the Message. */
+  /**
+   * Returns an array containing the wire format representation of the {@link Message}, but does not
+   * do any additional processing (e.g. OPT/TSIG records, truncation).
+   *
+   * <p>Do NOT use this to actually transmit a message, use {@link #toWire(int)} instead.
+   */
   public byte[] toWire() {
     DNSOutput out = new DNSOutput();
     toWire(out);
@@ -531,12 +539,15 @@ public byte[] toWire() {
    * Returns an array containing the wire format representation of the Message with the specified
    * maximum length. This will generate a truncated message (with the TC bit) if the message doesn't
    * fit, and will also sign the message with the TSIG key set by a call to setTSIG(). This method
-   * may return null if the message could not be rendered at all; this could happen if maxLength is
-   * smaller than a DNS header, for example.
+   * may return an empty byte array if the message could not be rendered at all; this could happen
+   * if maxLength is smaller than a DNS header, for example.
+   *
+   * <p>Do NOT use this method in conjunction with {@link TSIG#apply(Message, TSIGRecord)}, it
+   * produces inconsistent results! Use {@link #setTSIG(TSIG, int, TSIGRecord)} instead.
    *
    * @param maxLength The maximum length of the message.
-   * @return The wire format of the message, or null if the message could not be rendered into the
-   *     specified length.
+   * @return The wire format of the message, or an empty array if the message could not be rendered
+   *     into the specified length.
    * @see Flags
    * @see TSIG
    */

diff --git a/src/main/java/org/xbill/DNS/SimpleResolver.java b/src/main/java/org/xbill/DNS/SimpleResolver.java
@@ -240,12 +240,6 @@ private int maxUDPSize(Message query) {
    */
   @Override
   public CompletionStage<Message> sendAsync(Message query) {
-    Message ednsTsigQuery = query.clone();
-    applyEDNS(ednsTsigQuery);
-    if (tsig != null) {
-      tsig.apply(ednsTsigQuery, null);
-    }
-
     if (query.getHeader().getOpcode() == Opcode.QUERY) {
       Record question = query.getQuestion();
       if (question != null && question.getType() == Type.AXFR) {
@@ -263,10 +257,16 @@ public CompletionStage<Message> sendAsync(Message query) {
       }
     }
 
+    Message ednsTsigQuery = query.clone();
+    applyEDNS(ednsTsigQuery);
+    if (tsig != null) {
+      ednsTsigQuery.setTSIG(tsig, Rcode.NOERROR, null);
+    }
+
     return sendAsync(ednsTsigQuery, useTCP);
   }
 
-  private CompletableFuture<Message> sendAsync(Message query, boolean forceTcp) {
+  CompletableFuture<Message> sendAsync(Message query, boolean forceTcp) {
     int qid = query.getHeader().getID();
     byte[] out = query.toWire(Message.MAXLENGTH);
     int udpSize = maxUDPSize(query);

diff --git a/src/main/java/org/xbill/DNS/TSIGRecord.java b/src/main/java/org/xbill/DNS/TSIGRecord.java
@@ -145,7 +145,7 @@ String rrToString() {
 
     sb.append(timeSigned.getEpochSecond());
     sb.append(" ");
-    sb.append(fudge);
+    sb.append((int) fudge.getSeconds());
     sb.append(" ");
     sb.append(signature.length);
     if (Options.check("multiline")) {

diff --git a/src/test/java/org/xbill/DNS/TSIGRecordTest.java b/src/test/java/org/xbill/DNS/TSIGRecordTest.java
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: BSD-2-Clause
+package org.xbill.DNS;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.time.Duration;
+import java.time.Instant;
+import org.junit.jupiter.api.Test;
+
+public class TSIGRecordTest {
+  @Test
+  void testTsigToStringFudge() {
+    TSIGRecord r =
+        new TSIGRecord(
+            Name.root,
+            DClass.IN,
+            60,
+            TSIG.HMAC_MD5,
+            Instant.ofEpochSecond(1),
+            Duration.ofSeconds(5),
+            new byte[16],
+            1,
+            0,
+            null);
+    assertEquals(
+        "HMAC-MD5.SIG-ALG.REG.INT. 1 5 16 AAAAAAAAAAAAAAAAAAAAAA== NOERROR 0", r.rrToString());
+  }
+}
diff --git a/src/test/java/org/xbill/DNS/TSIGTest.java b/src/test/java/org/xbill/DNS/TSIGTest.java
@@ -1,10 +1,13 @@
 package org.xbill.DNS;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
 import org.junit.jupiter.api.Test;
 
 class TSIGTest {
@@ -25,6 +28,76 @@ void TSIG_query() throws IOException {
     assertTrue(parsed.isSigned());
   }
 
+  @Test
+  void TSIG_queryIsLastAddMessageRecord() throws IOException {
+    TSIG key = new TSIG(TSIG.HMAC_SHA256, "example.", "12345678");
+
+    Name qname = Name.fromString("www.example.");
+    Record rec = Record.newRecord(qname, Type.A, DClass.IN);
+    OPTRecord opt = new OPTRecord(SimpleResolver.DEFAULT_EDNS_PAYLOADSIZE, 0, 0, 0);
+    Message msg = Message.newQuery(rec);
+    msg.setTSIG(key, Rcode.NOERROR, null);
+    msg.addRecord(opt, Section.ADDITIONAL);
+    byte[] bytes = msg.toWire(512);
+    assertEquals(bytes[11], 2); // additional RR count, lower byte
+
+    Message parsed = new Message(bytes);
+    List<Record> additionalSection = parsed.getSection(Section.ADDITIONAL);
+    assertEquals(Type.string(Type.OPT), Type.string(additionalSection.get(0).getType()));
+    assertEquals(Type.string(Type.TSIG), Type.string(additionalSection.get(1).getType()));
+    int result = key.verify(parsed, bytes, null);
+    assertEquals(result, Rcode.NOERROR);
+    assertTrue(parsed.isSigned());
+  }
+
+  @Test
+  void TSIG_queryAndTsigApplyMisbehaves() throws IOException {
+    Name qname = Name.fromString("www.example.com.");
+    Record rec = Record.newRecord(qname, Type.A, DClass.IN);
+    OPTRecord opt = new OPTRecord(SimpleResolver.DEFAULT_EDNS_PAYLOADSIZE, 0, 0, 0);
+    Message msg = Message.newQuery(rec);
+    msg.addRecord(opt, Section.ADDITIONAL);
+    assertFalse(msg.isSigned());
+
+    TSIG key = new TSIG(TSIG.HMAC_SHA256, "example.", "12345678");
+    key.apply(msg, null); // additional RR count, lower byte
+    byte[] bytes = msg.toWire(Message.MAXLENGTH);
+
+    assertThrows(WireParseException.class, () -> new Message(bytes), "Expected TSIG error");
+  }
+
+  @Test
+  void TSIG_queryIsLastResolver() throws IOException {
+    Name qname = Name.fromString("www.example.com.");
+    Record rec = Record.newRecord(qname, Type.A, DClass.IN);
+    Message msg = Message.newQuery(rec);
+
+    TSIG key = new TSIG(TSIG.HMAC_SHA256, "example.", "12345678");
+    SimpleResolver res =
+        new SimpleResolver("127.0.0.1") {
+          @Override
+          CompletableFuture<Message> sendAsync(Message query, boolean forceTcp) {
+            byte[] out = query.toWire(Message.MAXLENGTH);
+            try {
+              return CompletableFuture.completedFuture(new Message(out));
+            } catch (IOException e) {
+              CompletableFuture<Message> f = new CompletableFuture<>();
+              f.completeExceptionally(e);
+              return f;
+            }
+          }
+        };
+    res.setTSIGKey(key);
+    Message parsed = res.send(msg);
+
+    List<Record> additionalSection = parsed.getSection(Section.ADDITIONAL);
+    assertEquals(Type.string(Type.OPT), Type.string(additionalSection.get(0).getType()));
+    assertEquals(Type.string(Type.TSIG), Type.string(additionalSection.get(1).getType()));
+    int result = key.verify(parsed, parsed.toWire(), null);
+    assertEquals(result, Rcode.NOERROR);
+    assertTrue(parsed.isSigned());
+  }
+
   @Test
   void TSIG_response() throws IOException {
     TSIG key = new TSIG(TSIG.HMAC_SHA256, "example.", "12345678");