Skip to content

Fix handling of multi-message TSIG responses#300

Merged
ibauersachs merged 6 commits intomasterfrom
tsig-stream
Nov 4, 2023
Merged

Fix handling of multi-message TSIG responses#300
ibauersachs merged 6 commits intomasterfrom
tsig-stream

Conversation

@ibauersachs
Copy link
Member

@ibauersachs ibauersachs commented Oct 29, 2023 •
edited
Loading

Closes #295
Closes #297
Closes #298
Closes #299
Closes #301

@ibauersachs @frankarinnet @nguichon
Fix handling of multi-message TSIG responses
36a0c7f 
Closes #295
Closes #297
Closes #298
Closes #299

Co-authored-by: Frank Hill <frank@arin.net>
Co-authored-by: Nick Guichon <nickg@arin.net>
@ibauersachs ibauersachs mentioned this pull request Oct 29, 2023
Closed
@codecov
Copy link

codecov bot commented Oct 29, 2023 •
edited
Loading

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Comparison is base (e361d1e) 64.18% compared to head (1f28212) 65.53%.
Report is 21 commits behind head on master.

Additional details and impacted files 
@@             Coverage Diff              @@
##             master     #300      +/-   ##
============================================
+ Coverage     64.18%   65.53%   +1.34%     
- Complexity     2765     2834      +69     
============================================
  Files           178      184       +6     
  Lines         12546    12724     +178     
  Branches       1915     1944      +29     
============================================
+ Hits           8053     8339     +286     
+ Misses         4002     3876     -126     
- Partials        491      509      +18
Files Coverage Δ
src/main/java/org/xbill/DNS/TCPClient.java 11.95% <ø> (+11.95%) ⬆️
src/main/java/org/xbill/DNS/ZoneTransferIn.java 33.79% <61.53%> (+33.79%) ⬆️
src/main/java/org/xbill/DNS/TSIG.java 63.82% <78.88%> (+18.54%) ⬆️

... and 28 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@frankarinnet
Copy link
Contributor

frankarinnet commented Oct 30, 2023

I'm afraid its not quite right. In this case, there is still a bug with verify process (and a symmetric bug with the generate process). The signature of the TSIG record isn't being added at quite the right time (as usual only in the case of mixed signed/unsigned messages).

On the bright side, the bug is much more narrow at this point.

I've got a branch of 'tsig-stream' I'll submit shortly as another PR.

@frankarinnet frankarinnet mentioned this pull request Oct 30, 2023
Closed
@ibauersachs @frankarinnet
Fix order of signature in TSIG mac
781ebd9 
Co-authored-by: Frank Hill <frank@arin.net>
frankarinnet
frankarinnet reviewed Oct 31, 2023
View reviewed changes
@ibauersachs ibauersachs added this to the v3.5.3 milestone Nov 2, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.4% 81.4% Coverage
0.0% 0.0% Duplication

@ibauersachs ibauersachs merged commit c4261b7 into master Nov 4, 2023
@ibauersachs ibauersachs deleted the tsig-stream branch November 4, 2023 15:32
@ibauersachs ibauersachs restored the tsig-stream branch November 4, 2023 16:28
@ibauersachs ibauersachs deleted the tsig-stream branch November 4, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@frankarinnet frankarinnet frankarinnet left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.5.3

Development

Successfully merging this pull request may close these issues.

bug with TSIG verification of large responses

2 participants

@ibauersachs @frankarinnet