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

rust/ldap: udp and frames support v2 #11535

Closed
wants to merge 2 commits into from
Closed

Conversation

glongo
Copy link
Contributor

@glongo glongo commented Jul 19, 2024

Make sure these boxes are signed before submitting your Pull Request -- thank you.

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

Describe changes:

SV_BRANCH=OISF/suricata-verify#1984

This permits to parse and log ldap over udp traffic.
This adds a pdu frame for both request and response, and removes invalid
returns in SCLdapParseRequest and SCLdapParseResponse.
@glongo glongo requested review from jasonish, victorjulien and a team as code owners July 19, 2024 17:01
@victorjulien victorjulien added this to the 8.0 milestone Jul 20, 2024
@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ASAN_TLPR1_suri.

Pipeline 21667

@victorjulien victorjulien removed this from the 8.0 milestone Jul 20, 2024
@victorjulien
Copy link
Member

suricata: app-layer-parser.c:1373: int AppLayerParserParse(ThreadVars *, AppLayerParserThreadCtx *, Flow *, AppProto, uint8_t, const uint8_t *, uint32_t): Assertion `!((f->proto != IPPROTO_TCP))' failed.
Error: suricata: stacktrace:sig 6:pthread_kill+0x0000012c;raise+0x00000016;abort+0x000000d3;__libc_freeres+0x000000d3;__assert_fail+0x00000046;AppLayerParserParse+0x000012e9;AppLayerHandleUdp+0x0000113c;FlowWorker+0x00000ff1;TmThreadsSlotVarRun+0x00000226;TmThreadsSlotVar+0x00000f56;pthread_condattr_setpshared+0x00000513;__xmknodat+0x00000230 [SignalHandlerUnexpected:suricata.c:327]

@victorjulien
Copy link
Member

With a protocol like dns, yaml looks like this:

    dns:
      tcp:                                                                                
        enabled: yes             
        detection-ports:           
          dp: 53                                        
      udp:                                                                         
        enabled: yes                                                          
        detection-ports:                                         
          dp: 53                         

Why is this not similar for ldap here?

@victorjulien victorjulien self-requested a review July 20, 2024 10:37
@glongo
Copy link
Contributor Author

glongo commented Jul 22, 2024

With a protocol like dns, yaml looks like this:

    dns:
      tcp:                                                                                
        enabled: yes             
        detection-ports:           
          dp: 53                                        
      udp:                                                                         
        enabled: yes                                                          
        detection-ports:                                         
          dp: 53                         

Why is this not similar for ldap here?

This configuration seems to be used only for DNS, whereas other protocols (such as SIP and KRB) are not.
How should this be handled?

@glongo
Copy link
Contributor Author

glongo commented Jul 22, 2024

suricata: app-layer-parser.c:1373: int AppLayerParserParse(ThreadVars *, AppLayerParserThreadCtx *, Flow *, AppProto, uint8_t, const uint8_t *, uint32_t): Assertion `!((f->proto != IPPROTO_TCP))' failed.
Error: suricata: stacktrace:sig 6:pthread_kill+0x0000012c;raise+0x00000016;abort+0x000000d3;__libc_freeres+0x000000d3;__assert_fail+0x00000046;AppLayerParserParse+0x000012e9;AppLayerHandleUdp+0x0000113c;FlowWorker+0x00000ff1;TmThreadsSlotVarRun+0x00000226;TmThreadsSlotVar+0x00000f56;pthread_condattr_setpshared+0x00000513;__xmknodat+0x00000230 [SignalHandlerUnexpected:suricata.c:327]

Is there a way to reproduce this issue?

@victorjulien
Copy link
Member

suricata: app-layer-parser.c:1373: int AppLayerParserParse(ThreadVars *, AppLayerParserThreadCtx *, Flow *, AppProto, uint8_t, const uint8_t *, uint32_t): Assertion `!((f->proto != IPPROTO_TCP))' failed.
Error: suricata: stacktrace:sig 6:pthread_kill+0x0000012c;raise+0x00000016;abort+0x000000d3;__libc_freeres+0x000000d3;__assert_fail+0x00000046;AppLayerParserParse+0x000012e9;AppLayerHandleUdp+0x0000113c;FlowWorker+0x00000ff1;TmThreadsSlotVarRun+0x00000226;TmThreadsSlotVar+0x00000f56;pthread_condattr_setpshared+0x00000513;__xmknodat+0x00000230 [SignalHandlerUnexpected:suricata.c:327]

Is there a way to reproduce this issue?

I don't have a reproducer to share, sorry. Private data.

@victorjulien
Copy link
Member

With a protocol like dns, yaml looks like this:

    dns:
      tcp:                                                                                
        enabled: yes             
        detection-ports:           
          dp: 53                                        
      udp:                                                                         
        enabled: yes                                                          
        detection-ports:                                         
          dp: 53                         

Why is this not similar for ldap here?

This configuration seems to be used only for DNS, whereas other protocols (such as SIP and KRB) are not. How should this be handled?

I think we need the dns approach.

@glongo
Copy link
Contributor Author

glongo commented Jul 26, 2024

With a protocol like dns, yaml looks like this:

    dns:
      tcp:                                                                                
        enabled: yes             
        detection-ports:           
          dp: 53                                        
      udp:                                                                         
        enabled: yes                                                          
        detection-ports:                                         
          dp: 53                         

Why is this not similar for ldap here?

This configuration seems to be used only for DNS, whereas other protocols (such as SIP and KRB) are not. How should this be handled?

I think we need the dns approach.

Yep, already implemented that approach. PR coming.

@glongo
Copy link
Contributor Author

glongo commented Jul 26, 2024

Replaced with #11558

@glongo glongo closed this Jul 26, 2024
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.

3 participants