-
Notifications
You must be signed in to change notification settings - Fork 134
TACACSPLUS_PASSKEY_ENCRYPTION support Part - II #81
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
Closed
Closed
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
8b909b8
TACACSPLUS_PASSKEY_ENCRYPTION support
nmoray 48e7d1d
code cleanup
nmoray c877edb
Added syslogs
nmoray b37ddcd
Removed debug prints
nmoray 945f4f2
Fixed AUT issues
nmoray 5b61c0c
Fixed build issues
nmoray 7cb53e6
Fixed build issues
nmoray 43641de
fixed build issues
nmoray 66b57b5
Added a check for passkey before appending into server configs
nmoray c6c8e0a
Fixed build issues
nmoray 977a268
TACACSPLUS_PASSKEY_ENCRYPTION support
nmoray a7d01d6
Merge branch 'tacacs_pass_encrypt' of https://github.com/nmoray/sonic…
nmoray 0a1b098
Fixed AUT issue
nmoray 7b14786
Fixed build issues
nmoray 911e2c0
Addressed comments
nmoray bb5e5ee
Incorporated security cipher class into the existing implemenatation
nmoray d745364
Added a logic to remove the cipher pass file file encryption is disabled
nmoray 1d85eb6
Updated class name
nmoray 9285df1
Merge branch 'master' into tacacs_pass_encrypt
nmoray File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
What will happen when OS upgrade from old version which not support this feature? the passkey not encrypt in old version, decrypt will failed here, code here need identify this.
#closed
another solution is add code to DB migrator, but that script is very complex.
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.
Yeah, you are right. Decrypt will fail if someone has either manually added the passkey in config_db or the device has old config where encrypted passkey was absent. So is it fine to add a check if the given server['passkey'] is the same as the one present in common-auth-sonic file (in plaintext). If so, skip decrypt_passkey() API and directly write the passkey in the same file in plaintext format only. Is it okay?
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.
I noticed in the HLD: A DB migration script will be added for users to migrate existing config_db to convert tacacs passkey plaintext to encrypted.
So please also create the DB migration script PR.
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 As I mentioned in the HLD PR, we can achieve the backward compatibility without DB migration by simply following the logic stated above. Please share your thoughts on the same.
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.
I think it's OK, please update code and HLD according to this design change.