-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Changes from all commits
f04f1e3
54ae956
4c9eb13
7a1be53
7834f80
bdd5c35
79fb5b8
5e1026b
ce63177
ff37410
59755b4
a8fd011
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
# 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 | | ||
| 0.1 | 11/09/2023| Madhu Paluru (madhupalu)| Addressed review comments | | ||
### 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. | ||
### 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 | | | | key-store +----------^----------+ | ||
| | Libraries | | +----> Master key | __ | | ||
| +-------------+ | | | Manager |---|r | | Encrypted passkey | ||
+---------------+----------------+ | |securitycipher| |o | | | ||
| | +-------------+ |o | | | ||
+----v----+ | | |t | +------+--------+ | ||
| | | | -- | | | ||
| CLI +----------------+ +------------------------> ConfigDB | | ||
| | Encrypted passkey | | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix the typo. ConifgDB There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it. @venkatmahalingam can you merge this PR? or should I ask @zhangyanzhao ? |
||
+---------+ +----------------+ | ||
``` | ||
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. | ||
|
||
|
||
### Approach | ||
1. A runtime flag (via config_db) for Enable / disable feature: "key_encrypt" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yang model need update to include this change. because this is config_db change, please also add schema and yang model to this HLD. When device already has passkey and key_encrypt is false, then user set this flag to true, the passkey need encrypt in config DB, which component will do that? Also, how to rollout this feature? there still backward compatibility issue: The gap is passkey need change from plaintext to encrypted when set key_encrypt flag. Will load_minigraph command do the migration when apply the new config? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The passkey encryption in CONFIG_DB will be done during the configuration part (in aaa.py). After rollout, if the user wants to enable this feature, he / she needs to simply enable "key_encrypt" flag and re-configure the passkey so that it will get encrypted and further replaced the same in CONFIG_DB. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When user enable the "key_encrypt" flag, he / she needs to reconfigure the passkey. This is the requirement. Two steps are required to be followed if anyone wants to use this feature. But this will certainly not hamper the back-compatibility. It is backward compatible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is like, if we need to configure SVI, we first need to add a vlan interface and later mark it as L3 interface by assigning an IP. Two steps. :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, if we are using CLI to encrypt the passkey, it will take care of updating the same into CONFIG_DB. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You want me to add CLI to enable 'key_encrypt' flag for TACACS? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @liuh-80 I have now provided an option to set the key_encrypt flag in TACPLUS table via CLI. Please check the PR-Part I. Now user need to provide -e option for enabling encryption feature. If not used, it will internally mark key_encrypt flag as false. Thus, post rollout of this feature, that flag will automatically be added in during the configuration itself.
And as I mentioned in the previous message, hostcfgd will take care of handling the backward compatibility by doing the decryption only when key_encrypt flag is present and it is set to true. Please refer PR - Part II |
||
2. CLI will ask for a encryption master key/password while configuring the TACACS passkey and at the backend it will be stored at /etc/cipher_pass file which is accessible to only root user with read only permissions | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI :
HTH. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @ludal35 for the references from the popular NOSs. @qiluo-msft If we want to have unique encrypted key for every device, it is possible with or without changing the password too. But the pain point here is, if we have to vary the password for every device, network operator has to keep track of those many passwords (in case he / she wants to change it in the future). Note: Here the encryption is solely based on two things, original passkey (in plaintext) and the associated password. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Junos approach seems to be a good compromise:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ludal35 you mean single master password for all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nmoray Yes, a single master password for all secrets stored in the SONiC config file (TACACS / Radius server keys and maybe the SNMP communities/secrets as well). Regarding the OSPF/BGP neighbor passwords, I assume this is handled by FRR so this is probably out of the SONiC scope. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah @ludal35 as I mentioned in the community review meeting, I intentionally kept the design flexible so that either we can have single master password for all or can keep different passwords for different features. |
||
3. Same file will be read while decrypting the passkey at hostcfgd | ||
4. The infra (encrypt/decrypt and master key/password storage & retrieval) will be common for all the features like TACACS, RADIUS, LDAP etc.. | ||
|
||
### Implementation details | ||
The implementation as follows | ||
1. OPENSSL toolkit is used for encryption / decryption. | ||
2. base64 is the encoding format used for encryption / decryption. | ||
4. sonic_utilities extended to passkey encyption using the master key/passwd manager. | ||
5. User has to enter master key/passwd at the time of configuring the passkey, this is mandatory requirement only if "key_encrypt" run time flag is enabled. | ||
6. The encrypted passkey stored in config_db | ||
7. The master key/paswd used for encryption/decryption and will be stored in the same device with root access previleges (/etc/cipher_pass). | ||
8. HostCfg will use the master key/passwd to decrypt the encrypted passkey and further store it in PAM configuration files. | ||
|
||
#### CLI Changes | ||
config tacacs passkey TEST1 --encrypt | ||
Password: | ||
|
||
Note: It will ask for a master key/password only when '--encrypt' flag is set. | ||
|
||
#### 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"] | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a section for limitations/restrictions, mention that if the MAC address of the device is known, we could decrypt the key and add some thoughts for the future work if you are not planning to fix in the initial release. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed. Added a section to the HLD describing the limitations and release plan. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @venkatmahalingam the reason behind choosing system MAC as the encryption salt is, it will be known only to the network admin. Additionally, it will be readily accessible within the Redis so, can be pulled in different modules without hardcoding anything. Moreover, we could consider other alternate salt too but again it will be an overhead of creating that uniquely for every node and also maintaining the same some where so that it can be consumed during the orchestration process. |
||
### Yang Changes | ||
Increase existing passkey leaf length to 256. | ||
Create a new leaf for newly introduced run time flag 'key_encrypt'. | ||
|
||
### Config DB changes | ||
A new run time flag to enable/disable the tacacs passkey encryption feature - "key_encrypt". | ||
|
||
### Schema changes | ||
``` | ||
"TACPLUS": { | ||
"global": { | ||
"auth_type": "login", | ||
"key_encrypt": "true", | ||
"passkey": "<Entrypted_Passkey>" | ||
} | ||
} | ||
``` | ||
### Benefits | ||
TACACS passkey encryption adds an extra layer of security to safeguard the passkey on each device throughout the network. Furthermore, the implementation of master key/password manager 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 root protected config file effectively reduces the risk of exposing the encryption/decryption master key/passwd since it is only accessible to root users and remains inaccessible to external entities. | ||
|
||
### 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 | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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";
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.