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

[draft] JA4 for TLS and QUIC -- with raw support #10135

Closed
wants to merge 1 commit into from

Conversation

satta
Copy link
Contributor

@satta satta commented Jan 8, 2024

Previous PR: #10095

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/6379

Changes to previous PR:

  • Rebase against current master.
  • Implement JA4_r and JA4_ro fingerprints; also change metadata JSON structure to contain those.
  • Add detection keywords for ja4.r/ja4_r and ja4.ro/ja4_ro
  • Add caching for JA4 state to enable multiple hash checks for unchanged state without need for hash recalculation.

Just opening this unrefined code for comments; adjusted S-V will come once the shape of the output is OK.

This change changes the output of the JA4 metadata to a sub-object containing raw descriptors in addition to the hash (see https://github.com/FoxIO-LLC/ja4/blob/main/technical_details/JA4.md#raw-output):

{
  "timestamp": "2015-03-06T19:12:22.423355+0000",
  "flow_id": 1779718999422175,
  "pcap_cnt": 7,
  "event_type": "tls",
  "src_ip": "192.168.56.1",
  "src_port": 49365,
  "dest_ip": "192.168.56.101",
  "dest_port": 443,
  "proto": "TCP",
  "pkt_src": "wire/pcap",
  "tls": {
    "subject": "C=FR, ST=IDF, L=Paris, O=Stamus, CN=SELKS",
    "issuerdn": "C=FR, ST=IDF, L=Paris, O=Stamus, CN=SELKS",
    "serial": "00:97:E6:47:09:8E:EA:C9:B4",
    "fingerprint": "3a:0b:3b:23:15:2c:44:5c:27:ac:6a:0c:41:d6:fa:74:af:b4:09:5b",
    "version": "TLS 1.2",
    "notbefore": "2015-02-12T18:07:27",
    "notafter": "2025-02-09T18:07:27",
    "ja4": {
      "ja4": "t12i1810s1_27d4652c4487_06a4338d0495",
      "ja4_r": "t12i1810s1_0004,0005,000a,002f,0032,0033,0035,0039,009c,009e,c007,c009,c00a,c011,c013,c014,c02b,c02f_0005,000a,000b,000d,0012,0023,3374,7550,ff01_0401,0501,0201,0403,0503,0203,0402,0202",
      "ja4_ro": "t12i1810s1_c02b,c02f,009e,c00a,c009,c013,c014,c007,c011,0033,0032,0039,009c,002f,0035,000a,0005,0004_ff01,000a,000b,0023,3374,0010,7550,0005,0012,000d_0401,0501,0201,0403,0503,0203,0402,0202"
    }
  }
}

vs. reference implementation

$ ./ja4 -r -O /home/satta/tmp/suricata-verify/tests/ja4-tls/input.pcap
- stream: 0
  transport: tcp
  src: 192.168.56.1
  dst: 192.168.56.101
  src_port: 49365
  dst_port: 443
  ja4_o: t12i1810s1_3b36b92f2f7b_5a9fdfe0e9a5
  ja4_ro: t12i1810s1_c02b,c02f,009e,c00a,c009,c013,c014,c007,c011,0033,0032,0039,009c,002f,0035,000a,0005,0004_ff01,000a,000b,0023,3374,0010,7550,0005,0012,000d_0401,0501,0201,0403,0503,0203,0402,0202
  ja4s: t120300_c02f_bec8bdbaef8a
  ja4s_r: t120300_c02f_ff01,000b,0023
  tls_certs:
  - x509:
    - ja4x: 2bab15409345_2bab15409345_795797892f9c
      ja4x_r: 550406,550408,550407,55040a,550403_550406,550408,550407,55040a,550403_551d0e,551d23,551d13
      issuerCountryName: FR
      issuerStateOrProvinceName: IDF
      issuerLocalityName: Paris
      issuerOrganizationName: Stamus
      issuerCommonName: SELKS
      subjectCountryName: FR
      subjectStateOrProvinceName: IDF
      subjectLocalityName: Paris
      subjectOrganizationName: Stamus
      subjectCommonName: SELKS
  ja4l_c: 15_64
  ja4l_s: 437_64

I also got in touch with FoxIO to sort out some minor gaps in the JA4 specification (FoxIO-LLC/ja4#40).

@catenacyber
Copy link
Contributor

Should #10095 be closed in favor of this one ?

@satta
Copy link
Contributor Author

satta commented Jan 18, 2024

Should #10095 be closed in favor of this one ?

That depends on the feedback I am expecting to get from this draft PR :)

  • Is the additional raw fingerprint info useful as it is (maybe to @regit?)
  • Should the generation of the raw fingerprint be configurable?
  • Any comments on the caching?
  • ...

So maybe not yet, I wanted to keep #10095 to be "the" canonical PR and this to be a RfC more than a final proposal.

@catenacyber
Copy link
Contributor

The raw ja4 looks like we want a tls.extensions keyword...

@satta
Copy link
Contributor Author

satta commented Jan 18, 2024

Yes, I thought of this as well. In general it might be helpful to extend the EVE-JSON output with some more readable details and also add detection capabilities on those. Different PR maybe ;)

