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

TACACSPLUS_PASSKEY_ENCRYPTION.md #1471

Merged
merged 12 commits into from
May 22, 2024
87 changes: 87 additions & 0 deletions doc/TACACSPLUS_PASSKEY_ENCRYPTION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# TACACS+ Passkey Encryption #
## Table of Contents
- [Revision](#revision)
- [Scope](#scope)
- [Abbreviations](#abbreviations)
- [Overview](#overview)
- [Requirements](#requirements)
- [High-Level Design](#high-level-design)
- [Implementation Details](#implementation-details)
- [Show CLI changes](@show-cli-changes)
- [Benefits](#benefits)
- [Testing Requirements](#testing-requirements)
### Revision
| Rev | Date | Author | Change Description |
|:---:|:-----------:|:-----------------------:|:----------------------------------|
| 0.1 | | Nikhil Moray (nmoray) | Initial version |
| 0.1 | | Madhu Paluru (madhupalu)| Updated |
### Scope
This document describes the High Level Design of "TACACS+ Passkey Encryption" feature support in SONiC. It encompasses design and implementation considerations for enhancing the security of TACACS+ (Terminal Access Controller Access-Control System) authentication by introducing an encrypted passkey.
### Abbreviations
| Term | Meaning |
|:-------:|:-------------------------------------------------------|
| TACACS | Terminal Access Controller Access Control System Plus) |
### Overview
This addition constitutes a substantial improvement in bolstering the security of the TACACS+ authentication protocol. TACACS+ has a well-established reputation as a reliable means of managing access to network devices. However, the previous practice of utilising a sensitive passkey in plaintext during the authentication process posed security concerns. With this enhancement, the vulnerability is effectively mitigated by introducing robust passkey encryption mechanisms (on the client side), ensuring the safeguarding of authentication credentials and an overall strengthening of network security.
### Requirements
The primary objective of this feature is to safeguard the TACACS passkey, which is stored in its plaintext format within CONFIG_DB.
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 add the section for DB migrator?

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 add the YANG change for the new TACACS passkey?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@venkatmahalingam passkey is already part of yang model, no new schema introduced part of HLD - https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-system-tacacs.yang
Only the backend will use system mac as unique salt to encrypt/decrypt the passkey.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need DB migrator as no change to the schema.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How about existing plaintext passkey to encrypted key migration in the config-db during image upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@venkatmahalingam The current yang model description already reflect the change you requested - ' description "Shared secret used for encrypting the communication";' Is the sufficient or do we think still need an update?

leaf passkey {
type string {
length "1..65";
pattern "[^ #,]*" {
error-message 'TACACS shared secret (Valid chars are ASCII printable except SPACE, "#", and ",")';
}
}
description "Shared secret used for encrypting the communication";
}

Copy link

Choose a reason for hiding this comment

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

@venkatmahalingam is sonic-system-tacacs.yang a pull model or push? If we need to configure the passkey via this model, we don't need any change as it will still be in a plaintext format only.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nmoray , I think the length part need change, because when you encrypt a text to binary then convert to text, the length will usually be much longer, currently sonic already enable yang validation, if the length not change, yang validation will failed with some TACACS key.

Copy link

@nmoray nmoray Nov 2, 2023

Choose a reason for hiding this comment

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

@liuh-80 one question related to sonic-system-tacacs.yang model. Is it being used for pushing the TACACS configs or pulling the telemetry data related to TACACS or both?

And in case of increasing the length of passkey leaf, do I need to make any other backward compatibility changes or nothing?

Copy link
Contributor

Choose a reason for hiding this comment

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

sonic-system-tacacs.yang is used for push TACACS config, also when change TACACS config on device with CLI, the model also will be used for validation.

Increase the length of passkey is backward compatibility. No need other change.

### High-Level Design
In line with the TACACS+ architecture, when a user initiates a SSH connection to a device with TACACS+ authentication enabled, they must utilize the TACACS+ login password. Conversely, the corresponding device must have the passkey provided by the TACACS+ server properly configured within its configDB. Should either of these elements be missing or incorrect, the user will be unable to access the device. Thus to meet the given requirement, the passkey will be encrypted in the configuration phase itself.
The current data flow among the various TACACS modules operates in the following manner.
1. When a user configures the TACACS passkey using the SONIC CLI, it is initially stored in the CONFIG_DB.
2. Subsequently, the same key is retrieved by the HostCfg Enforcer module to update the PAM configuration file(s). This configuration file is inherently included in the authentication processes for login or SSH within the Linux operating system.
3. When TACACS+ Authentication is activated on the device, a new PAM configuration file (common-auth-sonic) is generated and substituted in the login and SSH daemons. Importantly, the pre-existing configuration file remains unchanged.
The revised data handling procedure among the modules is outlined as follows:
1. When a user configures the TACACS passkey using the SONIC CLI, it will now be securely stored in encrypted format instead of plaintext.
2. Subsequently, the HostCfg Enforcer module retrieves this encrypted key. However, before writing it into the PAM configuration file(s), the hostCfgd module decrypts it.
```
+-------+ +---------+
| SSH | | Console |
+---+---+ +----+----+
| |
+----------v-----------v---------+ +---------------------+
| AUTHENTICATION | | |
| +-------------------------+ | Decrypted passkey | +------------+ |
| | PAM Configuration Files <-----------------------------------------+ | AAA Config | |
| +-------------------------+ | | +------------+ |
| | | |
| +-------------+ | | HostCfg Enforcer |
| | PAM | | +----------^----------+
| | Libraries | | |
| +-------------+ | | Encrypted passkey
+---------------+----------------+ |
| |
+----v----+ +-------+--------+
| | Encrypted passkey | |
| CLI +------------------------------------------------------> ConifgDB |
| | | |
+---------+ +----------------+
```
This decryption step is crucial because the login or SSH daemon references the PAM config file to verify the TACACS secret / passkey. If it remains encrypted, the SSH daemon will be unable to recognize the passkey, leading to login failures. The depicted block diagram clearly showcases the enhanced capabilities of the existing submodules.
### Implementation details
The implementation stands on four key pillars.
1. OPENSSL toolkit is used for encryption / decryption.
2. aes-128-cbc is the encoding format used for encryption / decryption.
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
3. A unique device admin password (encrypted hash from linux shadow file) will be used as a salt/password to encrypt/decrypt the configured pass key in ConfigDB.
Copy link
Contributor

@liuh-80 liuh-80 Oct 18, 2023

Choose a reason for hiding this comment

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

How to handle password change?

There was a password hardening feature will make the password expired: https://github.com/sonic-net/SONiC/blob/master/doc/passw_hardening/hld_password_hardening.md?plain=1

#Closed

Copy link

Choose a reason for hiding this comment

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

Yeah, the encryption and decryption is solely based on the "admin" password only. If the password expires and it has been changed, the network admin needs to generate a new encrypted passkey using the new admin password hash present in the shadow file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nmoray , I think the encrypted key update should be automatically done when admin password changed.

If not, when the admin password changed, device can't connect with tacacs server anymore, which will break many automation service in production environment.

Copy link

@nmoray nmoray Nov 2, 2023

Choose a reason for hiding this comment

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

@liuh-80 Automating this would not be straight forward as we need to modify "passwd" linux command to achieve the same.

We can document a requirement that this implementation is based on the admin password present in shadow file. Thus, if the operator is planing to change the "admin" password, he / she has to propagate that change to all the devices. Normally, the operators have their own ZTP scripts to do the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

So there will be added effort to adjust the hash key and perform the encryption-decryption once we change the admin password?

Copy link
Contributor

Choose a reason for hiding this comment

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

The best approach would be to introduce an infra that can provide encryption/decryption as a service across SONiC which will also take care of the key management and storage. This way, all protocols can use the service without worrying about management of the key. The service must be accessible to applications across dockers and host.

Copy link

@nmoray nmoray Nov 8, 2023

Choose a reason for hiding this comment

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

The suggested approach can be made common across all the features. But are we fine with the encryption / decryption and password management approach?

Copy link

Choose a reason for hiding this comment

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

The best approach would be to introduce an infra that can provide encryption/decryption as a service across SONiC which will also take care of the key management and storage. This way, all protocols can use the service without worrying about management of the key. The service must be accessible to applications across dockers and host.

The solution probably consists in deploying some sort of vault that could safely store passwords, keys, certificates. SONiC should be able to create such a vault during the NOS installation, populate the vault using a script (typically for ZTP deployments) rather than waiting for user input on the CLI. Last but not least, SONiC should be able to manage the associated access rights (TACACS client should not be allowed to read the LDAP password, and vice versa).

Copy link
Collaborator

@venkatmahalingam venkatmahalingam Nov 9, 2023

Choose a reason for hiding this comment

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

The best approach would be to introduce an infra that can provide encryption/decryption as a service across SONiC which will also take care of the key management and storage. This way, all protocols can use the service without worrying about management of the key. The service must be accessible to applications across dockers and host.

@nmoray This design looks generic and works for all protocols (TACACS+, RADIUS, LDAP, BGP..etc) that need the encryption/decryption. Refer below diagram for some details.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@venkatmahalingam thank you, I have updated the HLD. Yes the design is generic, support all three protocols (TACACS+, RADIUS, LDAP, BGP..etc) that need the encryption/decryption. 202311 release is around the corner, could you please approve and merge the HLD, we will work on code PRs and get it done.

4. In absence of configured admin password (default behaviour), a fixed / hardcoded hash will be used as a salt/password for encryption/decryption and it will be saved locally not in any of the databases.
Following is the snippet captured from a sample shadow file.
<snip_from_shadow_file>
admin:$6$YTJ7JKnfsB4esnbS$5XvmYk2.GXVWhDo2TYGN2hCitD/wU9Kov.uZD8xsnleuf1r0ARX3qodIKiDsdoQA444b8IMPMOnUWDmVJVkeg1:19446:0:99999:7:::
<snip_from_shadow_file>
Here, "YTJ7JKnfsB4esnbS$5XvmYk2.GXVWhDo2TYGN2hCitD/wU9Kov.uZD8xsnleuf1r0ARX3qodIKiDsdoQA444b8IMPMOnUWDmVJVkeg1" is the encrypted version of the admin level password.
#### Show CLI changes
Furthermore, aside from encrypting the passkey stored within CONFIG_DB, this infrastructure ensures that the passkey itself remains concealed from any of the displayed CLI outputs. Consequently, the passkey field has been eliminated from the "show tacacs" output, and it will now solely indicate the status whether the passkey is configured or not. For instance,
liuh-80 marked this conversation as resolved.
Show resolved Hide resolved
show tacacs
["TACPLUS global passkey configured Yes / No"]
### DB migration
A DB migration script will be added for users to migrate existing config_db to convert tacacs passkey plaintext to encrypted.
Copy link
Contributor

@liuh-80 liuh-80 Nov 2, 2023

Choose a reason for hiding this comment

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

Can this feature be enable/disable by CONFIG_DB setting?

I'm warry about backward compatibility, if a production environment has hundreds of thousands of devices need OS upgrade, and they are installed different old version, upgrade is difficult and may cause livesite.

If there is a flag to disable this feature, then it's much easy to do the upgrade, also can keep backward compatibility.
#Closed

Copy link

Choose a reason for hiding this comment

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

@liuh-80 We can achieve the backward compatibility by following your suggestion. We will add check in hostcfgd if the already configured passkey is in plaintext or not. If it is in plantext (same as the one in common-auth-sonic), we will skip the decryption and proceed. This way, operator doesn't need to change anything if he / she upgrades the image. The logic will allow the user to login with existing TACACS credentials.

Copy link

Choose a reason for hiding this comment

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

@liuh-80 what are your thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK, please update design doc and code.
Also if the feature been disabled, there need some signal check, maybe hostcfgd write a warning/error syslog when render the TACACS config files.

### Benefits
TACACS passkey encryption adds an extra layer of security to safeguard the passkey on each device throughout the network. Furthermore, the implementation of shadow file-based encryption ensures that encrypted passkeys can be reused across network nodes without any complications. Consequently, there are no obstacles when it comes to utilizing the config_db.json file from one device on another. Additionally, the use of a shadow file effectively reduces the risk of exposing the encryption/decryption salt since it is only accessible to root users and remains inaccessible to external entities.
### Limitation
The chosen way to encrypt the passkey is using an already encrypted admin password from the shadow file. Thus, the network admin needs to regenerate the encrypted passkey in ConfigDB only if there is a change in the admin password of the network.
For now we are planning to go ahead with this approach. However we are open to new ideas and thoughts to harden the security and we should be able to accommodate that with later releases.
### Testing Requirements
Need to add new / update the existing TACACS test cases to incorporate this new feature
Test cases to unit test encrypt and decrypt functions
Test cases to add test the TACACS+ functionality with passkey encryption
Test cases to cover DB migration