Fix tcpdump report error when tacacs enabled. #16372

liuh-80 · 2023-09-01T10:01:06Z

Fix tcpdump report error when tacacs enabled.

Why I did it

Fix tcpdump report error when tacacs enabled:
Sep 1 09:25:18.189395 vlab-01 ERR tcpdump: nss_tacplus: /etc/tacplus_nss.conf fopen failed
Sep 1 09:25:18.189606 vlab-01 ERR tcpdump: nss_tacplus: bad config or server line for nss_tacplus

This is because debian add a patch create AppArmor profile for resource access control. The profile need update to allow tcpdump access /etc/tacplus_nss.conf.

Work item tracking

Microsoft ADO: 17667308

How I did it

Modify tcpdump AppArmor profile, add new line to allow tcpdump access TACACS config file:

/etc/tacplus_nss.conf r,

How to verify it

Pass all UT.
Manually verify error fixed.

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

SONiC.master-16372.360223-d5d6c64c5
SONiC.202205-16610.367814-d52a71b48
SONiC.202211-16611.367823-11a8b8d55
SONiC.202305.388563-584c448b2

Description for the changelog

Fix tcpdump report error when tacacs enabled.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

lguohan · 2023-09-09T06:43:50Z

is this tested on 202205 branch?

lguohan · 2023-09-09T06:49:23Z

why tcpdump needs access to /etc/tacplus_nss.conf?

lguohan · 2023-09-09T06:53:56Z

the apparmor has local/usr.bin.tcpdump to allow local customization, suggest to use that mechanism instead of modifying this file which is a little bit fragile.

lguohan

suggest to override /etc/apparmor.d/local/usr.bin.tcpdump

liuh-80 · 2023-09-11T01:41:26Z

is this tested on 202205 branch?

Yes, this tested on 202205 branch by copy the modified profile.

liuh-80 · 2023-09-11T01:42:39Z

tacplus_nss

The code of tcpdump will invoke 'getpwnam' API, and nss-tacplus plugin hook this API for TACACS authentication.

liuh-80 · 2023-09-11T01:43:23Z

suggest to override /etc/apparmor.d/local/usr.bin.tcpdump

Fixed, validate with new image again and confirm the issue fixed.

lguohan · 2023-09-11T17:54:22Z

build_debian.sh

+# latest tcpdump control resource access with AppArmor.
+# override tcpdump profile to allow tcpdump access TACACS config file.
+if [ -f ${FILESYSTEM_ROOT}/etc/apparmor.d/local/usr.bin.tcpdump ]; then
+    sudo LANG=C chroot $FILESYSTEM_ROOT /bin/bash -c "echo -e '\n# for access tacacs config\n/etc/tacplus_nss.conf r,\n' > /etc/apparmor.d/local/usr.bin.tcpdump"


there is local file why to add some files in the buildimage, this does not seem to be the standard way.

This code is modifying the local file: '/etc/apparmor.d/local/usr.bin.tcpdump'
The local file been created by tcpdump install.

@lguohan , Could you please check this PR again? I reply your comments, current code does not add new file in buildimage, when install tcpdump, the package will install a local apparmer profile, and current code will check and modify the local apparmer profile.

the local apparmer profile is empty, right?

it is better to make the local file more deterministic

suggest to use this approach.

sudo cp files/dhcp/rfc3442-classless-routes $FILESYSTEM_ROOT/etc/dhcp/dhclient-exit-hooks.d

@lguohan , thanks, fixed according to your suggestion, please check.

build_debian.sh

qiluo-msft · 2023-09-20T19:26:34Z

@liuh-80 As the process, if you require backport to 202205, please make sure it could be clean cherry-pick to 202205 locally, test it locally, and add the version string into PR description "Tested branch (Please provide the tested image version)". This PR description process only applies to the earliest branch you want to backport.

In reply to: 1712434300

liuh-80 · 2023-09-21T07:09:47Z

@liuh-80 As the process, if you require backport to 202205, please make sure it could be clean cherry-pick to 202205 locally, test it locally, and add the version string into PR description "Tested branch (Please provide the tested image version)". This PR description process only applies to the earliest branch you want to backport.

In reply to: 1712434300

Fixed, verify this PR can clean cherry-pick to 202205, also tested image version updated.

StormLiangMS · 2023-10-07T14:49:03Z

@liuh-80 have you tested with 202305? Could you update in the description?

liuh-80 · 2023-10-18T01:58:28Z

@liuh-80 have you tested with 202305? Could you update in the description?

Verified with SONiC.202305.388563-584c448b2, PR description updated.

