Skip to content

DHCP DoS Logger for DHCP DoS Mitigation Feature#18947

Merged
qiluo-msft merged 13 commits intosonic-net:masterfrom
asraza07:dhcp_dos_mitigation_logger
May 29, 2025
Merged

DHCP DoS Logger for DHCP DoS Mitigation Feature#18947
qiluo-msft merged 13 commits intosonic-net:masterfrom
asraza07:dhcp_dos_mitigation_logger

Conversation

@asraza07
Copy link
Contributor

Why I did it

Added code for new daemon process responsible for detecting and logging DHCP DoS attack attempts (violation of DHCP rate limit)

How I did it

Added service and handler files for new systemd process dhcp_dos_logger

How to verify it

tc show command is used to identify dropped packets due to rate limiting

@asraza07 asraza07 changed the title Buildimage Changes for DHCP DoS Mitigation Feature DHCP DoS Logger for DHCP DoS Mitigation Feature May 24, 2024
logger.log_info(f"Port {port}: Current DHCP drop counter is {dropped_count}")
drop_pkts[port] = dropped_count
else:
logger.log_warning(f"No new dropped packets found on port {port}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why no dropped packets (Line 47 and line 49 log_warning) is treated as abnormal but dropped packets is treated as expected(line 44 log_info)? With this logic, whether actual DHCP packet rate low than tc rate treated as abnormal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yaqiangz , thanks for pointing that out. I have fixed this in the code. The systemd process will only log when dropped packets are observed, and it will do so as a warning, not as info.

@asraza07
Copy link
Contributor Author

@yaqiangz @lguohan , kindly help review this PR, thanks in advance!

Copy link
Contributor

@yaqiangz yaqiangz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ridahanif96
Copy link
Contributor

Hi @qiluo-msft can you pls help review and merged this PR. HLD is merged.

@ridahanif96
Copy link
Contributor

@qiluo-msft pls help merge this one

@ridahanif96
Copy link
Contributor

@qiluo-msft please help review this PR


# Main handler function
def handler():
"""
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add test case and check test coverage? #WontFix

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test coverage required for handler? currently, as per practice we don't add test cases and coverage here. The same is followed by all other features in image_config.

@ridahanif96
Copy link
Contributor

Hi Qi,

Please review this PR.

@wajahatrazi
Copy link
Contributor

Hi @qiluo-msft / @xincunli-sonic

Changes have been made as per your suggestions, please review to merge this PR.

Regards

@wajahatrazi
Copy link
Contributor

Hi @qiluo-msft / @lguohan

Kindly review and help merge this PR.

We are reaching out weekly, requesting for the above. Please support.

Regards

@ridahanif96
Copy link
Contributor

@qiluo-msft help merge this PR, pending for long, unable to understand merge delay!

@zhangyanzhao
Copy link

@lguohan this PR has been approved by reviewers, can you please check if you are ok to merge it? Thanks.

@wajahatrazi
Copy link
Contributor

Hi @lguohan, Sir

Kindly review the PR, and help merge. Regards

@muhammadalihussnain
Copy link
Contributor

Hi @lguohan Kindly review the PR, and help merge. Regards

@wajahatrazi
Copy link
Contributor

Hello there, @zhangyanzhao . Can you please help us with this PR merge? Thanks for your support.

@muhammadalihussnain
Copy link
Contributor

@qiluo-msft please review and help us to merge this PR

@wajahatrazi
Copy link
Contributor

@qiluo-msft @yaqiangz @lguohan

Kindly support in review & merge. Pending for so long

@qiluo-msft qiluo-msft merged commit b00a8d6 into sonic-net:master May 29, 2025
yaqiangz added a commit to yaqiangz/sonic-buildimage that referenced this pull request Jun 3, 2025
StormLiangMS pushed a commit that referenced this pull request Jun 3, 2025
…2831)

This reverts commit b00a8d6.

Why I did it
Revert PR #18947 because below 2 issues, it would block PR check

dhcp_dos_logger fail to start due to permission issue
admin@sonic:/usr/lib/systemd/system$ sudo systemctl status dhcp_dos_logger.service
× dhcp_dos_logger.service - Log DHCP rate limit violations
     Loaded: loaded (/lib/systemd/system/dhcp_dos_logger.service; enabled-runtime; preset: enabled)
     Active: failed (Result: exit-code) since Tue 2025-06-03 05:26:49 UTC; 4s ago
   Duration: 1ms
    Process: 9932 ExecStart=/usr/bin/dhcp_dos_logger.py (code=exited, status=203/EXEC)
   Main PID: 9932 (code=exited, status=203/EXEC)

Jun 03 05:26:49 sonic systemd[1]: Started dhcp_dos_logger.service - Log DHCP rate limit violations.
Jun 03 05:26:49 sonic (ogger.py)[9932]: dhcp_dos_logger.service: Failed to locate executable /usr/bin/dhcp_dos_logger.py: Permission denied
Jun 03 05:26:49 sonic (ogger.py)[9932]: dhcp_dos_logger.service: Failed at step EXEC spawning /usr/bin/dhcp_dos_logger.py: Permission denied
Jun 03 05:26:49 sonic systemd[1]: dhcp_dos_logger.service: Main process exited, code=exited, status=203/EXEC
Jun 03 05:26:49 sonic systemd[1]: dhcp_dos_logger.service: Failed with result 'exit-code'.
Even if grant permission to this file, it would still fail with below error
admin@sonic:/usr/bin$ sudo chmod 755 dhcp_dos_logger.py 
admin@sonic:/usr/bin$
admin@sonic:/usr/bin$ sudo systemctl restart dhcp_dos_logger.service 
admin@sonic:/usr/bin$ sudo systemctl status dhcp_dos_logger.service 
× dhcp_dos_logger.service - Log DHCP rate limit violations
     Loaded: loaded (/lib/systemd/system/dhcp_dos_logger.service; enabled-runtime; preset: enabled)
     Active: failed (Result: exit-code) since Tue 2025-06-03 05:29:38 UTC; 4s ago
   Duration: 85ms
    Process: 11312 ExecStart=/usr/bin/dhcp_dos_logger.py (code=exited, status=1/FAILURE)
   Main PID: 11312 (code=exited, status=1/FAILURE)

Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:     handler()
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:   File "/usr/bin/dhcp_dos_logger.py", line 39, in handler
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:     match = re.search(r'dropped (\d+)', output.stdout)
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:   File "/usr/lib/python3.11/re/__init__.py", line 176, in search
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:     return _compile(pattern, flags).search(string)
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]:            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Jun 03 05:29:38 sonic dhcp_dos_logger.py[11312]: TypeError: cannot use a string pattern on a bytes-like object
Jun 03 05:29:38 sonic systemd[1]: dhcp_dos_logger.service: Main process exited, code=exited, status=1/FAILURE
Jun 03 05:29:38 sonic systemd[1]: dhcp_dos_logger.service: Failed with result 'exit-code'.
admin@sonic:/usr/bin$
@StormLiangMS
Copy link
Contributor

hi @asraza07 this PR causing PR check failure, we get this reverted by below PR, pls check the failure and recommit when it passed the test.

#22831

@yaqiangz
Copy link
Contributor

yaqiangz commented Jun 9, 2025

hi @asraza07 this PR causing PR check failure, we get this reverted by below PR, pls check the failure and recommit when it passed the test.

#22831

@ridahanif96 for vis

@ridahanif96
Copy link
Contributor

@muhammadalihussnain Please look into this. Revert is being done but provide fix for this permission issue. Thanks

muhammadalihussnain added a commit to muhammadalihussnain/sonic-buildimage that referenced this pull request Jun 23, 2025
GaladrielZhao added a commit to GaladrielZhao/sonic-buildimage that referenced this pull request Mar 12, 2026
SONiC 202511 should use FRR 10.4.1 Release.
This commit updates submodule sonic-frr pointing to @88f5c06:
    FRR Release 10.4.1
    Bug Fixes:

    * bgpd: initialize local variable (backport sonic-net#19233)
    * ospfd: Use after free cleanup of lsa (backport sonic-net#19224)
    * vtysh: copy config from file should actually apply (backport sonic-net#19242)
    * Revert PR sonic-net#18358: BGP evpn testing and bug fixes related to non default EVPN backbone  (backport sonic-net#19241)
    * topotests: improve embedded RP test reliability (backport sonic-net#19240)
    * lib, zebra: mark singleton nexthops inactive/active on link state changes for wecmp (backport sonic-net#18947)
    * bgpd: LL next-hop capabilty fixes (backport sonic-net#19261)
    * eigrp: validate hello packets and tlvs better (backport sonic-net#19251)
    * bgpd: Fix compilation error in bgpd module: Update TP_ARGS for bgp (backport sonic-net#19266)
    * bgpd: Ensure addpath does not withdraw selected route in some situations (backport sonic-net#19210)
    * bgpd: [GR] fixed selectionDeferralTimer to display select_defer_time val by sonic-net#19282
    * bgpd: LL next-hop capabilty fixes (round 2) (backport sonic-net#19277)
    * lib: compute link-state zapi message size (backport sonic-net#19290)
    * zebra: Fix buffer overflows found by fuzzing. (backport sonic-net#19303)

    Signed-off-by: Jafar Al-Gharaibeh <[email protected]>

Signed-off-by: Yuqing Zhao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants