Integrated Security Cipher for TACPLUS passkey encryption #3936

nmoray · 2025-06-25T12:17:59Z

What I did
Made use of Security Cipher module to encrypt the TACPLUS passkey.
How I did it
Implemented the feature by following HLD
How to verify it
Unit test logs:

1. Encrypt passkey:
sonic# config tacacs passkey nikhil --encrypt
sonic# show run al | jq .TACPLUS
{
  "global": {
    "auth_type": "login",
    "key_encrypt": "True",
    "passkey": "U2FsdGVkX19YTUcG+0r3+d78A5E0zTXEukS3ZNrfyFk=",
    "src_intf": "Loopback0"
  }
}
sonic# cat /etc/pam.d/common-auth-sonic
#THIS IS AN AUTO-GENERATED FILE
#
# /etc/pam.d/common-auth- authentication settings common to all services
# This file is included from other service-specific PAM config files,
# and should contain a list of the authentication modules that define
# the central authentication scheme for use on the system
# (e.g., /etc/shadow, LDAP, Kerberos, etc.). The default is to use the
# traditional Unix authentication mechanisms.
#
# here are the per-package modules (the "Primary" block)

auth	[success=done new_authtok_reqd=done default=ignore auth_err=die]	pam_tacplus.so server=<> secret=nikhil login=login timeout=5   try_first_pass
auth	[success=done new_authtok_reqd=done default=ignore auth_err=die]	pam_tacplus.so server=<> secret=nikhil login=login timeout=5   try_first_pass
auth	[success=1 default=ignore]	pam_unix.so nullok try_first_pass