Fix tcpdump report error when tacacs enabled. Why I did it Fix tcpdump report error when tacacs enabled: Sep 1 09:25:18.189395 vlab-01 ERR tcpdump: nss_tacplus: /etc/tacplus_nss.conf fopen failed Sep 1 09:25:18.189606 vlab-01 ERR tcpdump: nss_tacplus: bad config or server line for nss_tacplus This is because debian add a patch create AppArmor profile for resource access control. The profile need update to allow tcpdump access /etc/tacplus_nss.conf. Work item tracking Microsoft ADO: 17667308 How I did it Modify tcpdump AppArmor profile, add new line to allow tcpdump access TACACS config file: /etc/tacplus_nss.conf r,

mssonicbld · 2023-11-02T14:44:39Z

Cherry-pick PR to 202305: #17077

liuh-80 marked this pull request as ready for review September 4, 2023 08:31

liuh-80 requested a review from lguohan as a code owner September 4, 2023 08:31

liuh-80 requested a review from qiluo-msft September 4, 2023 08:32

qiluo-msft previously approved these changes Sep 4, 2023

View reviewed changes

lguohan requested changes Sep 9, 2023

View reviewed changes

liuh-80 dismissed qiluo-msft’s stale review via 10b726f September 11, 2023 03:26

lguohan reviewed Sep 11, 2023

View reviewed changes

qiluo-msft reviewed Sep 12, 2023

View reviewed changes

build_debian.sh Outdated Show resolved Hide resolved

qiluo-msft previously approved these changes Sep 13, 2023

View reviewed changes

liuh-80 dismissed qiluo-msft’s stale review via ba8e89f September 21, 2023 01:16

Improve code by PR comments, and fix cherry-pick to 202205 issue

7dbb86c

liuh-80 force-pushed the dev/liuh/fix-tcpdump-tacacs branch from ba8e89f to 7dbb86c Compare September 21, 2023 01:22

lguohan approved these changes Sep 23, 2023

View reviewed changes

lguohan merged commit 11f5a75 into sonic-net:master Sep 23, 2023
17 checks passed

lguohan added Request for 202205 Branch Request for 202305 Branch Request for 202211 Branch labels Sep 23, 2023

k-v1 mentioned this pull request Sep 30, 2023

[build] add support for 2 stage rootfs build #15924

Merged

11 tasks

StormLiangMS added the Approved for 202305 Branch label Nov 2, 2023

mssonicbld added the Created PR to 202305 Branch label Nov 2, 2023

mssonicbld mentioned this pull request Nov 2, 2023

[action] [PR:16372] Fix tcpdump report error when tacacs enabled. #17077

Merged

13 tasks

mssonicbld added a commit that referenced this pull request Nov 2, 2023

[tacacs]: Fix tcpdump report error when tacacs enabled (#16372) (#17077)

fbf30ec

mssonicbld added Included in 202305 Branch and removed Created PR to 202305 Branch labels Nov 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tcpdump report error when tacacs enabled. #16372

Fix tcpdump report error when tacacs enabled. #16372

liuh-80 commented Sep 1, 2023 •

edited

Loading

lguohan commented Sep 9, 2023 •

edited by qiluo-msft

Loading

lguohan commented Sep 9, 2023

lguohan commented Sep 9, 2023

lguohan left a comment

liuh-80 commented Sep 11, 2023

liuh-80 commented Sep 11, 2023

liuh-80 commented Sep 11, 2023 •

edited

Loading

lguohan Sep 11, 2023

liuh-80 Sep 12, 2023

liuh-80 Sep 20, 2023

lguohan Sep 20, 2023

liuh-80 Sep 21, 2023

qiluo-msft commented Sep 20, 2023

liuh-80 commented Sep 21, 2023

StormLiangMS commented Oct 7, 2023

liuh-80 commented Oct 18, 2023

mssonicbld commented Nov 2, 2023

Fix tcpdump report error when tacacs enabled. #16372

Fix tcpdump report error when tacacs enabled. #16372

Conversation

liuh-80 commented Sep 1, 2023 • edited Loading

Why I did it

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

lguohan commented Sep 9, 2023 • edited by qiluo-msft Loading

lguohan commented Sep 9, 2023

lguohan commented Sep 9, 2023

lguohan left a comment

Choose a reason for hiding this comment

liuh-80 commented Sep 11, 2023

liuh-80 commented Sep 11, 2023

liuh-80 commented Sep 11, 2023 • edited Loading

lguohan Sep 11, 2023

Choose a reason for hiding this comment

liuh-80 Sep 12, 2023

Choose a reason for hiding this comment

liuh-80 Sep 20, 2023

Choose a reason for hiding this comment

lguohan Sep 20, 2023

Choose a reason for hiding this comment

liuh-80 Sep 21, 2023

Choose a reason for hiding this comment

qiluo-msft commented Sep 20, 2023

liuh-80 commented Sep 21, 2023

StormLiangMS commented Oct 7, 2023

liuh-80 commented Oct 18, 2023

mssonicbld commented Nov 2, 2023

liuh-80 commented Sep 1, 2023 •

edited

Loading

lguohan commented Sep 9, 2023 •

edited by qiluo-msft

Loading

liuh-80 commented Sep 11, 2023 •

edited

Loading