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

Jump to bottom
Merged
merged 6 commits into from
Nov 4, 2023
Merged

Fix handling of multi-message TSIG responses #300

merged 6 commits into from
Nov 4, 2023

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

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 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

@frankarinnet frankarinnet frankarinnet left review comments

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
0