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.
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
Adding support of common security cipher module for encryption and decryption of a passkey #17201
base: master
Are you sure you want to change the base?
Adding support of common security cipher module for encryption and decryption of a passkey #17201
Changes from 18 commits
be19898
05e0e46
e681d60
bc02ccf
883fe3f
e917f4d
4b3ebbd
dc67537
075f45e
6089def
4efb99d
5a2ef2f
5c0455b
2079558
b7f4f1d
542754a
382690e
494879b
014a0d4
b3babbd
470550f
167e820
7fc4c8d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do we need a master key per feature? I thought we're using one master key for all features.
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 guess it is better to have different Master keys for different feature. This way, there will not be any inter-dependency between them.
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.
Let's not complicate things for the user, just follow well established practices (e.g a master key for all features) from the popular NOS if possible, my 2 cents.
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.
With this approach, we can further extend this implementation for other modules too not just TACPLUS, RADIUS and LDAP. Additionally, it is upto the user if he / she needs to use different keys or can use same keys for all.
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.
It will give flexibility to the user, in my opinion.
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.
If we go with single key, it will be having huge dependancy in case of changing that key. User needs to change the encrypted passkey in CONFIG_DB for all the features.
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 disagree, IMO, we should just look at the behavior of other popular NOS that's been there for many years, if required for any SONiC use-case, we can think about providing the flexibility in the future.
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.
With this approach, we are already providing the flexibility to the user (why to wait for the future :-)). They can have either same or different keys for different features.
The proprietary NOSes have different architectures and they have implemented the feature which can be fitted into their infrastructure. :-) I guess, it is always better if we can add bit of flexibility into any of the designs.