#
# here's the fallback if no module succeeds
auth    requisite                       pam_deny.so
# prime the stack with a positive return value if there isn't one already;
# this avoids us returning an error just because nothing sets a success code
# since the modules above will each just jump around
auth    required                        pam_permit.so
# and here are more per-package modules (the "Additional" block)
sonic# cat /etc/cipher_pass.json
{
  "TACPLUS": {
    "table_info": [
      "TACPLUS|global"
    ],
    "password": "TEST1"
  }

2. Encrypt passkey and update existing password
sonic# config tacacs passkey nikhil --encrypt --rotate
Password:
sonic# cat /etc/cipher_pass.json
{
  "TACPLUS": {
    "table_info": [
      "TACPLUS|global"
    ],
    "password": "TEST3"
  }
sonic# cat /etc/pam.d/common-auth-sonic
#THIS IS AN AUTO-GENERATED FILE
#
# /etc/pam.d/common-auth- authentication settings common to all services
# This file is included from other service-specific PAM config files,
# and should contain a list of the authentication modules that define
# the central authentication scheme for use on the system
# (e.g., /etc/shadow, LDAP, Kerberos, etc.). The default is to use the
# traditional Unix authentication mechanisms.
#
# here are the per-package modules (the "Primary" block)

auth	[success=done new_authtok_reqd=done default=ignore auth_err=die]	pam_tacplus.so server=<> secret=nikhil login=login timeout=5   try_first_pass
auth	[success=done new_authtok_reqd=done default=ignore auth_err=die]	pam_tacplus.so server=<> secret=nikhil login=login timeout=5   try_first_pass
auth	[success=1 default=ignore]	pam_unix.so nullok try_first_pass

#
# here's the fallback if no module succeeds
auth    requisite                       pam_deny.so
# prime the stack with a positive return value if there isn't one already;
# this avoids us returning an error just because nothing sets a success code
# since the modules above will each just jump around
auth    required                        pam_permit.so
# and here are more per-package modules (the "Additional" block)
sonic# show run al | jq .TACPLUS
{
  "global": {
    "auth_type": "login",
    "key_encrypt": "True",
    "passkey": "U2FsdGVkX18/8nFrAiCe01pOqgPoUUZFaTpmtawdi8I=",
    "src_intf": "Loopback0"
  }
}

3. Revert back to plaintext passkey
sonic# config tacacs passkey ravi
sonic# cat /etc/pam.d/common-auth-sonic
#THIS IS AN AUTO-GENERATED FILE
#
# /etc/pam.d/common-auth- authentication settings common to all services
# This file is included from other service-specific PAM config files,
# and should contain a list of the authentication modules that define
# the central authentication scheme for use on the system
# (e.g., /etc/shadow, LDAP, Kerberos, etc.). The default is to use the
# traditional Unix authentication mechanisms.
#
# here are the per-package modules (the "Primary" block)

auth	[success=done new_authtok_reqd=done default=ignore auth_err=die]	pam_tacplus.so server=<> secret=ravi login=login timeout=5   try_first_pass
auth	[success=done new_authtok_reqd=done default=ignore auth_err=die]	pam_tacplus.so server=<> secret=ravi login=login timeout=5   try_first_pass
auth	[success=1 default=ignore]	pam_unix.so nullok try_first_pass

#
# here's the fallback if no module succeeds
auth    requisite                       pam_deny.so
# prime the stack with a positive return value if there isn't one already;
# this avoids us returning an error just because nothing sets a success code
# since the modules above will each just jump around
auth    required                        pam_permit.so
# and here are more per-package modules (the "Additional" block)
sonic# show run al | jq .TACPLUS
{
  "global": {
    "auth_type": "login",
    "key_encrypt": "False",
    "passkey": "ravi",
    "src_intf": "Loopback0"
  }
}
sonic# cat /etc/cipher_pass.json
{
  "TACPLUS": {
    "table_info": [],
    "password": null
  }

Related PR(s)
PR#17201
PR#22711
PR#81

Note: This PR is created by closing the old PR due to several merge conflicts.

mssonicbld · 2025-06-25T12:18:07Z

/azp run

azure-pipelines · 2025-06-25T12:18:18Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-06-25T12:22:38Z

@anders-nexthop @qiluo-msft Please review the updated integration of Security Cipher module with TACPLUS feature.

mssonicbld · 2025-06-26T04:40:01Z

/azp run

azure-pipelines · 2025-06-26T04:40:11Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-06-26T04:41:59Z

@anders-nexthop please help in adding the reviewers.

linux-foundation-easycla · 2025-06-26T05:00:52Z

The committers listed above are authorized under a signed CLA.

✅ login: nmoray-ebay / name: Nikhil Moray (df0cbec, a12b028, 92201bc, 2e8e79a, b70ad67, f01c202, d3b1ce2, ca28b5c)

mssonicbld · 2025-06-26T05:00:56Z

/azp run

azure-pipelines · 2025-06-26T05:01:06Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-26T05:23:27Z

/azp run

azure-pipelines · 2025-06-26T05:23:37Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-26T05:26:47Z

/azp run

azure-pipelines · 2025-06-26T05:26:56Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-26T05:31:03Z

/azp run

azure-pipelines · 2025-06-26T05:31:12Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-26T05:47:24Z

/azp run

azure-pipelines · 2025-06-26T05:47:35Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-26T05:49:51Z

/azp run

azure-pipelines · 2025-06-26T05:50:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-27T11:50:07Z

/azp run

azure-pipelines · 2025-06-27T11:50:17Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-27T13:21:05Z

/azp run

azure-pipelines · 2025-06-27T13:21:15Z

Azure Pipelines successfully started running 1 pipeline(s).

anders-nexthop

Needs some work still, but seems much cleaner after your changes to the underlying key_encrypt system.

anders-nexthop · 2025-09-10T16:22:12Z

config/aaa.py

 VALID_CHARS_MSG = "Valid chars are ASCII printable except SPACE, '#', and ','"
+TACACS_PASSKEY_MAX_LEN = 65
+
+secure_cipher = master_key_mgr(security_cipher_clbk_lookup)


where is security_cipher_cblk_lookup defined?

anders-nexthop · 2025-09-10T16:22:36Z

config/aaa.py

 def login(db, auth_protocol):
    """Switch login authentication [ {ldap, radius, tacacs+, local} | default ]"""
-    if len(auth_protocol) is 0:
+    if len(auth_protocol) == 0:


seems unrelated?

anders-nexthop · 2025-09-10T16:29:50Z

config/aaa.py

+                add_table_kv('TACPLUS', 'global', 'passkey', b64_encoded)
+            else:
+                # Deregister feature with Security Cipher module
+                secure_cipher.deregister("TACPLUS", rotate_tacplus_key)


why would we deregister if the encryption failed? also, where is rotate_tacplus_key defined?

anders-nexthop · 2025-09-10T16:31:14Z

config/aaa.py

+                secure_cipher.set_feature_password("TACPLUS", passwd)
+            else:
+                # Check if password rotation is enabled
+                if rotate:


nit: could be else-if with else statement above

anders-nexthop · 2025-09-10T16:32:31Z

config/aaa.py

+                if rotate:
+                    passwd = getpass.getpass()
+                    # Rotate password for TACPLUS feature and re-encrypt the secret
+                    secure_cipher.rotate_feature_passwd("TACPLUS", passwd)


don't we need to encrypt the secret here as well? maybe this shouldn't return?

anders-nexthop · 2025-09-10T16:41:30Z

config/aaa.py

    """Specify TACACS+ server global passkey <STRING>"""
    if ctx.obj == 'default':
        del_table_key(db, 'TACPLUS', 'global', 'passkey')
    elif secret:


in secret is not defined? then what? is the idea that an empty secret disables the passkey entirely? what happens if we pass an empty secret and --encrypt?

anders-nexthop · 2025-09-10T16:45:41Z

config/aaa.py

    else:
-        click.echo('Argument "secret" is required')
+        if secure_cipher.is_key_encrypt_enabled("TACPLUS", "global"):
+            secure_cipher.deregister("TACPLUS", "TACPLUS|global")


so if we run without --encrypt, we will remove the associated key encryption password and set key_encrypt to false, since the user is no longer wanting encryption. this seems correct.

What about the --rotate option though? if a user passes --rotate without --encrypt, what then? we should at least throw an error in that case, rather than removing the encryption.

anders-nexthop · 2025-09-10T16:47:49Z

config/aaa.py

 @click.option('-m', '--use-mgmt-vrf', help="Management vrf, default is no vrf", is_flag=True)
 @clicommon.pass_db
-def add(db, address, timeout, key, auth_type, port, pri, use_mgmt_vrf):
+def add(address, timeout, key, encrypted_key, rotate, auth_type, port, pri, use_mgmt_vrf, encrypt):


I don't see --encrypt as an arg?

anders-nexthop · 2025-09-10T16:48:43Z

config/aaa.py

+            raise click.UsageError("You must provide either --key or --encrypted_key")
+
+        if encrypted_key is not None:
+            try:


this is almost the same as the block above, lets refactor them to re-use the logic

anders-nexthop · 2025-09-10T16:52:15Z

config/aaa.py

-        if key is not None:
-            data['passkey'] = key
+
+        if key and encrypted_key:


would it make more sense to keep just --key and have an --encrypt argument, instead of --encrypted_key? --encrypted_key sounds like you are passing in a key that is already encrypted, which is not true; you are passing in a plaintext key that you WANT to be encrypted.

Integrated Security Cipher for TACPLUS passkey encryption/decryption

f01c202

nmoray changed the title ~~Integrated Security Cipher for TACPLUS passkey encryption/decryption~~ Integrated Security Cipher for TACPLUS passkey encryption Jun 25, 2025

nmoray mentioned this pull request Jun 25, 2025

TACACSPLUS_PASSKEY_ENCRYPTION support Part - I #3027

Closed

nmoray mentioned this pull request Jun 25, 2025

Integrated Security Cipher module with Hostcfgd for passkey decryption sonic-net/sonic-host-services#274

Open

Added missed callback argument

df0cbec

nmoray force-pushed the security_cipher_tacplus_encrypt branch from 12a512c to df0cbec Compare June 26, 2025 05:23

Fixed precommit failures

a12b028

Fixed build issues

d3b1ce2

Fixed precommit issues

2e8e79a

Fixed precommits issues

92201bc

nmoray mentioned this pull request Jun 27, 2025

Security Cipher Module: Added password rotate feature and enabled Backup and Restore support for NOS Upgrades sonic-net/sonic-buildimage#22711

Open

8 tasks

Updated implementation as per modified design

ca28b5c

Fixed build issues

b70ad67

anders-nexthop suggested changes Sep 10, 2025

View reviewed changes

Integrated Security Cipher for TACPLUS passkey encryption #3936

Are you sure you want to change the base?

Integrated Security Cipher for TACPLUS passkey encryption #3936

Uh oh!

Conversation

nmoray commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Jun 25, 2025

Uh oh!

azure-pipelines bot commented Jun 25, 2025

Uh oh!

nmoray commented Jun 25, 2025

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

nmoray commented Jun 26, 2025

Uh oh!

linux-foundation-easycla bot commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

mssonicbld commented Jun 26, 2025

Uh oh!

azure-pipelines bot commented Jun 26, 2025

Uh oh!

mssonicbld commented Jun 27, 2025

Uh oh!

azure-pipelines bot commented Jun 27, 2025

Uh oh!

mssonicbld commented Jun 27, 2025

Uh oh!

azure-pipelines bot commented Jun 27, 2025

Uh oh!

anders-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

nmoray commented Jun 25, 2025 •

edited

Loading

linux-foundation-easycla bot commented Jun 26, 2025 •

edited

Loading