-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Password Hardening HLD #874
Conversation
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 have following concern and questions:
-
What's the responsbility of PASSWH? If this daemon only for read hardening setting from config DB and update PAM config file, then there already hostcfgd for this, not necessary add a new daemon. And if this daemon also will handle the password history to config DB storage, I think there will be security risk.
-
According to the design, password history will store in DB, if that means config DB, then because currently there is no ACL for config DB, so password history can be access by anyone, this will be a security risk, but if the password history stored by pam_pwhistory it self, It think it's OK.
Hi, regarding question 1: Regarding concern 2, the "old passwords" (passwords history), the meaning of saving the password history is to save only have many old passwords the feature can save, not to save the passwords itself. The passwords will be saved like you commented by pw_history in /etc/security/opasswd or similar file with just root permission access. |
…stead creating a new daemon and remove the STATE_DB
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.
Found some minor issue in latest document.
range 1..30; | ||
} | ||
} | ||
leaf history { |
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.
history_cnt?
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.
thanks, I will update this one
…in yang model and HLD, add bash example using expiration time
description | ||
"First Revision"; | ||
} | ||
typedef feature_state { |
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.
Please follow sonic yang model guideline, every definition should inside top level container, in this case, should inside sonic-passwh
https://github.com/Azure/SONiC/blob/master/doc/mgmt/SONiC_YANG_Model_Guidelines.md
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.
Done, I moved the "typedef feature_state" inside the container.
@liuh-80 @liat-grozovik |
@venkatmahalingam can you please approve from your side? you had comments and i wish to merge once we agree they were handled. |
this is a kind reminder to reply to the Liat Q in the comment above. |
This PR contains the Password Hardening HLD.