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
Prev Previous commit
Next Next commit
Treat tcp dumps as binary, extend Javadoc
  • Loading branch information
@ibauersachs
ibauersachs committed Nov 4, 2023
commit b3eff09f694e8ea288de1ad8a8836798b4c45922
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
* text eol=lf
*.bin binary
44 changes: 38 additions & 6 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 @@ -542,6 +542,8 @@ public void applyStream(Message m, TSIGRecord old, boolean fullSignature) {
* 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.
*
* <p>Use {@link StreamVerifier} to validate multiple messages in a stream.
*
* @param m The message
* @param b 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
Expand All @@ -562,6 +564,8 @@ public byte verify(Message m, byte[] b, int length, TSIGRecord old) {
* 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.
*
* <p>Use {@link StreamVerifier} to validate multiple messages in a stream.
*
* @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
Expand All @@ -579,6 +583,8 @@ public int verify(Message m, byte[] messageBytes, TSIGRecord requestTSIG) {
* 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.
*
* <p>Use {@link StreamVerifier} to validate multiple messages in a stream.
*
* @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
Expand Down Expand Up @@ -763,7 +769,11 @@ private static void writeTsigTime(Instant instant, DNSOutput out) {
out.writeU32(timeLow);
}

/** A helper class for generating signed message responses. */
/**
* A utility class for generating signed message responses.
*
* @since 3.5.3
*/
public static class StreamGenerator {
private final TSIG key;
private final Mac sharedHmac;
Expand All @@ -772,8 +782,16 @@ public static class StreamGenerator {
private int numGenerated;
private TSIGRecord lastTsigRecord;

/**
* Creates an instance to sign multiple message for use in a stream.
*
* <p>This class creates a {@link TSIGRecord} on every message to conform with <a
* href="https://www.rfc-editor.org/rfc/rfc8945.html#section-5.3.1">RFC 8945, 5.3.1</a>.
*
* @param key The TSIG key used to create the signature records.
* @param queryTsig The initial TSIG records, e.g. from a query to a server.
*/
public StreamGenerator(TSIG key, TSIGRecord queryTsig) {
// 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, queryTsig, 1);
}
Expand All @@ -793,7 +811,16 @@ public StreamGenerator(TSIG key, TSIGRecord queryTsig) {
sharedHmac = this.key.initHmac();
}

public void generate(Message message, boolean isLastMessage) {
/**
* Generate TSIG a signature for use of the message in a stream.
*
* @param message The message to sign.
*/
public void generate(Message message) {
generate(message, true);
}

void generate(Message message, boolean isLastMessage) {
boolean isNthMessage = numGenerated % signEveryNthMessage == 0;
boolean isFirstMessage = numGenerated == 0;
if (isFirstMessage || isNthMessage || isLastMessage) {
Expand All @@ -818,14 +845,16 @@ public void generate(Message message, boolean isLastMessage) {
}
}

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

private int nresponses;
private int lastsigned;

/** {@code null} or the detailed error when validation failed due to a {@link Rcode#FORMERR}. */
@Getter private String errorMessage;

/** Creates an object to verify a multiple message response */
Expand All @@ -839,7 +868,8 @@ public StreamVerifier(TSIG tsig, TSIGRecord queryTsig) {
/**
* Verifies a TSIG record on an incoming message that is part of a multiple message response.
* TSIG records must be present on the first and last messages, and at least every 100 records
* in between. After calling this routine, Message.isVerified() may be called on this message.
* in between. After calling this routine,{@link Message#isVerified()} may be called on this
* message.
*
* <p>This overload assumes that the verified message is not the last one, which is required to
* have a {@link TSIGRecord}. Use {@link #verify(Message, byte[], boolean)} to explicitly
Expand All @@ -858,13 +888,15 @@ public int verify(Message message, byte[] messageBytes) {
/**
* Verifies a TSIG record on an incoming message that is part of a multiple message response.
* TSIG records must be present on the first and last messages, and at least every 100 records
* in between. After calling this routine, Message.isVerified() may be called on this message.
* in between. After calling this routine, {@link Message#isVerified()} may be called on this
* message.
*
* @param message The message
* @param messageBytes The message in unparsed form
* @param isLastMessage If true, verifies that the {@link Message} has an {@link TSIGRecord}.
* @return The result of the verification (as an Rcode)
* @see Rcode
* @since 3.5.3
*/
public int verify(Message message, byte[] messageBytes, boolean isLastMessage) {
TSIGRecord tsig = message.getTSIG();
Expand Down
Binary file modified src/test/resources/tsig-axfr/response.bin
View file
Open in desktop
Binary file not shown.
0