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 Bash TACACS+ plugin for per-command authorization. #8715

Merged
merged 24 commits into from
Nov 13, 2021

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Sep 9, 2021

This pull request add a bash plugin for TACACS+ per-command authorization

Why I did it

  1. To support TACACS per command authorization, we check user command before execute it.
  2. Fix libtacsupport.so can't parse tacplus_nss.conf correctly issue:
    Support debug=on setting.
    Support put server address and secret in same row.
  3. Fix the parse_config_file method not reset server list before parse config file issue.

How I did it

The bash plugin will be called before every user command, and check user command with remote TACACS+ server for per-command authorization.

How to verify it

UT with CUnit cover all code in this plugin.
Also pass all current UT.

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

N/A

Description for the changelog

Add Bash TACACS+ plugin.

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

@yozhao101
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

src/tacacs/pam/Makefile Outdated Show resolved Hide resolved
rules/tacacs.mk Outdated Show resolved Hide resolved
ThirdPartyLicenses.txt Outdated Show resolved Hide resolved
ThirdPartyLicenses.txt Outdated Show resolved Hide resolved
@liuh-80 liuh-80 force-pushed the dev/liuh/bash_tacplus branch from a94028c to db26d95 Compare October 29, 2021 06:39
From: liuh-80 <58683130+liuh-80@users.noreply.github.com>
Date: Tue, 12 Oct 2021 10:09:10 +0800
Subject: [PATCH 3/4] Extract tacacs support functions into library.
Subject: [PATCH] Extract tacacs support functions into library.
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 30, 2021

Choose a reason for hiding this comment

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

Extract tacacs support functions into library.

What is the intention of changing this file? Is it relating to src/tacacs/bash_tacplus? #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some code bug in this patch file:

  1. When parse nss_tacplus.conf, can't get TACACS server passkey, because the file format little different with upstream file format.
  2. Not release tacacs server data before load tacacs config file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the code change in this file, please check this PR: https://github.com/liuh-80/pam_tacplus/pull/4/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1 comments about code position fixed in this patch file.
the other comments about debug code change replied, it's necessary for debug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to discussion, update the patch and this code to only support debug, which is the upstream project beahvior.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Discussed offline: we will not use pam_tacplus function to parse nss_tacplus config in future. Then we will abandon this patch.

@liuh-80 liuh-80 marked this pull request as ready for review November 8, 2021 08:52
@liuh-80 liuh-80 requested review from lguohan and xumia as code owners November 8, 2021 08:52
slave.mk Outdated Show resolved Hide resolved
slave.mk Show resolved Hide resolved
@liuh-80 liuh-80 merged commit ff09b8b into sonic-net:master Nov 13, 2021
liuh-80 added a commit to liuh-80/sonic-buildimage that referenced this pull request Jul 11, 2023
liuh-80 added a commit to liuh-80/sonic-buildimage that referenced this pull request Jul 11, 2023
qiluo-msft pushed a commit that referenced this pull request Jul 13, 2023
Fix libtacsupport.so  can't parse tacplus_nss.conf issue and not reset server list before parse config file issue.

##### Work item tracking
- Microsoft ADO **(number only)**: 24433713

#### Why I did it
1. Fix libtacsupport.so can't parse tacplus_nss.conf correctly issue:
            Support debug=on setting.
            Support put server address and secret in same row.
2. Fix the parse_config_file method not reset server list before parse config file issue.

#### How I did it
Fix libtacsupport.so  can't parse tacplus_nss.conf issue and not reset server list before parse config file issue.

#### How to verify it
UT with CUnit cover all code in this plugin.
Also pass all current UT.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Tested branch (Please provide the tested image version)
Extract tacacs support functions into library, this will share TACACS config file parse code with other project.
Also fix memory leak issue in parse config code.

- [ ]  SONiC.202012-15723.312602-e230e2d3e

#### Description for the changelog
Fix libtacsupport.so  can't parse tacplus_nss.conf issue and not reset server list before parse config file issue.
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.

4 participants