-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[TACACS+]: Add support for TACACS+ Authentication #1019
Conversation
* pam_tacplus - A TACACS+ protocol client library and PAM module to supports core TACACS+ functions for AAA. * nss_tacplus - A NSS plugin for TACACS+ to extend function getpwnam, make the TACACS+ authenticated user which is not found in local could login successfully. * Add make rules for pam_tacplus and install script * Add a patch for pam_tacplus to disable pam-auth-update pam-tacplus by default * Add a patch for pam_tacplus to inlucde and build nss_tacplus * hostcfgd - configDB enforcer for TACACS+, listen configDB to modify the pam configuration for Authentication in host * Add a service script for hostcfgd Signed-off-by: chenchen.qcc@alibaba-inc.com
Build finished. No test results found. |
@@ -0,0 +1,51 @@ | |||
From 80c1d3c1810bf283bbe12fc927de24e48afc2991 Mon Sep 17 00:00:00 2001 | |||
From: Liuqu <chenchen.qcc@alibaba-inc.com> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest not to change the way the patch libpam-tacplus, instead, we can disable the pam-tacplus by default in our image. the idea to minimize the change to the original package as little as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some scripts to disable the pam-tacplus after install libpam-tacplus?
|
||
[Service] | ||
Type=simple | ||
ExecStart=/usr/local/bin/hostcfgd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this hostcfgd, what is the usage of this hostcfgd? I cannot find this file in the PR. I suggest to have a separate PR for hostcfgd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hostcfgd has been created as another PR in sonic-utilities sonic-net/sonic-utilities#125
@@ -0,0 +1,1071 @@ | |||
From af404e6b4e2eddf529aad5f20ba6e0ee816ffdbf Mon Sep 17 00:00:00 2001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the libnss-tacplus is from https://github.com/daveolson53/libnss-tacplus
since this is a different repo, we should track it separately.
under src directory, we should have pam-tacplus and nss-tacplus directory to track each source separately.
nss-tacplus should also use the same approach as pam-tacplus, in the source repo, we should only track the patch instead of maintaining all the source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I misunderstood your reivew at the last PR #746
I agree with the new src directory for nss-tacplus. But our requirements for nss plugin are different from the original repo, such as creating local user for TACACS+ user, not need libtacplus-map. These are many changes in the soucre code. Can we use a new repo?
src/tacacs/Makefile
Outdated
rm -rf ./pam_tacplus | ||
git clone https://github.com/jeroennijhof/pam_tacplus.git | ||
pushd ./pam_tacplus | ||
git checkout -f v1.5.0-beta.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we use beta version? can we use a stable version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some build error at last stable version v1.4.1 in sonic build environment, and this beta version has fix them. To minimize the change, this beta version is chosen.
I will change it to the stable version v1.4.1 and add the patches to make it build successfully.
Two commits can fix the building error, but it will produce the conflict when they're applied to v1.4.1 directly. Some unnecessary commits have to be applied to resolve the conflict. Can we create two new commits as patch to minimize the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as comments
* Separate nss-tacplus from pam-tacplus, modify tacacs.mk and makefile, add a patch to adapt to the new user map profile. * Use the lastest stable version for pam-tacplus, add a dependent package in sonic-salve, add two patches to fix build error. * Add scripts to disable tacplus by default. * Remove hostcfgd service file Signed-off-by: Chenchen Qi <chenchen.qcc@alibaba-inc.com>
Already finished the changes, please review it again. Thanks
|
* The NAME_REGEX for username check in plugin nss-tacplus is the ANSI version "^[0-9a-zA-Z_-\ ]*$", but the regular expression in /etc/adduser.conf is not defined as ANSI version. To avoid nss-tacplus filter some valid TACACS+ username, remove username check. Signed-off-by: Chenchen Qi <chenchen.qcc@alibaba-inc.com>
slave.mk
Outdated
@@ -391,7 +390,9 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \ | |||
$(LINUX_KERNEL) \ | |||
$(IGB_DRIVER) \ | |||
$(SONIC_DEVICE_DATA) \ | |||
$(SONIC_UTILS)) \ | |||
$(SONIC_UTILS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing backslash
slave.mk
Outdated
@@ -391,7 +390,9 @@ $(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : \ | |||
$(LINUX_KERNEL) \ | |||
$(IGB_DRIVER) \ | |||
$(SONIC_DEVICE_DATA) \ | |||
$(SONIC_UTILS)) \ | |||
$(SONIC_UTILS) | |||
$(LIBPAM_TACPLUS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing backslash
[sairedis]: ARM32 bit fixes, for 64bit printf format specifier (#490) fix wrong API type , add cmdline parse option , remove unuseful api call (#499) Use pointer to member function to reduce the footprint of collectCounters() (#488) [syncd] Add support for Innovium platform (#496) [saisdkdump]: Fix dump generation crash (#493) [swss]: Add scope attribute to VNET table. (#954) [portsyncd]: Fix portsyncd restart case (#1019) [vnet]: Enable "vnet_orch_1" VS test case (#1020) [vlan] Add pytest cases to validate the behavior about add LAG member to vlan. (#875) [policerorch]: Add the capability to update policer rate/size (#1017) [aclorch]: Add MIRRORv6 support for NPS platform (#1018) [orchagent]: Avoid crash by setting g_syncMode flag before create_switch (#1014) [orchagent] Add support for Innovium platform (#1005) Provide broadcast IP while configuring interface ip address (#1007) [VLAN] Add pytest cases to validate different use-case of tagging_mode. (#860) [vstest]: Add test_port_config.py which include breakout port test. (#866) [vstests]: change 'test_acl_egress_table.py' to UNIX format (#1010) [vlan] Add pytest cases to verify data in app db and state db. (#895) [vstest]: Add the CRM tests (#1013) [warm-reboot]: add bgp eoiu support to speed up route reconcile (#856) [fdborch] support mac update (#877) [vxlanorch] Fix Logic of Vxlan tunnel removal (#995) Fix VLAN error introduced with new 4.9 kernel behavior (#1001) Config DB manual is being moved from Wiki to SWSS repo (#1002) Trap DHCPv6 packets for supporting ZTP over in-band interfaces using … (#997) Signed-off-by: Andriy Kokhan <akokhan@barefootnetworks.com>
I noticed that after swss restart in VS orchagent does not receive PortInitDone. I looked at the comment about "g_portSet" says that: When this LinkSync class is * initialized, we check the database to see if some of the ports' host * interfaces are already created and remove them from this set. However g_portSet was filled after LinkSync is initialized, so I considered this is a bug causing orchagent does not receive PortInitDone when portsyncd starts after host interfaces were created. Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
[sairedis]: ARM32 bit fixes, for 64bit printf format specifier (sonic-net#490) fix wrong API type , add cmdline parse option , remove unuseful api call (sonic-net#499) Use pointer to member function to reduce the footprint of collectCounters() (sonic-net#488) [syncd] Add support for Innovium platform (sonic-net#496) [saisdkdump]: Fix dump generation crash (sonic-net#493) [swss]: Add scope attribute to VNET table. (sonic-net#954) [portsyncd]: Fix portsyncd restart case (sonic-net#1019) [vnet]: Enable "vnet_orch_1" VS test case (sonic-net#1020) [vlan] Add pytest cases to validate the behavior about add LAG member to vlan. (sonic-net#875) [policerorch]: Add the capability to update policer rate/size (sonic-net#1017) [aclorch]: Add MIRRORv6 support for NPS platform (sonic-net#1018) [orchagent]: Avoid crash by setting g_syncMode flag before create_switch (sonic-net#1014) [orchagent] Add support for Innovium platform (sonic-net#1005) Provide broadcast IP while configuring interface ip address (sonic-net#1007) [VLAN] Add pytest cases to validate different use-case of tagging_mode. (sonic-net#860) [vstest]: Add test_port_config.py which include breakout port test. (sonic-net#866) [vstests]: change 'test_acl_egress_table.py' to UNIX format (sonic-net#1010) [vlan] Add pytest cases to verify data in app db and state db. (sonic-net#895) [vstest]: Add the CRM tests (sonic-net#1013) [warm-reboot]: add bgp eoiu support to speed up route reconcile (sonic-net#856) [fdborch] support mac update (sonic-net#877) [vxlanorch] Fix Logic of Vxlan tunnel removal (sonic-net#995) Fix VLAN error introduced with new 4.9 kernel behavior (sonic-net#1001) Config DB manual is being moved from Wiki to SWSS repo (sonic-net#1002) Trap DHCPv6 packets for supporting ZTP over in-band interfaces using … (sonic-net#997) Signed-off-by: Andriy Kokhan <akokhan@barefootnetworks.com>
pam_tacplus - A TACACS+ protocol client library and PAM module to
supports core TACACS+ functions for AAA.
nss_tacplus - A NSS plugin for TACACS+ to extend function getpwnam,
make the TACACS+ authenticated user which is not found in local
could login successfully.
Add make rules for pam_tacplus and install script
Add a patch for pam_tacplus to disable pam-auth-update pam-tacplus
by default
Add a patch for pam_tacplus to inlucde and build nss_tacplus
hostcfgd - configDB enforcer for TACACS+, listen configDB to
modify the pam configuration for Authentication in host
Add a service script for hostcfgd
Signed-off-by: chenchen.qcc@alibaba-inc.com