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.

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

Quic ietf 4967 v4 #7101

Closed
wants to merge 10 commits into from
Closed

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/4967
https://redmine.openinfosecfoundation.org/issues/5143

Describe changes:

  • Parses standardized QUIC IETF v1
  • bump up rust carte tls_parser already used by rep, so that it can be used by quic
  • Rustfmt previous code

suricata-verify-pr: 775
OISF/suricata-verify#775

Replaces #7095 with review taken into account

Still to do more generally about quic : see https://redmine.openinfosecfoundation.org/issues/4966

Should I tackle https://redmine.openinfosecfoundation.org/issues/5166 ? ie Support previous quic versions like Q039 and Q043
We get the right version for Q050 but rustls cannot then decrypt it...

Ticket: 4967

The format of initial packet for quic ietf, ie quic v1,
is described in rfc 9000, section 17.2.2
so that we can use new functions in quic parser
and logs interesting extensions from crypto frame
As it can be 4, but it can also be 1, based on the first
decrypted byte
The way to determine if the payload is encrypted is by storing
in the state if we have seen a crypto frame in both directions...
So as to keep parse not too big
@catenacyber catenacyber mentioned this pull request Mar 3, 2022
@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #7101 (1ee7792) into master (935ea74) will decrease coverage by 0.01%.
The diff coverage is 90.69%.

@@            Coverage Diff             @@
##           master    #7101      +/-   ##
==========================================
- Coverage   78.01%   77.99%   -0.02%     
==========================================
  Files         628      629       +1     
  Lines      185402   185443      +41     
==========================================
- Hits       144637   144634       -3     
- Misses      40765    40809      +44     
Flag Coverage Δ
fuzzcorpus 59.67% <38.46%> (-0.01%) ⬇️
suricata-verify 54.56% <85.71%> (-0.06%) ⬇️
unittests 63.15% <69.04%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@victorjulien
Copy link
Member

Something is still off in my testing. I have a large pcap, that when I just print versions in tshark, has these numbers per version output:

 328760 0x00000001
  83622 0x00000001,0x00000001
  56529 0xff00001d,0xff00001d
  40341 0xff00001d
  26329 0xfaceb002

In this branch, suricata has almost 3.5milllion unique version values:

 126536 "1"
  58347 "ff00001d"
  21606 "faceb002"
    182 "54"
    180 "Q046"
    120 "50"
     64 "a2158291"
     45 "555b32a0"
     35 "1000052"
     13 "921ae87e"
     13 "1b020000"
     10 "4e"
      7 "713ccc29"
      6 "e9907a02"
      6 "8086a215"
      6 "2fd05ba"
      6 "0"
      5 "e4fcfa42"
      5 "b5f4cd89"
      5 "b046f205"
      5 "6b6755db"
      5 "60"
      5 "4d"
.... many many more ...

@victorjulien
Copy link
Member

victorjulien commented Mar 4, 2022

Master for the same pcap just outputs

 368820 "1"
  75065 "ff00001d"
  21737 "faceb002"
    188 "Q046"

@catenacyber
Copy link
Contributor Author

Should I tackle https://redmine.openinfosecfoundation.org/issues/5166 ?

Something is still off in my testing.

TL;DR
You answered "yes" to my question

Long version :
Older quic versions do not have the same header fields, neither the same flags meaning for the first byte (sic)... So I guess we will need to do some probing to know the version, so as to know how to parse the header...

@victorjulien
Copy link
Member

Should I tackle https://redmine.openinfosecfoundation.org/issues/5166 ?

Something is still off in my testing.

TL;DR You answered "yes" to my question

Long version : Older quic versions do not have the same header fields, neither the same flags meaning for the first byte (sic)... So I guess we will need to do some probing to know the version, so as to know how to parse the header...

I think the issue for me is not that our support isn't complete, but that we somehow log garbage data.

@catenacyber catenacyber mentioned this pull request Mar 4, 2022
@catenacyber
Copy link
Contributor Author

Replaced by #7106

@catenacyber catenacyber closed this Mar 4, 2022
@catenacyber
Copy link
Contributor Author

that we somehow log garbage data.

Now I get

jq .quic.version log/eve.json | sort | uniq -c | sort -n
   2 "Q043"
   4 "Q039"
  66 "ff00001d"
 136 "51303530"
 175 null
 768 "Q046"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants