8000 Fix handling of multi-message TSIG responses by ibauersachs · Pull Request #300 · dnsjava/dnsjava · GitHub
[go: up one dir, main page]
Skip to content

Fix handling of multi-message TSIG responses #300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 4, 2023
Merged

Fix handling of multi-message TSIG responses #300

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
36a0c7f
Fix handling of multi-message TSIG responses
ibauersachs Oct 29, 2023
781ebd9
Fix order of signature in TSIG mac
ibauersachs Oct 30, 2023
a9ed360
Verify that the last message is signed as required
ibauersachs Nov 2, 2023
14f083d
Add tests based on data from an actual transfer
ibauersachs Nov 4, 2023
b3eff09
Treat tcp dumps as binary, extend Javadoc
ibauersachs Nov 4, 2023
1f28212
Fix test assertions and reduce method complexity
ibauersachs Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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>
  • Loading branch information
@ibauersachs @frankarinnet @nguichon
3 people committed Oct 29, 2023
commit 36a0c7fc2d7904117fa47ccd046c24eba11ce8b5
145 changes: 132 additions & 13 deletions src/main/java/org/xbill/DNS/TSIG.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,20 +369,38 @@ public TSIGRecord generate(Message m, byte[] b, int error, TSIGRecord old) {
*/
public TSIGRecord generate(
Message m, byte[] b, int error, TSIGRecord old, boolean fullSignature) {
Mac hmac = null;
if (error == Rcode.NOERROR || error == Rcode.BADTIME || error == Rcode.BADTRUNC) {
hmac = initHmac();
}

return generate(m, b, error, old, fullSignature, hmac);
}

/**
* Generates a TSIG record with a specific error for a message that has been rendered.
*
* @param m The message
* @param b The rendered message
* @param error The error
* @param old If this message is a response, the TSIG from the request
* @param fullSignature {@code true} if this {@link TSIGRecord} is the to be added to the first of
* many messages in a TCP connection and all TSIG variables (rfc2845, 3.4.2.) should be
* included in the signature. {@code false} for subsequent messages with reduced TSIG
* variables set (rfc2845, 4.4.).
* @param hmac A mac instance to reuse for a stream of messages to sign, e.g. when doing a zone
* transfer.
* @return The TSIG record to be added to the message
*/
private TSIGRecord generate(
Message m, byte[] b, int error, TSIGRecord old, boolean fullSignature, Mac hmac) {
Instant timeSigned;
if (error == Rcode.BADTIME) {
timeSigned = old.getTimeSigned();
} else {
timeSigned = clock.instant();
}

boolean signing = false;
Mac hmac = null;
if (error == Rcode.NOERROR || error == Rcode.BADTIME || error == Rcode.BADTRUNC) {
signing = true;
hmac = initHmac();
}

Duration fudge;
int fudgeOption = Options.intValue("tsigfudge");
if (fudgeOption < 0 || fudgeOption > 0x7FFF) {
Expand All @@ -391,6 +409,7 @@ public TSIGRecord generate(
fudge = Duration.ofSeconds(fudgeOption);
}

boolean signing = hmac != null;
if (old != null && signing) {
hmacAddSignature(hmac, old);
}
Expand Down Expand Up @@ -572,6 +591,27 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG) {
* @since 3.2
*/
public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolean fullSignature) {
return verify(m, messageBytes, requestTSIG, fullSignature, null);
}

/**
* Verifies a TSIG record on an incoming message. Since this is only called in the context where a
* TSIG is expected to be present, it is an error if one is not present. After calling this
* routine, Message.isVerified() may be called on this message.
*
* @param m The message to verify
* @param messageBytes An array containing the message in unparsed form. This is necessary since
* TSIG signs the message in wire format, and we can't recreate the exact wire format (with
* the same name compression).
* @param requestTSIG If this message is a response, the TSIG from the request
* @param fullSignature {@code true} if this message is the first of many in a TCP connection and
* all TSIG variables (rfc2845, 3.4.2.) should be included in the signature. {@code false} for
* subsequent messages with reduced TSIG variables set (rfc2845, 4.4.).
* @return The result of the verification (as an Rcode)
* @see Rcode
*/
private int verify(
Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolean fullSignature, Mac hmac) {
m.tsigState = Message.TSIG_FAILED;
TSIGRecord tsig = m.getTSIG();
if (tsig == null) {
Expand All @@ -589,7 +629,10 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
return Rcode.BADKEY;
}

Mac hmac = initHmac();
if (hmac == null) {
hmac = initHmac();
}

if (requestTSIG != null && tsig.getError() != Rcode.BADKEY && tsig.getError() != Rcode.BADSIG) {
hmacAddSignature(hmac, requestTSIG);
}
Expand Down Expand Up @@ -664,7 +707,7 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
}

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

/** A helper class for generating signed message responses. */
public static class StreamGenerator {
private final TSIG key;

private final Mac sharedHmac;

private final int signEveryNthMessage;

private int numGenerated;
private TSIGRecord lastTsigRecord;

public StreamGenerator(TSIG key, TSIGRecord lastTsigRecord) {
// https://www.rfc-editor.org/rfc/rfc8945.html#section-5.3.1
// The TSIG MUST be included on all DNS messages in the response.
this(key, lastTsigRecord, 1);
}

/**
* This constructor is <b>only</b> for unit-testing {@link StreamVerifier} with responses where
* not every message is signed.
*/
StreamGenerator(TSIG key, TSIGRecord lastTsigRecord, int signEveryNthMessage) {
if (signEveryNthMessage < 1 || signEveryNthMessage > 100) {
throw new IllegalArgumentException("signEveryNthMessage must be between 1 and 100");
}

this.key = key;
this.lastTsigRecord = lastTsigRecord;
this.signEveryNthMessage = signEveryNthMessage;
sharedHmac = this.key.initHmac();
}

public void generate(Message message, boolean isLastMessage) {
boolean isNthMessage = numGenerated % signEveryNthMessage == 0;
boolean isFirstMessage = numGenerated == 0;
if (isFirstMessage || isNthMessage || isLastMessage) {
TSIGRecord r =
key.generate(
message,
message.toWire(),
Rcode.NOERROR,
lastTsigRecord,
isFirstMessage,
sharedHmac);
message.addRecord(r, Section.ADDITIONAL);
message.tsigState = Message.TSIG_SIGNED;
lastTsigRecord = r;
} else {
byte[] responseBytes = message.toWire(Message.MAXLENGTH);
sharedHmac.update(responseBytes);
}

numGenerated++;
}
}

/** A helper class for verifying multiple message responses. */
public static class StreamVerifier {
/** A helper class for verifying multiple message responses. */
private final TSIG key;

private final Mac sharedHmac;

private int nresponses;
private int lastsigned;
private TSIGRecord lastTSIG;

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

nresponses++;
if (nresponses == 1) {
int result = key.verify(m, b, lastTSIG);
int result = key.verify(m, b, lastTSIG, true, sharedHmac);
lastTSIG = tsig;
lastsigned = nresponses;
return result;
}

if (tsig != null) {
int result = key.verify(m, b, lastTSIG, false);
int result = key.verify(m, b, lastTSIG, false, sharedHmac);
lastsigned = nresponses;
lastTSIG = tsig;
return result;
Expand All @@ -767,10 +870,26 @@ public int verify(Message m, byte[] b) {
return Rcode.FORMERR;
} else {
log.trace("Intermediate message {} without signature", nresponses);
m.tsigState = Message.TSIG_INTERMEDIATE;
addUnsignedMessageToMac(m, b, sharedHmac);
return Rcode.NOERROR;
}
}
}

private void addUnsignedMessageToMac(Message m, byte[] messageBytes, Mac hmac) {
byte[] header = m.getHeader().toWire();
if (log.isTraceEnabled()) {
log.trace(hexdump.dump("TSIG-HMAC header", header));
}

hmac.update(header);
int len = messageBytes.length - header.length;
if (log.isTraceEnabled()) {
log.trace(hexdump.dump("TSIG-HMAC message after header", messageBytes, header.length, len));
}

hmac.update(messageBytes, header.length, len);
m.tsigState = Message.TSIG_INTERMEDIATE;
}
}
}
Loading
0