Skip to content

Issue 295, (alternate) addresses bug with TSIG verification of large responses #297

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

Closed
frankarinnet wants to merge 1 commit into from
+35 −9
Closed

Issue 295, (alternate) addresses bug with TSIG verification of large responses #297

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 35 additions & 9 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 @@ -132,7 +132,8 @@ public static String nameToAlgorithm(Name name) {
private final Name name;
private final SecretKey macKey;
private final String macAlgorithm;
private final Mac sharedHmac;
// TODO: thread safety still broken
private Mac sharedHmac;

/**
* Verifies the data (computes the secure hash and compares it to the input)
Expand Down Expand Up @@ -589,9 +590,12 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
return Rcode.BADKEY;
}

Mac hmac = initHmac();
if (requestTSIG != null && tsig.getError() != Rcode.BADKEY && tsig.getError() != Rcode.BADSIG) {
hmacAddSignature(hmac, requestTSIG);
if (fullSignature) {
sharedHmac = initHmac();

if (requestTSIG != null && tsig.getError() != Rcode.BADKEY && tsig.getError() != Rcode.BADSIG) {
hmacAddSignature(sharedHmac, requestTSIG);
}
}

m.getHeader().decCount(Section.ADDITIONAL);
Expand All @@ -600,13 +604,13 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
if (log.isTraceEnabled()) {
log.trace(hexdump.dump("TSIG-HMAC header", header));
}
hmac.update(header);
sharedHmac.update(header);

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

DNSOutput out = new DNSOutput();
if (fullSignature) {
Expand All @@ -630,10 +634,10 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
if (log.isTraceEnabled()) {
log.trace(hexdump.dump("TSIG-HMAC variables", tsigVariables));
}
hmac.update(tsigVariables);
sharedHmac.update(tsigVariables);

byte[] signature = tsig.getSignature();
int digestLength = hmac.getMacLength();
int digestLength = sharedHmac.getMacLength();

// rfc4635#section-3.1, 4.:
// "MAC size" field is less than the larger of 10 (octets) and half
Expand All @@ -651,7 +655,7 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolea
signature.length);
return Rcode.BADSIG;
} else {
byte[] expectedSignature = hmac.doFinal();
byte[] expectedSignature = sharedHmac.doFinal();
if (!verify(expectedSignature, signature)) {
if (log.isDebugEnabled()) {
log.debug(
Expand Down Expand Up @@ -750,12 +754,28 @@ public int verify(Message m, byte[] b) {
nresponses++;
if (nresponses == 1) {
int result = key.verify(m, b, lastTSIG);

byte[] signature = tsig.getSignature();
DNSOutput out = new DNSOutput();
out.writeU16(signature.length);
byte[] z = out.toByteArray();
key.sharedHmac.update(z); // need to add hexdump at trace
key.sharedHmac.update(signature); // need to add hexdump at trace

lastTSIG = tsig;
return result;
}

if (tsig != null) {
int result = key.verify(m, b, lastTSIG, false);

key.sharedHmac.reset();
DNSOutput out = new DNSOutput();
out.writeU16(tsig.getSignature().length);
byte[] outBytes = out.toByteArray();
key.sharedHmac.update(outBytes); // need to add hexdump at trace
key.sharedHmac.update(tsig.getSignature()); // need to add hexdump at trace

lastsigned = nresponses;
lastTSIG = tsig;
return result;
Expand All @@ -766,6 +786,12 @@ public int verify(Message m, byte[] b) {
m.tsigState = Message.TSIG_FAILED;
return Rcode.FORMERR;
} else {

byte[] header = m.getHeader().toWire();
key.sharedHmac.update(header); // need to add hexdump at trace
int len = b.length - header.length;
key.sharedHmac.update(b, header.length, len); // need to add hexdump at trace

log.trace("Intermediate message {} without signature", nresponses);
m.tsigState = Message.TSIG_INTERMEDIATE;
return Rcode.NOERROR;
Expand Down