Adding support of common security cipher module for encryption and decryption of a passkey#17201
Conversation
madhupalu
left a comment
madhupalu
left a comment
There was a problem hiding this comment.
Changes looks good me except few minor comments. please address it.
|
@venkatmahalingam I have added the AUT too. Can you please approve the PR now? This is an independent module. It doesn't have any impact on any other functionalities. Thus we can merge this into master. I will work on other PRs parallelly to integrate this module into TACACS feature. |
|
@venkatmahalingam Once this PR is merged, build will get pass of both the following PRs. PR: TACACS passkey encryption Part - I |
venkatmahalingam
left a comment
venkatmahalingam
left a comment
There was a problem hiding this comment.
Please address the outstanding comments.
@venkatmahalingam I have replied to the comments. IMO, Flexible design is a good approach. Why to restrict user to go on a certain path only. He / she should have multiple options. |
@nmoray @venkatmahalingam giving multiple option is good but also customers who migrates from other well known NOS to SONiC will be using one master key. To accommodate those customers , Can you include one master key option as well. |
@avinothcgl Using one master key for all the feature is still possible with the current implementation. Just keep same master key for all in cipher_pass file. For instance, One more advantage of this design is, same security_cipher module can be extended to other applications as well which need to make use of encryption / decryption. That application can have it's own master_key which will be different from TACPLUS, RADIUS and LDAP. |
|
@venkatmahalingam @avinothcgl @venkatmahalingam shouldn't it be enough to solve the migration use case? please suggest to move forward with this PR? For instance, One more advantage of this design is, same security_cipher module can be extended to other applications as well which need to make use of encryption / decryption. That application can have it's own master_key which will be different from TACPLUS, RADIUS and LDAP. |
|
Reviewers, if you are ok with this PR, please help to approve it. Thanks. |
|
@venkatmahalingam As requested, updated the class name as "master_key_mgr". |
|
This is a great feature! In addition to the TACPLUS PRs already proposed, I would also recommend that the RADIUS keys (partially implemented per this PR) and SNMPv3 user passwords/privs also have the ability to be encrypted to align with industry security standards. If a new issue needs to be opened for tracking of RADIUS/SNMPv3 components let me know and I can submit them. |
Thanks @brholmes1. IMO, let's first merge the initial infra. Later, we can easily extend the support for other modules. For tracking purpose, please open the new issues in the community. |
|
@lguohan Can you please help in approving / merging this PR. |
|
Folks please help in merging the PR if there are no more comments. It is open from more than 6 months now. Please help in merging the PR. |
There was a problem hiding this comment.
It's probably worth generating this file from the features in _feature_list. that way if we want to add a new feature we just add a list entry instead of having to make changes here.
There was a problem hiding this comment.
Looking at how this is used in later commits, I think this is incorrect. We should not be removing the entire file, but instead just changing/deleting the entry. If I have both TACACS and RADIUS configured and i remove the passkey for one of them, I still want the other one to work.
There was a problem hiding this comment.
I will update the method.
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
@qiluo-msft @anders-nexthop I have addressed the comments. |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
@anders-nexthop you mean just rebase the
@anders-nexthop I just rebased my remote branch and re-pushed it. Let's see if it makes any difference. |
@qiluo-msft The checks are still failing. Please let us know what needs to be done to fix these failures. |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@qiluo-msft Please re-approve the PR. I pushed one more commit for removing commented code. |
|
@qiluo-msft can you please re-approve and merge the PR. All the checks have been passed now. |
| } | ||
|
|
||
| leaf key_encrypt { | ||
| type boolean; |
There was a problem hiding this comment.
Sure. I will raise a new PR to accomodate this change.
| default 5; | ||
| } | ||
|
|
||
| leaf key_encrypt { |
There was a problem hiding this comment.
Mix tabs with spaces, please reformat this "leaf key_encrypt" section.
There was a problem hiding this comment.
You mean, replace the tabs with spaces for the entire leaf section?
There was a problem hiding this comment.
@qiluo-msft The link that you shared is not accessible.
There was a problem hiding this comment.
I removed the link. Yes, I mean to replace the tabs with spaces for the entire leaf section
There was a problem hiding this comment.
Sure, will do as a part of new PR.
|
|
||
| def __init__(self): | ||
| if not self._initialized: | ||
| self._file_path = "/etc/cipher_pass" |
There was a problem hiding this comment.
@qiluo-msft As per https://github.com/sonic-net/SONiC/blob/master/doc/ztp/SONiC-config-setup.md#222-config-migration
the backup and restore of cipher_pass file will automatically be taken care, right?
There was a problem hiding this comment.
Thanks for the link! The file "/etc/cipher_pass" is outside the folder of config migration design, could you check again?
There was a problem hiding this comment.
oh yes. Okay then, I will create another PR to add pre-hook and post-hook scripts to backup and restore the cipher_pass file during the up-gradation. Is that okay? Or just change the location to "/etc/sonic"?
…kup and Restore support for NOS Upgrades (#22711) 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: TACACSPLUS_PASSKEY_ENCRYPTION.md SONiC#1471 Link to config_db schema for YANG module changes https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md https://github.com/sonic-net/SONiC/blob/master/doc/ztp/SONiC-config-setup.md#222-config-migration 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 }
…kup and Restore support for NOS Upgrades (sonic-net#22711) 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 sonic-net#17201 Reference HLDs: TACACSPLUS_PASSKEY_ENCRYPTION.md SONiC#1471 Link to config_db schema for YANG module changes https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md https://github.com/sonic-net/SONiC/blob/master/doc/ztp/SONiC-config-setup.md#222-config-migration 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 } Signed-off-by: Feng Pan <fenpan@microsoft.com>
…kup and Restore support for NOS Upgrades (#22711) 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: TACACSPLUS_PASSKEY_ENCRYPTION.md SONiC#1471 Link to config_db schema for YANG module changes https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md https://github.com/sonic-net/SONiC/blob/master/doc/ztp/SONiC-config-setup.md#222-config-migration 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 } Signed-off-by: dprital <drorp@nvidia.com>
10 participants
Why I did it
This module is created to handle the passkey encryption, decryption and the cipher storage. It's a common module which will be used for feature like TACACS, RADIUS, LDAP etc.
This implementation is aligned with HLD
How I did it
This module will expose following APIs.
How to verify it
`
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Master
Link to config_db schema for YANG module changes
Schema
Related PRs:
Part I
Part II