8000 Merge pull request #249 from dnsjava/bugfix/tsig-response · dnsjava/dnsjava@17fe79c · GitHub
[go: up one dir, main page]
Skip to content

Commit 17fe79c

Browse files
ibauersachsibauersachs
authored
Merge pull request #249 from dnsjava/bugfix/tsig-response
Fix validation of TSIG signed responses
2 parents 181aaaf + a0b3470 commit 17fe79c

File tree

7 files changed

+352
-196
lines changed

7 files changed

+352
-196
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ private void verifyTSIG(Message query, Message response, byte[] b, TSIG tsig) {
640640
return;
641641
}
642642

643-
int error = tsig.verify(response, b, query.getTSIG());
643+
int error = tsig.verify(response, b, query.getGeneratedTSIG());
644644
log.debug(
645645
"TSIG verify for query {}, {}/{}: {}",
646646
query.getHeader().getID(),

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

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public class Message implements Cloneable {
3232
private List<Record>[] sections;
3333
private int size;
3434
private TSIG tsigkey;
35+
private TSIGRecord generatedTsig;
3536
private TSIGRecord querytsig;
3637
private int tsigerror;
3738
private Resolver resolver;
@@ -274,7 +275,7 @@ public boolean findRRset(Name name, int type) {
274275
*/
275276
public Record getQuestion() {
276277
List<Record> l = sections[Section.QUESTION];
277-
if (l == null || l.size() == 0) {
278+
if (l == null || l.isEmpty()) {
278279
return null;
279280
}
280281
return l.get(0);
@@ -300,6 +301,16 @@ public TSIGRecord getTSIG() {
300301
return (TSIGRecord) rec;
301302
}
302303

304+
/**
305+
* Gets the generated {@link TSIGRecord}. Only valid if the messages has been converted to wire
306+
* format with {@link #toWire(int)} before.
307+
*
308+
* @return A generated TSIG record or {@code null}.
309+
*/
310+
TSIGRecord getGeneratedTSIG() {
311+
return generatedTsig;
312+
}
313+
303314
/**
304315
* Was this message signed by a TSIG?
305316
*
@@ -325,9 +336,9 @@ public boolean isVerified() {
325336
* @see Section
326337
*/
327338
public OPTRecord getOPT() {
328-
for (Record record : getSection(Section.ADDITIONAL)) {
329-
if (record instanceof OPTRecord) {
330-
return (OPTRecord) record;
339+
for (Record r : getSection(Section.ADDITIONAL)) {
340+
if (r instanceof OPTRecord) {
341+
return (OPTRecord) r;
331342
}
332343
}
333344
return null;
@@ -516,6 +527,7 @@ private void toWire(DNSOutput out, int maxLength) {
516527
TSIGRecord tsigrec = tsigkey.generate(this, out.toByteArray(), tsigerror, querytsig);
517528

518529
tsigrec.toWire(out, Section.ADDITIONAL, c);
530+
generatedTsig = tsigrec;
519531
out.writeU16At(additionalCount + 1, startpos + 10);
520532
}
521533
}
@@ -536,9 +548,9 @@ public byte[] toWire() {
536548
/**
537549
* Returns an array containing the wire format representation of the Message with the specified
538550
* maximum length. This will generate a truncated message (with the TC bit) if the message doesn't
539-
* fit, and will also sign the message with the TSIG key set by a call to setTSIG(). This method
540-
* may return an empty byte array if the message could not be rendered at all; this could happen
541-
* if maxLength is smaller than a DNS header, for example.
551+
* fit, and will also sign the message with the TSIG key set by a call to {@link #setTSIG(TSIG,
552+
* int, TSIGRecord)}. This method may return an empty byte array if the message could not be
553+
* rendered at all; this could happen if maxLength is smaller than a DNS header, for example.
542554
*
543555
* <p>Do NOT use this method in conjunction with {@link TSIG#apply(Message, TSIGRecord)}, it
544556
* produces inconsistent results! Use {@link #setTSIG(TSIG, int, TSIGRecord)} instead.
@@ -556,6 +568,16 @@ public byte[] toWire(int maxLength) {
556568
return out.toByteArray();
557569
}
558570

571+
/**
572+
* Sets the TSIG key to sign a message.
573+
*
574+
* @param key The TSIG key.
575+
* @since 3.5.1
576+
*/
577+
public void setTSIG(TSIG key) {
578+
setTSIG(key, Rcode.NOERROR, null);
579+
}
580+
559581
/**
560582
* Sets the TSIG key and other necessary information to sign a message.
561583
*
@@ -668,6 +690,9 @@ public Message clone() {
668690
if (querytsig != null) {
669691
m.querytsig = (TSIGRecord) querytsig.cloneRecord();
670692
}
693+
if (generatedTsig != null) {
694+
m.generatedTsig = (TSIGRecord) generatedTsig.cloneRecord();
695+
}
671696
return m;
672697
}
673698

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public class SimpleResolver implements Resolver {
3636

3737
private InetSocketAddress address;
3838
private InetSocketAddress localAddress;
39-
private boolean useTCP, ignoreTruncation;
39+
private boolean useTCP;
40+
private boolean ignoreTruncation;
4041
private OPTRecord queryOPT = new OPTRecord(DEFAULT_EDNS_PAYLOADSIZE, 0, 0, 0);
4142
private TSIG tsig;
4243
private Duration timeoutValue = Duration.ofSeconds(10);
@@ -267,12 +268,13 @@ private Message parseMessage(byte[] b) throws WireParseException {
267268
}
268269
}
269270

270-
private void verifyTSIG(Message query, Message response, byte[] b, TSIG tsig) {
271+
private void verifyTSIG(Message query, Message response, byte[] b) {
271272
if (tsig == null) {
272273
return;
273274
}
274-
int error = tsig.verify(response, b, query.getTSIG());
275-
log.debug("TSIG verify: {}", Rcode.TSIGstring(error));
275+
int error = tsig.verify(response, b, query.getGeneratedTSIG());
276+
log.debug(
277+
"TSIG verify on message id {}: {}", query.getHeader().getID(), Rcode.TSIGstring(error));
276278
}
277279

278280
private void applyEDNS(Message query) {
@@ -431,7 +433,7 @@ CompletableFuture<Message> sendAsync(Message query, boolean forceTcp, Executor e
431433
return f;
432434
}
433435

434-
verifyTSIG(query, response, in, tsig);
436+
verifyTSIG(query, response, in);
435437
if (!tcp && !ignoreTruncation && response.getHeader().getFlag(Flags.TC)) {
436438
if (log.isTraceEnabled()) {
437439
log.trace(
@@ -466,8 +468,8 @@ private Message sendAXFR(Message query) throws IOException {
466468
response.getHeader().setFlag(Flags.AA);
467469
response.getHeader().setFlag(Flags.QR);
468470
response.addRecord(query.getQuestion(), Section.QUESTION);
469-
for (Record record : records) {
470-
response.addRecord(record, Section.ANSWER);
471+
for (Record r : records) {
472+
response.addRecord(r, Section.ANSWER);
471473
}
472474
return response;
473475
}

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ public TSIG(Name algorithm, String name, String key) {
296296
* @param key The shared key's data represented as a base64 encoded string.
297297
* @throws IllegalArgumentException The key name is an invalid name
298298
* @throws IllegalArgumentException The key data is improperly encoded
299-
* @see RFC8945
299+
* @see <a href="https://datatracker.ietf.org/doc/html/rfc8945">RFC8945</a>
300300
*/
301301
public TSIG(String algorithm, String name, String key) {
302302
this(algorithmToName(algorithm), name, key);
@@ -543,15 +543,15 @@ public byte verify(Message m, byte[] b, int length, TSIGRecord old) {
543543
* routine, Message.isVerified() may be called on this message.
544544
*
545545
* @param m The message to verify
546-
* @param b An array containing the message in unparsed form. This is necessary since TSIG signs
547-
* the message in wire format, and we can't recreate the exact wire format (with the same name
548-
* compression).
549-
* @param old If this message is a response, the TSIG from the request
546+
* @param messageBytes An array containing the message in unparsed form. This is necessary since
547+
* TSIG signs the message in wire format, and we can't recreate the exact wire format (with
548+
* the same name compression).
549+
* @param requestTSIG If this message is a response, the TSIG from the request
550550
* @return The result of the verification (as an Rcode)
551551
* @see Rcode
552552
*/
553-
public int verify(Message m, byte[] b, TSIGRecord old) {
554-
return verify(m, b, old, true);
553+
public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG) {
554+
return verify(m, messageBytes, requestTSIG, true);
555555
}
556556

557557
/**
@@ -560,18 +560,18 @@ public int verify(Message m, byte[] b, TSIGRecord old) {
560560
* routine, Message.isVerified() may be called on this message.
561561
*
562562
* @param m The message to verify
563-
* @param b An array containing the message in unparsed form. This is necessary since TSIG signs
564-
* the message in wire format, and we can't recreate the exact wire format (with the same name
565-
* compression).
566-
* @param old If this message is a response, the TSIG from the request
563+
* @param messageBytes An array containing the message in unparsed form. This is necessary since
564+
* TSIG signs the message in wire format, and we can't recreate the exact wire format (with
565+
* the same name compression).
566+
* @param requestTSIG If this message is a response, the TSIG from the request
567567
* @param fullSignature {@code true} if this message is the first of many in a TCP connection and
568568
* all TSIG variables (rfc2845, 3.4.2.) should be included in the signature. {@code false} for
569569
* subsequent messages with reduced TSIG variables set (rfc2845, 4.4.).
570570
* @return The result of the verification (as an Rcode)
571571
* @see Rcode
572572
* @since 3.2
573573
*/
574-
public int verify(Message m, byte[] b, TSIGRecord old, boolean fullSignature) {
574+
public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG, boolean fullSignature) {
575575
m.tsigState = Message.TSIG_FAILED;
576576
TSIGRecord tsig = m.getTSIG();
577577
if (tsig == null) {
@@ -580,7 +580,8 @@ public int verify(Message m, byte[] b, TSIGRecord old, boolean fullSignature) {
580580

581581
if (!tsig.getName().equals(name) || !tsig.getAlgorithm().equals(alg)) {
582582
log.debug(
583-
"BADKEY failure, expected: {}/{}, actual: {}/{}",
583+
"BADKEY failure on message id {}, expected: {}/{}, actual: {}/{}",
584+
m.getHeader().getID(),
584585
name,
585586
alg,
586587
tsig.getName(),
@@ -589,8 +590,8 @@ public int verify(Message m, byte[] b, TSIGRecord old, boolean fullSignature) {
589590
}
590591

591592
Mac hmac = initHmac();
592-
if (old != null && tsig.getError() != Rcode.BADKEY && tsig.getError() != Rcode.BADSIG) {
593-
hmacAddSignature(hmac, old);
593+
if (requestTSIG != null && tsig.getError() != Rcode.BADKEY && tsig.getError() != Rcode.BADSIG) {
594+
hmacAddSignature(hmac, requestTSIG);
594595
}
595596

596597
m.getHeader().decCount(Section.ADDITIONAL);
@@ -603,9 +604,9 @@ public int verify(Message m, byte[] b, TSIGRecord old, boolean fullSignature) {
603604

604605
int len = m.tsigstart - header.length;
605606
if (log.isTraceEnabled()) {
606-
log.trace(hexdump.dump("TSIG-HMAC message after header", b, header.length, len));
607+
log.trace(hexdump.dump("TSIG-HMAC message after header", messageBytes, header.length, len));
607608
}
608-
hmac.update(b, header.length, len);
609+
hmac.update(messageBytes, header.length, len);
609610

610611
DNSOutput out = new DNSOutput();
611612
if (fullSignature) {

‎src/main/java/org/xbill/DNS/tools/jnamed.java

Lines changed: 17 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public jnamed(String conffile) throws IOException, ZoneTransferException {
6464
FileInputStream fs;
6565
InputStreamReader isr;
6666
BufferedReader br;
67-
List<Integer> ports = new ArrayList<Integer>();
68-
List<InetAddress> addresses = new ArrayList<InetAddress>();
67+
List<Integer> ports = new ArrayList<>();
68+
List<InetAddress> addresses = new ArrayList<>();
6969
try {
7070
fs = new FileInputStream(conffile);
7171
isr = new InputStreamReader(fs);
@@ -76,9 +76,9 @@ public jnamed(String conffile) throws IOException, ZoneTransferException {
7676
}
7777

7878
try {
79-
caches = new HashMap<Integer, Cache>();
79+
caches = new HashMap<>();
8080
znames = new HashMap<>();
81-
TSIGs = new HashMap<Name, TSIG>();
81+
TSIGs = new HashMap<>();
8282

8383
String line;
8484
while ((line = br.readLine()) != null) {
@@ -127,21 +127,20 @@ public jnamed(String conffile) throws IOException, ZoneTransferException {
127127
}
128128
}
129129

130-
if (ports.size() == 0) {
130+
if (ports.isEmpty()) {
131131
ports.add(53);
132132
}
133133

134-
if (addresses.size() == 0) {
134+
if (addresses.isEmpty()) {
135135
addresses.add(Address.getByAddress("0.0.0.0"));
136136
}
137137

138-
for (Object address : addresses) {
139-
InetAddress addr = (InetAddress) address;
140-
for (Object o : ports) {
141-
int port = (Integer) o;
142-
addUDP(addr, port);
143-
addTCP(addr, port);
144-
System.out.println("jnamed: listening on " + addrport(addr, port));
138+
for (InetAddress address : addresses) {
139+
for (Integer o : ports) {
140+
int port = o;
141+
addUDP(address, port);
142+
addTCP(address, port);
143+
System.out.println("jnamed: listening on " + addrport(address, port));
145144
}
146145
}
147146
System.out.println("jnamed: running");
@@ -172,12 +171,7 @@ public void addTSIG(String algstr, String namestr, String key) throws IOExceptio
172171
}
173172

174173
public Cache getCache(int dclass) {
175-
Cache c = caches.get(dclass);
176-
if (c == null) {
177-
c = new Cache(dclass);
178-
caches.put(dclass, c);
179-
}
180-
return c;
174+
return caches.computeIfAbsent(dclass, Cache::new);
181175
}
182176

183177
public Zone findBestZone(Name name) {
@@ -197,7 +191,7 @@ public Zone findBestZone(Name name) {
197191
return null;
198192
}
199193

200-
public <T extends Record> RRset findExactMatch(Name name, int type, int dclass, boolean glue) {
194+
public RRset findExactMatch(Name name, int type, int dclass, boolean glue) {
201195
Zone zone = findBestZone(name);
202196
if (zone != null) {
203197
return zone.findExactMatch(name, type);
@@ -217,8 +211,7 @@ public <T extends Record> RRset findExactMatch(Name name, int type, int dclass,
217211
}
218212
}
219213

220-
<T extends Record> void addRRset(
221-
Name name, Message response, RRset rrset, int section, int flags) {
214+
void addRRset(Name name, Message response, RRset rrset, int section, int flags) {
222215
for (int s = 1; s <= section; s++) {
223216
if (response.findRRset(name, rrset.getType(), s)) {
224217
return;
@@ -403,6 +396,7 @@ byte[] doAXFR(Name name, Message query, TSIG tsig, TSIGRecord qtsig, Socket s) {
403396
try {
404397
s.close();
405398
} catch (IOException ex) {
399+
// ignore
406400
}
407401
return null;
408402
}
@@ -414,7 +408,6 @@ byte[] doAXFR(Name name, Message query, TSIG tsig, TSIGRecord qtsig, Socket s) {
414408
*/
415409
byte[] generateReply(Message query, byte[] in, Socket s) {
416410
Header header;
417-
boolean badversion;
418411
int maxLength;
419412
int flags = 0;
420413

@@ -515,13 +508,12 @@ public byte[] errorMessage(Message query, int rcode) {
515508
}
516509

517510
public void TCPclient(Socket s) {
518-
try {
511+
try (InputStream is = s.getInputStream()) {
519512
int inLength;
520513
DataInputStream dataIn;
521514
DataOutputStream dataOut;
522515
byte[] in;
523516

524-
InputStream is = s.getInputStream();
525517
dataIn = new DataInputStream(is);
526518
inLength = dataIn.readUnsignedShort();
527519
in = new byte[inLength];
@@ -544,11 +536,6 @@ public void TCPclient(Socket s) {
544536
} catch (IOException e) {
545537
System.out.println(
546538
"TCPclient(" + addrport(s.getLocalAddress(), s.getLocalPort()) + "): " + e);
547-
} finally {
548-
try {
549-
s.close();
550-
} catch (IOException e) {
551-
}
552539
}
553540
}
554541

0 commit comments

Comments
 (0)
0