Skip to content

Commit 36a0c7f

Browse files
ibauersachsfrankarinnetnguichon
committed
Fix handling of multi-message TSIG responses
Closes #295 Closes #297 Closes #298 Closes #299 Co-authored-by: Frank Hill <frank@arin.net> Co-authored-by: Nick Guichon <nickg@arin.net>
1 parent e361d1e commit 36a0c7f

File tree

2 files changed

+288
-46
lines changed

2 files changed

+288
-46
lines changed

‎src/main/java/org/xbill/DNS/TSIG.java‎

Lines changed: 132 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -369,20 +369,38 @@ public TSIGRecord generate(Message m, byte[] b, int error, TSIGRecord old) {
369369
*/
370370
public TSIGRecord generate(
371371
Message m, byte[] b, int error, TSIGRecord old, boolean fullSignature) {
372+
Mac hmac = null;
373+
if (error == Rcode.NOERROR || error == Rcode.BADTIME || error == Rcode.BADTRUNC) {
374+
hmac = initHmac();
375+
}
376+
377+
return generate(m, b, error, old, fullSignature, hmac);
378+
}
379+
380+
/**
381+
* Generates a TSIG record with a specific error for a message that has been rendered.
382+
*
383+
* @param m The message
384+
* @param b The rendered message
385+
* @param error The error
386+
* @param old If this message is a response, the TSIG from the request
387+
* @param fullSignature {@code true} if this {@link TSIGRecord} is the to be added to the first of
388+
* many messages in a TCP connection and all TSIG variables (rfc2845, 3.4.2.) should be
389+
* included in the signature. {@code false} for subsequent messages with reduced TSIG
390+
* variables set (rfc2845, 4.4.).
391+
* @param hmac A mac instance to reuse for a stream of messages to sign, e.g. when doing a zone
392+
* transfer.
393+
* @return The TSIG record to be added to the message
394+
*/
395+
private TSIGRecord generate(
396+
Message m, byte[] b, int error, TSIGRecord old, boolean fullSignature, Mac hmac) {
372397
Instant timeSigned;
373398
if (error == Rcode.BADTIME) {
374399
timeSigned = old.getTimeSigned();
375400
} else {
376401
timeSigned = clock.instant();
377402
}
378403

379-
boolean signing = false;
380-
Mac hmac = null;
381-
if (error == Rcode.NOERROR || error == Rcode.BADTIME || error == Rcode.BADTRUNC) {
382-
signing = true;
383-
hmac = initHmac();
384-
}
385-
386404
Duration fudge;
387405
int fudgeOption = Options.intValue("tsigfudge");
388406
if (fudgeOption < 0 || fudgeOption > 0x7FFF) {
@@ -391,6 +409,7 @@ public TSIGRecord generate(
391409
fudge = Duration.ofSeconds(fudgeOption);
392410
}
393411

412+
boolean signing = hmac != null;
394413
if (old != null && signing) {
395414
hmacAddSignature(hmac, old);
396415
}
@@ -572,6 +591,27 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG) {
572591
* @since 3.2
573592
*/
574593
public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolean fullSignature) {
594+
return verify(m, messageBytes, requestTSIG, fullSignature, null);
595+
}
596+
597+
/**
598+
* Verifies a TSIG record on an incoming message. Since this is only called in the context where a
599+
* TSIG is expected to be present, it is an error if one is not present. After calling this
600+
* routine, Message.isVerified() may be called on this message.
601+
*
602+
* @param m The message to verify
603+
* @param messageBytes An array containing the message in unparsed form. This is necessary since
604+
* TSIG signs the message in wire format, and we can't recreate the exact wire format (with
605+
* the same name compression).
606+
* @param requestTSIG If this message is a response, the TSIG from the request
607+
* @param fullSignature {@code true} if this message is the first of many in a TCP connection and
608+
* all TSIG variables (rfc2845, 3.4.2.) should be included in the signature. {@code false} for
609+
* subsequent messages with reduced TSIG variables set (rfc2845, 4.4.).
610+
* @return The result of the verification (as an Rcode)
611+
* @see Rcode
612+
*/
613+
private int verify(
614+
Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolean fullSignature, Mac hmac) {
575615
m.tsigState = Message.TSIG_FAILED;
576616
TSIGRecord tsig = m.getTSIG();
577617
if (tsig == null) {
@@ -589,7 +629,10 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
589629
return Rcode.BADKEY;
590630
}
591631

592-
Mac hmac = initHmac();
632+
if (hmac == null) {
633+
hmac = initHmac();
634+
}
635+
593636
if (requestTSIG != null && tsig.getError() != Rcode.BADKEY && tsig.getError() != Rcode.BADSIG) {
594637
hmacAddSignature(hmac, requestTSIG);
595638
}
@@ -664,7 +707,7 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
664707
}
665708

666709
// validate time after the signature, as per
667-
// https://tools.ietf.org/html/draft-ietf-dnsop-rfc2845bis-08#section-5.4.3
710+
// https://www.rfc-editor.org/rfc/rfc8945.html#section-5.4
668711
Instant now = clock.instant();
669712
Duration delta = Duration.between(now, tsig.getTimeSigned()).abs();
670713
if (delta.compareTo(tsig.getFudge()) > 0) {
@@ -719,17 +762,76 @@ private static void writeTsigTime(Instant instant, DNSOutput out) {
719762
out.writeU32(timeLow);
720763
}
721764

765+
/** A helper class for generating signed message responses. */
766+
public static class StreamGenerator {
767+
private final TSIG key;
768+
769+
private final Mac sharedHmac;
770+
771+
private final int signEveryNthMessage;
772+
773+
private int numGenerated;
774+
private TSIGRecord lastTsigRecord;
775+
776+
public StreamGenerator(TSIG key, TSIGRecord lastTsigRecord) {
777+
// https://www.rfc-editor.org/rfc/rfc8945.html#section-5.3.1
778+
// The TSIG MUST be included on all DNS messages in the response.
779+
this(key, lastTsigRecord, 1);
780+
}
781+
782+
/**
783+
* This constructor is <b>only</b> for unit-testing {@link StreamVerifier} with responses where
784+
* not every message is signed.
785+
*/
786+
StreamGenerator(TSIG key, TSIGRecord lastTsigRecord, int signEveryNthMessage) {
787+
if (signEveryNthMessage < 1 || signEveryNthMessage > 100) {
788+
throw new IllegalArgumentException("signEveryNthMessage must be between 1 and 100");
789+
}
790+
791+
this.key = key;
792+
this.lastTsigRecord = lastTsigRecord;
793+
this.signEveryNthMessage = signEveryNthMessage;
794+
sharedHmac = this.key.initHmac();
795+
}
796+
797+
public void generate(Message message, boolean isLastMessage) {
798+
boolean isNthMessage = numGenerated % signEveryNthMessage == 0;
799+
boolean isFirstMessage = numGenerated == 0;
800+
if (isFirstMessage || isNthMessage || isLastMessage) {
801+
TSIGRecord r =
802+
key.generate(
803+
message,
804+
message.toWire(),
805+
Rcode.NOERROR,
806+
lastTsigRecord,
807+
isFirstMessage,
808+
sharedHmac);
809+
message.addRecord(r, Section.ADDITIONAL);
810+
message.tsigState = Message.TSIG_SIGNED;
811+
lastTsigRecord = r;
812+
} else {
813+
byte[] responseBytes = message.toWire(Message.MAXLENGTH);
814+
sharedHmac.update(responseBytes);
815+
}
816+
817+
numGenerated++;
818+
}
819+
}
820+
821+
/** A helper class for verifying multiple message responses. */
722822
public static class StreamVerifier {
723-
/** A helper class for verifying multiple message responses. */
724823
private final TSIG key;
725824

825+
private final Mac sharedHmac;
826+
726827
private int nresponses;
727828
private int lastsigned;
728829
private TSIGRecord lastTSIG;
729830

730831
/** Creates an object to verify a multiple message response */
731832
public StreamVerifier(TSIG tsig, TSIGRecord queryTsig) {
732833
key = tsig;
834+
sharedHmac = key.initHmac();
733835
nresponses = 0;
734836
lastTSIG = queryTsig;
735837
}
@@ -749,13 +851,14 @@ public int verify(Message m, byte[] b) {
749851

750852
nresponses++;
751853
if (nresponses == 1) {
752-
int result = key.verify(m, b, lastTSIG);
854+
int result = key.verify(m, b, lastTSIG, true, sharedHmac);
753855
lastTSIG = tsig;
856+
lastsigned = nresponses;
754857
return result;
755858
}
756859

757860
if (tsig != null) {
758-
int result = key.verify(m, b, lastTSIG, false);
861+
int result = key.verify(m, b, lastTSIG, false, sharedHmac);
759862
lastsigned = nresponses;
760863
lastTSIG = tsig;
761864
return result;
@@ -767,10 +870,26 @@ public int verify(Message m, byte[] b) {
767870
return Rcode.FORMERR;
768871
} else {
769872
log.trace("Intermediate message {} without signature", nresponses);
770-
m.tsigState = Message.TSIG_INTERMEDIATE;
873+
addUnsignedMessageToMac(m, b, sharedHmac);
771874
return Rcode.NOERROR;
772875
}
773876
}
774877
}
878+
879+
private void addUnsignedMessageToMac(Message m, byte[] messageBytes, Mac hmac) {
880+
byte[] header = m.getHeader().toWire();
881+
if (log.isTraceEnabled()) {
882+
log.trace(hexdump.dump("TSIG-HMAC header", header));
883+
}
884+
885+
hmac.update(header);
886+
int len = messageBytes.length - header.length;
887+
if (log.isTraceEnabled()) {
888+
log.trace(hexdump.dump("TSIG-HMAC message after header", messageBytes, header.length, len));
889+
}
890+
891+
hmac.update(messageBytes, header.length, len);
892+
m.tsigState = Message.TSIG_INTERMEDIATE;
893+
}
775894
}
776895
}

0 commit comments

Comments
 (0)