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

Added new functionality, new attribute, and new default config options #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kevinnelson-affirmed
Copy link

  • Allow a unique secret per server
  • Added a new "group" attribute that allows the server config to
    specify the group by name instead of GID.
  • New default config allows for minimal TACACS+ server config per user
    • Defaults are used if the TACACS+ reply doesn't include, UID, HOME,
      or SHELL
    • default-hashed-uid
      • If "yes", a default UID is hashed from username
        • Not guaranteed to be unique, but should be for most cases
    • default-home
      • If set, this is used as the default home prefix
        • Ex: "/home" would set the user's home to /home/
    • default-shell
      • If set, this is used as the default shell
  • Updated README.md and etc/tacplus.conf with info on new options

- Allow a unique secret per server
- Added a new "group" attribute that allows the server config to
  specify the group by name instead of GID.
- New default config allows for minimal TACACS+ server config per user
    - Defaults are used if the TACACS+ reply doesn't include, UID, HOME,
      or SHELL
    - default-hashed-uid
        - If "yes", a default UID is hashed from username
            - Not guaranteed to be unique, but should be for most cases
    - default-home
        - If set, this is used as the default home prefix
            - Ex: "/home" would set the user's home to /home/<username>
    - default-shell
        - If set, this is used as the default shell
- Updated README.md and etc/tacplus.conf with info on new options
@daveolson53
Copy link

daveolson53 commented Jul 15, 2019 via email

@kevinnelson-affirmed
Copy link
Author

Kevin Nelson notifications@github.com wrote:
• Allow a unique secret per server
I'm pretty sure a unique secret per server has been available for quite a while. Or perhaps I'm misremembering, and added that myself in my version.

In Ben's version, it only allowed a single secret to be configured for all servers.

• Added a new "group" attribute that allows the server config to specify the group by name instead of GID. • New default config allows for minimal TACACS+ server config per user □ Defaults are used if the TACACS+ reply doesn't include, UID, HOME, or SHELL □ default-hashed-uid ☆ If "yes", a default UID is hashed from username ○ Not guaranteed to be unique, but should be for most cases
Interesting approach. Is this constrained to 32 bits, or 64? I ran into some issues with > 16 bit uids and the sparse wtmp file, and system backups. Some people might have issues with that. Looking at the pull request, looks like it's 31 bits. You might want to provide an option to make it 16 bits (increasing collisions). Did you do any studies based on public usernames how significant hash collisions might be? Is it common for tacacs servers to supply group attributes (numbers or names) in their response? I've not encountered that.
Dave Olson olson@cumulusnetworks.com

I don't have any issues with the "last" command (which reads /var/log/wtmp) on my systems. I'm not sure what system backup you are using. I've read that older systems can have trouble with UID larger than 16 bits, but modern Linux systems should be fine with 32 bits. I found in my testing that the UID was interpreted as a signed integer and Linux didn't like negative UIDs, so I limited it to 31 bits. On systems that can't handle larger than 16 bit UIDs I would suggest not setting "default-hashed-uid" to "yes" as 16 bits would probably increase collisions too much.

I included the following URL in the source code above the name_hash function:
https://stackoverflow.com/questions/2351087/what-is-the-best-32bit-hash-function-for-short-strings-tag-names#

The following was included in the comments under the algorithm:
Yep we use this exact hashing function with MULTIPLIER = 37 for strings and paths. Works well for us and I've yet to encounter a collision issue even after 2 years ( of course there's no guarantee we won't though ) – zebrabox Feb 28 '10 at 14:05

Although not a definitive study, it is one data point that suggests the hash is good.

This version of nss_tacplus (Ben's version) requires provisioning on the tacacs server of UID, GID, SHELL, and HOME for each user. By adding the various default options, I was able to reduce the provisioning to just a GID or GROUP. I added the group option to make it more human readable than a GID.

Thanks for the comments. Do you know if Ben is still maintaining this repo?

Regards,
Kevin Nelson

@daveolson53
Copy link

daveolson53 commented Jul 16, 2019 via email

kevinnelson-affirmed and others added 3 commits May 29, 2020 13:02
…pletion and will cause the completion to hang if any of the TACACS+ servers are down.
config options:
mapped-group <remote group> <local group>
mapped-shell <remote group> <shell>

A mapped shell takes precedence over a SHELL returned by the TACACS+ server.

Changed the hashed UID lower bound from 2000 to 10000 to avoid conflicts
with local users.
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.

2 participants