Security Cipher Module: Added password rotate feature and enabled Backup and Restore support for NOS Upgrades #22711

nmoray · 2025-05-27T09:52:47Z

What I did it

Updated the existing design to support password rotate feature in the Security Cipher module.
Additionally, now multiple modules of same feature type can make use of same password for generating their unique encrypted passkey.
Added backup and restore support for cipher_pass.json file stored at /etc dir.
Reference PR: Adding support of common security cipher module for encryption and decryption of a passkey #17201
Reference HLDs:

Now the module / application / feature who wants to use the security cipher module for encryption / decryption, it needs to first register with it by providing a callback function which will be responsible for updating the existing encrypted passkey in the CONFIG_DB table or which ever the storage media used. This way, all the registered parties will be notified and at the same time gets updated with the new password. The security cipher module keeps a list of all the callbacks and at the time of password rotation, it simply notifies and updates everyone one after the other.

How to verify it

Encrypt the passkey for multiple TACPLUS / RADIUS servers using same password
NOS upgrade to verify backup / restore feature

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
  }

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

202311

Link to config_db schema for YANG module changes

https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md

mssonicbld · 2025-05-27T09:52:55Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-27T09:53:08Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-05-27T09:54:35Z

/azpw ms_conflict

nmoray · 2025-05-27T09:55:29Z

@qiluo-msft As per your comment, I have added pre/post migration scripts for Security Cipher module.
cc: @anders-nexthop @madhupalu

mssonicbld · 2025-05-27T14:35:46Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-05-27T14:35:58Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-05-28T07:02:24Z

@qiluo-msft please help me understand ms_conflict failure. I don't know why this is failing.
Additionally, please approve the PR in case there are no new comments.

anders-nexthop · 2025-06-12T16:35:56Z

@qiluo-msft @lguohan are there any further comments on this PR? If not, can we merge it and unblock this feature?

@nmoray this is what the MS-Conflict says:

failure: @StormLiangMS,@liushilongbuaa Conflict already exists in master
Please wait a few hours to run ms_conflict again!
'/azpw ms_conflict'

I don't know what this check is for, but maybe just rebase and push again to re-run it and see if that resolves things?

nmoray · 2025-06-13T03:54:18Z

@qiluo-msft @lguohan are there any further comments on this PR? If not, can we merge it and unblock this feature?

@nmoray this is what the MS-Conflict says:
failure: @StormLiangMS,@liushilongbuaa Conflict already exists in master
Please wait a few hours to run ms_conflict again!
'/azpw ms_conflict'
I don't know what this check is for, but maybe just rebase and push again to re-run it and see if that resolves things?

Yeah @anders-nexthop, I have already tried this option but no luck.

…urity Cipher module. Additionally, now multiple modules of same feature type can make use of same password for generating their unique encrypted passkey. For instance, now multiple TACPLUS server can have their own passkey encrypted from the same password.

mssonicbld · 2025-06-19T13:55:02Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-06-19T13:55:13Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-19T15:37:31Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-06-19T15:37:42Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-06-20T03:20:41Z

@anders-nexthop @qiluo-msft Please review the updated code and share the comments if any.

anders-nexthop

Looks good, the rotation code is a good change.

src/sonic-py-common/sonic_py_common/security_cipher.py

mssonicbld · 2025-06-23T04:50:16Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-06-23T04:50:27Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-06-23T06:33:39Z

/azp run Azure.sonic-buildimage

anders-nexthop

I am approving this change, as I think this is a workable implementation and would like to see it move forward. However, please consider my point about separating encryption password handling from table registration.

@qiluo-msft can you take another look, or assign another reviewer to get more eyes on this latest version?

@bhouse-nexthop your thoughts would be welcome here given your security background.

src/sonic-py-common/sonic_py_common/security_cipher.py

mssonicbld · 2025-07-02T11:48:53Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-07-02T11:49:05Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-02T13:17:54Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-07-02T13:18:07Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-03T05:06:41Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-07-03T05:06:54Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-03T12:09:55Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-07-03T12:10:08Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-07-04T10:31:56Z

Thanks @anders-nexthop for the diligent review.
@qiluo-msft @bhouse-nexthop I have addressed the comments. Please approve in case there are no more comments.

nmoray · 2025-07-07T09:16:58Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-07-07T09:17:04Z

Commenter does not have sufficient privileges for PR 22711 in repo sonic-net/sonic-buildimage

mssonicbld · 2025-07-07T09:27:26Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2025-07-07T09:27:40Z

Azure Pipelines successfully started running 1 pipeline(s).

nmoray · 2025-07-07T12:54:20Z

@qiluo-msft please approve in case there are no more comments so that other dependent PRs can also be merged.
sonic-net/sonic-utilities#3936
sonic-net/sonic-host-services#274

nmoray · 2025-07-24T05:10:59Z

@qiluo-msft will you please help in merging this PR in case there are no more comments?

anders-nexthop · 2025-09-09T15:03:40Z

@qiluo-msft and @lguohan can we get this PR merged if there are no more comments?

prabhataravind · 2025-09-12T20:06:44Z

hi @qiluo-msft is this good to be merged?

arlakshm · 2025-10-08T15:10:10Z

@qiluo-msft, please help approve if no other comments.

anders-nexthop · 2025-11-02T21:16:07Z

