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

[TACACS+]: Add TACACS+ Authentication #746

Closed
wants to merge 9 commits into from

Conversation

liuqu
Copy link

@liuqu liuqu commented Jun 23, 2017

Signed-off-by: Liuqu chenchen.qcc@alibaba-inc.com

六曲 added 4 commits June 23, 2017 00:25
Summary:
* Add pam-tacplus module
* Add nss-tacplus module
* Add TACACS+ make rules and install script
* Add TACACS+ build dependence packages
* Add remote_user and remote_user_su
* Add AAA configuration file
.gitmodules Outdated
@@ -66,3 +66,9 @@
[submodule "platform/broadcom/sonic-platform-modules-accton"]
path = platform/broadcom/sonic-platform-modules-accton
url = https://github.com/edge-core/sonic-platform-modules-accton.git
[submodule "src/tacacs/sonic-pam-tacplus"]
path = src/tacacs/sonic-pam-tacplus
url = https://github.com/liuqu/sonic-pam-tacplus.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can see only one patch is made. It is better maintian it as a patch, like what we did for libteam.

Can you follow that as an example?

https://github.com/Azure/sonic-buildimage/tree/master/src/libteam

ttps://github.com/liuqu/sonic-pam-tacplus/commit/6aab5b6cafb65763e297f99e889418677528cd35

Copy link
Author

Choose a reason for hiding this comment

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

ok, I will change it.

.gitmodules Outdated
url = https://github.com/liuqu/sonic-pam-tacplus.git
[submodule "src/tacacs/sonic-nss-tacplus"]
path = src/tacacs/sonic-nss-tacplus
url = https://github.com/liuqu/sonic-nss-tacplus.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you maintain this as a patch?

Copy link
Author

Choose a reason for hiding this comment

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

This is a NSS plugin for TACACS+. Do you mean to maintain it as a patch for pam-tacplus?

Copy link
Author

Choose a reason for hiding this comment

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

I will think about how to change it as a patch to make.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Take https://github.com/Azure/sonic-buildimage/tree/master/src/initramfs-tools as an example. The patching method is recommended if your changes is small.

build_debian.sh Outdated
@@ -158,6 +158,13 @@ sudo LANG=C chroot $FILESYSTEM_ROOT useradd -G sudo,docker $USERNAME -c "$DEFAUL
## Create password for the default user
echo $USERNAME:$PASSWORD_ENCRYPTED | sudo LANG=C chroot $FILESYSTEM_ROOT chpasswd -e

## Create remote user
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you explain why we need this remote user?

Copy link
Author

Choose a reason for hiding this comment

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

It's used for user role map. If an authenticated user only exists in TACACS+ server database, not exists in local, it can't get passwd info without user role map. So I create remote user for TACACS+ user.

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. It is confusing for all other users not using TACACS+.
  2. Do we need to install rbash on SONiC host?
  3. The username, uid, gid are hard-coded. Are they wellknown in TACACS+?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your review.

  1. You're right. I will integrate the users' creation for TACACS+ Authentication into TACACS+ command. When the user enable TACACS+ by command, the remote users will be created.
  2. We don't need to install rbash. It's used for limit the TACACS+ user who only have show privilege. We will replace it with the CLI shell.
  3. The nss plugin for TACACS+ provides the getpwnam_r() entry point. It parses the privilege level of TACACS+ user by connecting with TACACS+ server, and maps to the pre-added user. If a use's priv-level is 15, the nss plugin will return the passwd info of remote_user_su in /etc/passwd.

@@ -89,6 +89,16 @@ sudo cp -f $IMAGE_CONFIGS/bash/bash.bashrc $FILESYSTEM_ROOT/etc/
sudo dpkg --root=$FILESYSTEM_ROOT -i target/debs/sonic-device-data_*.deb || \
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y install -f

# Install pam-tacplus
Copy link
Collaborator

@qiluo-msft qiluo-msft Jul 1, 2017

