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
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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.

[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.

7 changes: 7 additions & 0 deletions build_debian.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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.

## TODO: remote_user's login shell will be changed to cli shell.
sudo LANG=C chroot $FILESYSTEM_ROOT useradd -G docker "remote_user" -u 1001 -g 999 -c \
"remote user" -d /home/remote_user -m -s /bin/rbash
sudo LANG=C chroot $FILESYSTEM_ROOT useradd -G sudo,docker "remote_user_su" -u 1002 -g 1000 -c \
"remote sudo user" -d /home/remote_user_su -m -s /bin/bash

## Pre-install hardware drivers
sudo LANG=C chroot $FILESYSTEM_ROOT apt-get -y install \
firmware-linux-nonfree
Expand Down
10 changes: 10 additions & 0 deletions files/build_templates/sonic_debian_extension.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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.

sudo dpkg --root=$FILESYSTEM_ROOT -i target/debs/libtac2_1.4.1-1_amd64.deb
sudo dpkg --root=$FILESYSTEM_ROOT -i target/debs/libpam-tacplus_1.4.1-1_amd64.deb

# Install nss-tacplus
sudo dpkg --root=$FILESYSTEM_ROOT -i target/debs/libnss-tacplus_1.0.3-1_amd64.deb

# Copy aaa configuration files
sudo cp -f $IMAGE_CONFIGS/aaa/aaa.json $FILESYSTEM_ROOT/etc/sonic/

# Copy crontabs
sudo cp -f $IMAGE_CONFIGS/cron.d/* $FILESYSTEM_ROOT/etc/cron.d/

Expand Down
13 changes: 13 additions & 0 deletions files/image_config/aaa/aaa.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"debug": true,
"src_ip": "",
"authentication": {
"login": {
"pam_priority": [
"local"
]
}
},
"tacacs_server_list": {},
"fail_through": false
}
29 changes: 29 additions & 0 deletions rules/tacacs.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# libpam-tacplus packages

PAM_TACPLUS_VERSION = 1.4.1-1

LIBTAC2 = libtac2_$(PAM_TACPLUS_VERSION)_amd64.deb
$(LIBTAC2)_SRC_PATH = $(SRC_PATH)/tacacs/sonic-pam-tacplus
SONIC_DPKG_DEBS += $(LIBTAC2)

LIBPAM_TACPLUS = libpam-tacplus_$(PAM_TACPLUS_VERSION)_amd64.deb
$(LIBPAM_TACPLUS)_RDEPENDS += $(LIBTAC2)
$(LIBPAM_TACPLUS)_SRC_PATH = $(SRC_PATH)/tacacs/sonic-pam-tacplus
SONIC_DPKG_DEBS += $(LIBPAM_TACPLUS)
$(eval $(call add_derived_package,$(LIBTAC2),$(LIBPAM_TACPLUS)))

LIBTAC_DEV = libtac-dev_$(PAM_TACPLUS_VERSION)_amd64.deb
$(LIBTAC_DEV)_RDEPENDS += $(LIBTAC2)
$(eval $(call add_derived_package,$(LIBTAC2),$(LIBTAC_DEV)))

# libnss-tacplus packages

NSS_TACPLUS_VERSION = 1.0.3-1

LIBNSS_TACPLUS = libnss-tacplus_$(NSS_TACPLUS_VERSION)_amd64.deb
$(LIBNSS_TACPLUS)_DEPENDS += $(LIBTAC_DEV)
$(LIBNSS_TACPLUS)_RDEPENDS += $(LIBTAC2)
$(LIBNSS_TACPLUS)_SRC_PATH = $(SRC_PATH)/tacacs/sonic-nss-tacplus
SONIC_DPKG_DEBS += $(LIBNSS_TACPLUS)
$(eval $(call add_derived_package,$(LIBTAC2)))

2 changes: 1 addition & 1 deletion slave.mk
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ $(DOCKER_LOAD_TARGETS) : $(TARGET_PATH)/%.gz-load : .platform docker-start $$(TA
###############################################################################

# targets for building installers with base image
$(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : .platform onie-image.conf $$(addprefix $(DEBS_PATH)/,$$($$*_DEPENDS)) $$(addprefix $(DEBS_PATH)/,$$($$*_INSTALLS)) $(addprefix $(DEBS_PATH)/,$(INITRAMFS_TOOLS) $(LINUX_KERNEL) $(IGB_DRIVER) $(SONIC_DEVICE_DATA) $(SONIC_UTILS)) $$(addprefix $(TARGET_PATH)/,$$($$*_DOCKERS)) $$(addprefix $(PYTHON_WHEELS_PATH)/,$(SONIC_CONFIG_ENGINE))
$(addprefix $(TARGET_PATH)/, $(SONIC_INSTALLERS)) : $(TARGET_PATH)/% : .platform onie-image.conf $$(addprefix $(DEBS_PATH)/,$$($$*_DEPENDS)) $$(addprefix $(DEBS_PATH)/,$$($$*_INSTALLS)) $(addprefix $(DEBS_PATH)/,$(INITRAMFS_TOOLS) $(LINUX_KERNEL) $(IGB_DRIVER) $(SONIC_DEVICE_DATA) $(SONIC_UTILS) $(LIBPAM_TACPLUS) $(LIBNSS_TACPLUS)) $$(addprefix $(TARGET_PATH)/,$$($$*_DOCKERS)) $$(addprefix $(PYTHON_WHEELS_PATH)/,$(SONIC_CONFIG_ENGINE))
$(HEADER)
## Pass initramfs and linux kernel explicitly. They are used for all platforms
export initramfs_tools="$(DEBS_PATH)/$(INITRAMFS_TOOLS)"
Expand Down
2 changes: 2 additions & 0 deletions sonic-slave/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ RUN apt-get update && apt-get install -y \
python-yaml \
# For lockfile
procmail \
# For pam-tacplus build
autoconf-archive \
# For gtest
libgtest-dev \
cmake \
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-utilities
1 change: 1 addition & 0 deletions src/tacacs/sonic-nss-tacplus
Submodule sonic-nss-tacplus added at 93183d
1 change: 1 addition & 0 deletions src/tacacs/sonic-pam-tacplus
Submodule sonic-pam-tacplus added at 6aab5b