@qiluo-msft @lguohan Besides wanting this for TACACS, there are two outstanding feature requests that are dependent on this basic infrastructure getting merged (#22276 and #22277), can we please get this reviewed by an authorized reviewer and merged up?

lguohan · 2025-11-03T05:47:29Z

do we have sonic-mgmt test to test this?

lguohan · 2025-11-03T05:50:33Z

@anders-nexthop , i will ask qiluo to take a look this PR. Thanks for your patience.

qiluo-msft · 2025-11-05T18:30:44Z

files/image_config/config-setup/01-post-security-cipher

+# Restore cipher_pass file from persistent storage
+if [ -f /host/security_cipher/cipher_pass ]; then
+    cp /host/security_cipher/cipher_pass.json /etc/cipher_pass.json
+    chmod 640 /etc/cipher_pass.json


640

Is it better to use 600 if the only reader is root?

qiluo-msft · 2025-11-05T18:32:27Z

src/sonic-py-common/sonic_py_common/security_cipher.py

+        try:
+            with open(CIPHER_PASS_FILE, 'r') as f:
+                return json.load(f)
+        except Exception as e:


Exception

Could you use more specific exception type?

qiluo-msft

As commented

nmoray added 3 commits May 27, 2025 09:35

Added support of backup and restore for cipher_pass file

67df31f

Made default value as false for key_encrypt flag and removed uneven tabs

1c23a3d

Updated jinja template to copy pre/post migration scripts

12ef9e5

nmoray requested review from lguohan and qiluo-msft as code owners May 27, 2025 09:52

Added sudo while executing chmod

1469610

nmoray changed the title ~~Security Cipher Module: Enabled Backup and Restore support for NOS Upgrades~~ Security Cipher Module: Added password rotate feature and enabled Backup and Restore support for NOS Upgrades Jun 19, 2025

nmoray mentioned this pull request Jun 19, 2025

TACACSPLUS_PASSKEY_ENCRYPTION support Part - I sonic-net/sonic-utilities#3027

Closed

Added key_encrypt option under individual TACPLUS server configs

eb1709f

anders-nexthop reviewed Jun 20, 2025

View reviewed changes

src/sonic-py-common/sonic_py_common/security_cipher.py Outdated Show resolved Hide resolved

src/sonic-py-common/sonic_py_common/security_cipher.py Show resolved Hide resolved

src/sonic-py-common/sonic_py_common/security_cipher.py Outdated Show resolved Hide resolved

Addressed comments

6e1b08b

Updated method description

aaac0c6

anders-nexthop approved these changes Jul 1, 2025

View reviewed changes

src/sonic-py-common/sonic_py_common/security_cipher.py Outdated Show resolved Hide resolved

Fixed build issues

986eec0

fixed build issues

a6f641a

Fixed build issues

dea228d

Addressed comments

f746e09

Triggered another build

06b4b7a

qiluo-msft reviewed Nov 5, 2025

View reviewed changes

qiluo-msft requested changes Nov 5, 2025

View reviewed changes

Security Cipher Module: Added password rotate feature and enabled Backup and Restore support for NOS Upgrades #22711

Are you sure you want to change the base?

Security Cipher Module: Added password rotate feature and enabled Backup and Restore support for NOS Upgrades #22711

Uh oh!

Conversation

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

Uh oh!

What I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Link to config_db schema for YANG module changes

Uh oh!

mssonicbld commented May 27, 2025

Uh oh!

azure-pipelines bot commented May 27, 2025

Uh oh!

nmoray commented May 27, 2025

Uh oh!

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

Uh oh!

Uh oh!

mssonicbld commented May 27, 2025

Uh oh!

azure-pipelines bot commented May 27, 2025

Uh oh!

nmoray commented May 28, 2025

Uh oh!

anders-nexthop commented Jun 12, 2025

Uh oh!

nmoray commented Jun 13, 2025

Uh oh!

mssonicbld commented Jun 19, 2025

Uh oh!

azure-pipelines bot commented Jun 19, 2025

Uh oh!

mssonicbld commented Jun 19, 2025

Uh oh!

azure-pipelines bot commented Jun 19, 2025

Uh oh!

nmoray commented Jun 20, 2025

Uh oh!

anders-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jun 23, 2025

Uh oh!

azure-pipelines bot commented Jun 23, 2025

Uh oh!

mssonicbld commented Jun 23, 2025

Uh oh!

anders-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssonicbld commented Jul 2, 2025

Uh oh!

azure-pipelines bot commented Jul 2, 2025

Uh oh!

mssonicbld commented Jul 2, 2025

Uh oh!

azure-pipelines bot commented Jul 2, 2025

Uh oh!

mssonicbld commented Jul 3, 2025

Uh oh!

azure-pipelines bot commented Jul 3, 2025

Uh oh!

mssonicbld commented Jul 3, 2025

Uh oh!

azure-pipelines bot commented Jul 3, 2025

Uh oh!

nmoray commented Jul 4, 2025

Uh oh!

nmoray commented Jul 7, 2025

Uh oh!

azure-pipelines bot commented Jul 7, 2025

Uh oh!

nmoray commented May 27, 2025 •

edited

Loading

nmoray commented May 27, 2025 •

edited

Loading