Choose a reason for hiding this comment

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

Is it possible to capsulate the TACACS+ functionality into a docker container like docker-vas? It is not an easy job but technically feasible.

Copy link
Author

Choose a reason for hiding this comment

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

The pam_tacplus is only a dynamic library for Linux PAM module, and nss-tacplus is a dynamic library for Linux NSS module. I think there's no need to capsulate them into a docker container.

Liuqu added 4 commits July 26, 2017 11:54
* Delete sonic-nss-tacplus and sonic-pam-tacplus submodule
* The nss-tacplus plugin is applied as patch for pam-tacplus
* Create remote user in aaa_config when TACACS+ enable, not default
  in build image
* Remove TACACS+ default config file, load default config by
  aaa config
* Modify the default user mapping policy, create local user for
  each TACACS+ user.
* Add configuration to change the default gid, group and shell of
  the local user to be created.
* Delete build dependence 'autoconf-archive'
@sonic-net sonic-net deleted a comment from msftclas Sep 26, 2017
@sonic-net sonic-net deleted a comment from msftclas Sep 27, 2017
@liuqu liuqu closed this Oct 6, 2017
@liuqu liuqu deleted the features/tacacs+ branch October 6, 2017 16:34
lguohan pushed a commit that referenced this pull request Dec 3, 2019
[config] Add 'feature' subcommand (#746)
Fix a bug in idempotent check. (#755)
lguohan pushed a commit to Sabareesh-Kumar-Anandan/sonic-buildimage that referenced this pull request Dec 19, 2020
[vs] Skip MACsec clean up if /sbin/ip is not accessible (sonic-net#750)
Configure enable -Wcast-align=strict when supported by compiler (sonic-net#749)
[syncd] Translate depreacated attr enum values to new ones (sonic-net#746)
[sairedis]vs SAI support for voq neighbor (sonic-net#725)

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>
lguohan pushed a commit that referenced this pull request Dec 19, 2020
[vs] Add workaround for clean up macsec ports (#752)
[logfile]: Add handling of Sairedis rec filename (#747)
Update README.md
[meta] Fix stat_mode enums to sai_bulk_op_error_mode_t (#753)
[syncd][tests] Add syncd deprecated attribute value test (#751)
[vs] Skip MACsec clean up if /sbin/ip is not accessible (#750)
Configure enable -Wcast-align=strict when supported by compiler (#749)
[syncd] Translate depreacated attr enum values to new ones (#746)
[sairedis]vs SAI support for voq neighbor (#725)
[syncd] Translate removed RIDs in fdb notification (#734)
[syncd] Move syncd classes to syncd namespace (#742)
[vs] Use /sbin/ip absolute path for ip command in MACsecManager (#744)
[saidiscovery] Update saidiscovery to use VendorSai object and metadata (#736)
Remove Winline warning since it depends on external headers (#741)
[meta] Enable strict cast-align warning (#738)
[vs] Use meta class instead info when using unittests (#740)
[vs] Support flush entry type all on virtual switch (#735)
[vslib]: Add MACsec state to state base (#722)
[README.md] Update installation steps (#730)
Switch Capability support (#728)
[vs] Fail switch create when warm boot requested and no warm boot state (#739)
Dynamic Port breakout fix the crash, port down event processing after<80> (#727)
Code clean (#721)

Signed-off-by: Sabareesh Kumar Anandan <sanandan@marvell.com>
stepanblyschak pushed a commit to stepanblyschak/sonic-buildimage that referenced this pull request May 10, 2021
AidanCopeland pushed a commit to Metaswitch/sonic-buildimage that referenced this pull request Apr 14, 2022
…#746)

If attribute is enum, and we are loading some older SAI values it may happen that we get deprecated/ignored value string and we need to update it to current one to not cause attribute compare confusion since they are compared by string value.

This fixes issues when warm shutdown on 201811 branch, and warm boot on master branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants