Skip to content
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

IKEv2: improve dissection of IKEv2 redirect notifications #3925

Merged
merged 1 commit into from Mar 23, 2023
Merged

IKEv2: improve dissection of IKEv2 redirect notifications #3925

merged 1 commit into from Mar 23, 2023

Conversation

mspncp
Copy link
Contributor

@mspncp mspncp commented Mar 5, 2023

Checklist:

  • I added unit tests
  • I executed the regression tests (using cd test && ./run_tests or tox)

Replaces #3779

@mspncp mspncp mentioned this pull request Mar 5, 2023
Closed
2 tasks
@mspncp
Copy link
Contributor Author

mspncp commented Mar 5, 2023 •
edited

tox -e spell fails because the spell checker fails to spell check the key exchange payload?!

What I can I do to prevent it from spell checking bytes objects?

test/contrib/ikev2.uts:1532: WyA ==> way

scapy/test/contrib/ikev2.uts

Line 1532 in d6a5011

ke=b'F\x16\x84\x82\xfeS#?\xc1\xe2/\x97&\xb7\xad\xfe\r\xfc\xf5=\x15X\xfdf1h$\xce\xec2\xd4\xd3?WyA\xd3\xd5.\x92\x9b;\xed\x0b.\xef\x12\x88a\x17\xcd5\x86U\xf2\xf6\xff\xd6\xfbT\xfdH\xbb\xc5'

@codecov
Copy link

codecov bot commented Mar 5, 2023 •
edited

Codecov Report

Merging #3925 (eb97f7b) into master (d05de38) will increase coverage by 4.89%.
The diff coverage is 100.00%.

❗ Current head eb97f7b differs from pull request most recent head 3c5dcdb. Consider uploading reports for the commit 3c5dcdb to get more accurate results

@@            Coverage Diff             @@
##           master    #3925      +/-   ##
==========================================
+ Coverage   81.64%   86.53%   +4.89%     
==========================================
  Files         326      310      -16     
  Lines       75429    70598    -4831     
==========================================
- Hits        61582    61093     -489     
+ Misses      13847     9505    -4342
Impacted Files Coverage Δ
scapy/contrib/ikev2.py 95.41% <100.00%> (+0.48%) ⬆️

... and 53 files with indirect coverage changes

@gpotter2
Copy link
Member

gpotter2 commented Mar 5, 2023 •
edited

You can use b'\x57\x61\x59'

@mspncp mspncp marked this pull request as draft March 5, 2023 17:01
@mspncp
Copy link
Contributor Author

mspncp commented Mar 5, 2023

I overlooked something, see fixup commit. Since I edited the original tcpdump capture using the broken implementation, I need to fix the test.

@mspncp mspncp marked this pull request as ready for review March 5, 2023 22:30
@mspncp
Copy link
Contributor Author

mspncp commented Mar 5, 2023

The CI failure seems to be unrelated.

@mspncp mspncp requested a review from gpotter2 March 10, 2023 20:35
@mspncp
IKEv2: improve dissection of IKEv2 redirect notifications
3c5dcdb 
* improve dissection of IKEv2 redirect notifications

  See RFC 5685, section 9

* add some more missing notifications

  https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xml
@mspncp
Copy link
Contributor Author

mspncp commented Mar 16, 2023

Last force-push was a plain rebase on the master branch without further changes. CI is clean now.

@gpotter2 gpotter2 merged commit ef2a779 into secdev:master Mar 23, 2023
@gpotter2 gpotter2 added this to the 2.6.0 milestone Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpotter2 gpotter2 gpotter2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mspncp @gpotter2