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
111 changes: 111 additions & 0 deletions doc/TACACSPLUS_PASSKEY_ENCRYPTION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# 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)
- [Benifits](#benifits)
- [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, passkey will be encrypted is 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 showcase the enhanced capbalities of the existing submodules.


### Implementation details

The implementation stands on three key pillars.
1. OPENSSL toolkit is used for encryption / decryption
2. aes-128-cbc is the encoding format used for encryption / decryption
3. Device MAC address is used as a Password for encryption / decryption
Copy link
Collaborator

@venkatmahalingam venkatmahalingam Sep 13, 2023

Choose a reason for hiding this comment

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

We can easily decrypt and know the actual password if we know the device MAC address, right? any option for user to configure the password other than MAC-address?

Also, if I want to use the config_db (encrypted passkey) generated on one system to another (may be we forced to replace the failed node with same config), hope passkey cant be used on another device, what procedure should be followed for this use-case?

Copy link

@nmoray nmoray Sep 13, 2023

Choose a reason for hiding this comment

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

Yes @venkatmahalingam you are right. We can decrypt the passkey only if we knew the system MAC address. And if we allow the user to use any custom password then it will be bit difficult in terms of maintainability.

For instance, either we need to use some fixed cipher for encryption / decryption which will be more susceptible to the overall network breach. Alternatively, we need to save the password somewhere (for instance in redis). This is the reason I chosen system MAC as a password which will make every encrypted passkey unique within the network.

An encrypted passkey on one node will not be applicable or useable on another as hostcfgd will not be able to decrypt that passkey. To handle this sort of scenario, the Orchestrator / network admin has to create the config_db.json or the encrypted passkey for every node (considering the orchestrator have the list of system MAC of all the nodes).

Let's say for some debugging scenario, we need to use same config_db.json on another device, we can simply re-generate the encrypted passkey on the device where we have copied the config_db.json by running "config tacacs passkey <actual_passkey_in_plaintext>" command.



#### 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we also remove this password from all log files in the system?
I assume it must remain in the tacplus_nss_conf and also in PAM, but we must make sure that it is not exposed in other files.

Copy link

Choose a reason for hiding this comment

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

Yes @Yarden-Z. The passkey in plaintext format will only be visible in PAM config file. Else where it will be masked.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand, can we make sure to test that the key is not present in anywhere else in the system once it is configured?
Also, might be worth checking that the files where this key is present is not globally readable.
If it is and an un-privileged user can read it, then encrypting it in the DB will serve minimal purpose.


show tacacs
["TACPLUS global passkey configured Yes / No"]

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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


### Benifits

TACACS passkey encryption adds an extra layer of security to safeguard the passkey on each device throughout the network. Moreover, the utilization of MAC address-based encryption ensures that each network device possesses its distinct encrypted passkey. This strategy effectively mitigates the risk of a major network breach in case one of the devices is compromised.
Copy link
Contributor

Choose a reason for hiding this comment

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

This feature enforces password encryption across all tacacs password settings (both global and per-server passwords), correct?

Copy link

Choose a reason for hiding this comment

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

Yes, right.



### Testing Requirements

Need to add new / update the existing TACACS testcases to incorporate this new feature