@regit
Copy link
Contributor

regit commented Jan 18, 2024

Should #10095 be closed in favor of this one ?

That depends on the feedback I am expecting to get from this draft PR :)

  • Is the additional raw fingerprint info useful as it is (maybe to @regit?)

It allows user to extract a posteriori some information such as TLS extensions so that can be really useful

  • Should the generation of the raw fingerprint be configurable?

I would make it optional.

  • Any comments on the caching?
  • ...

So maybe not yet, I wanted to keep #10095 to be "the" canonical PR and this to be a RfC more than a final proposal.

@catenacyber
Copy link
Contributor

  • Is the additional raw fingerprint info useful as it is (maybe to @regit?)

It allows user to extract a posteriori some information such as TLS extensions so that can be really useful

Why not allow logging of tls extensions then ? instead of hiding them in a ja4...

@satta
Copy link
Contributor Author

satta commented Jan 19, 2024

  • Is the additional raw fingerprint info useful as it is (maybe to @regit?)

It allows user to extract a posteriori some information such as TLS extensions so that can be really useful

Why not allow logging of tls extensions then ? instead of hiding them in a ja4...

I agree. I'd rather see the extension logging in QUIC and TLS being extended (and also maybe unified to output an identical format? 🤔) Any thoughts?

@victorjulien victorjulien marked this pull request as draft January 19, 2024 21:11
@victorjulien
Copy link
Member

  • Is the additional raw fingerprint info useful as it is (maybe to @regit?)

It allows user to extract a posteriori some information such as TLS extensions so that can be really useful

Why not allow logging of tls extensions then ? instead of hiding them in a ja4...

I agree. I'd rather see the extension logging in QUIC and TLS being extended (and also maybe unified to output an identical format? 🤔) Any thoughts?

I agree this is more interesting.

@victorjulien
Copy link
Member

(sorry fat fingered things, so reopened)

Copy link

codecov bot commented Jan 19, 2024

Codecov Report

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

Comparison is base (a37fa62) 82.15% compared to head (0e9f89f) 82.46%.
Report is 67 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10135      +/-   ##
==========================================
+ Coverage   82.15%   82.46%   +0.30%     
==========================================
  Files         974      980       +6     
  Lines      271925   281170    +9245     
==========================================
+ Hits       223394   231858    +8464     
- Misses      48531    49312     +781     
Flag Coverage Δ
fuzzcorpus 63.26% <57.64%> (+0.36%) ⬆️
suricata-verify 61.43% <54.46%> (-0.02%) ⬇️
unittests 63.50% <58.50%> (+0.65%) ⬆️

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

@catenacyber
Copy link
Contributor

I understand that #10095 rather than this PR should be the way to go, and that TLS extensions logging should get into another redmine ticket targeted to 8 (cf https://redmine.openinfosecfoundation.org/issues/2426 )

Is that correct ?
If so, we can close this PR...

@victorjulien
Copy link
Member

I understand that #10095 rather than this PR should be the way to go, and that TLS extensions logging should get into another redmine ticket targeted to 8 (cf https://redmine.openinfosecfoundation.org/issues/2426 )

Is that correct ? If so, we can close this PR...

Yes, agree.

@catenacyber
Copy link
Contributor

Closing in favor of #10095 and https://redmine.openinfosecfoundation.org/issues/6695

@satta
Copy link
Contributor Author

satta commented Jan 22, 2024

Thanks for taking this to a decision, fully in support 👍🏻

@satta satta deleted the 6379-ja4-raw branch January 22, 2024 13:25
@satta satta restored the 6379-ja4-raw branch January 22, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants