Skip to content
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

User creation with password #437

Merged
merged 2 commits into from
Jun 14, 2023
Merged

User creation with password #437

merged 2 commits into from
Jun 14, 2023

Conversation

djach7
Copy link
Contributor

@djach7 djach7 commented Mar 2, 2023

This pr attempts to implement the creation of a user and password when onboarding a device, using a username and password provided within the serviceinfo_api_server.yml config.

closes #321

This is a draft, current known issues include:

  • Password encryption on on user creation
  • Confirming sshkey creation still functional
  • Confirming edge case functionality

@miabbott
Copy link
Contributor

miabbott commented Mar 2, 2023

Could you use a Rust crate to handle the password hashing instead of shelling out to openssl?

https://docs.rs/password-hash/latest/password_hash/

https://docs.rs/bcrypt/latest/bcrypt/

@miabbott
Copy link
Contributor

miabbott commented Mar 2, 2023

Also we should be defaulting to SHA512 for the algorithm; MD5 is considered insecure these days.

https://access.redhat.com/articles/880733

@nullr0ute
Copy link
Contributor

Could you use a Rust crate to handle the password hashing instead of shelling out to openssl?

Or using a function in the openssl crate.

@sarmahaj
Copy link
Contributor

sarmahaj commented Mar 3, 2023

are we not contradicting to FIDO policy of "password less" logins by providing a way to set password for initial user ?
I think we should prefer sshkey rather than password for at least initial user which is for first boot/onboarding. As I recall we did this because for local user it asks for password .

@say-paul
Copy link
Contributor

say-paul commented Mar 3, 2023

An general thought on having the plain text password in .yaml which may raise some eyebrows.We can force user to put the encrypted password in the file as its done with the useradd command , advantage is 1. simplify the implementation , 2. security.

@say-paul
Copy link
Contributor

say-paul commented Mar 3, 2023

are we not contradicting to FIDO policy of "password less" logins by providing a way to set password for initial user ? I think we should prefer sshkey rather than password for at least initial user which is for first boot/onboarding. As I recall we did this because for local user it asks for password .

the ssh_key feature already exists, this added feature is for any user who requires password to be set also.

@djach7
Copy link
Contributor Author

djach7 commented Mar 3, 2023

Thank you all for the comments/ideas, I'm going to try to understand the openssl crate and I'll make the other changes above too.

@djach7 djach7 force-pushed the loginWithPW branch 2 times, most recently from 7aad656 to c17a543 Compare May 9, 2023 19:20
@djach7 djach7 force-pushed the loginWithPW branch 10 times, most recently from c93c4b9 to 8cdbcbe Compare May 15, 2023 19:41
@djach7 djach7 force-pushed the loginWithPW branch 11 times, most recently from a9943aa to 5e3af19 Compare June 12, 2023 15:51
@djach7 djach7 marked this pull request as ready for review June 12, 2023 16:11
@7flying
Copy link
Contributor

7flying commented Jun 13, 2023

There are some changes made into the serviceinfo-api-server, owner-onboarding-server and client-linuxapp on the chore(integration-tests) commit that are not tests, I would like those changes to be moved to the implementation commit.

Other than that, we are not testing that you can add more than one ssh key per user, but we can add that as a follow-up PR.

So, feel free to mark the PR as ready

@nullr0ute
Copy link
Contributor

Other than that, we are not testing that you can add more than one ssh key per user, but we can add that as a follow-up PR.

Can we ensure an issue is created so that isn't lost.

@7flying
Copy link
Contributor

7flying commented Jun 13, 2023

Other than that, we are not testing that you can add more than one ssh key per user, but we can add that as a follow-up PR.

Can we ensure an issue is created so that isn't lost.

Yep, linking the issue here for future reference #507

@djach7 djach7 force-pushed the loginWithPW branch 2 times, most recently from 2ed6ae5 to ee6960f Compare June 14, 2023 13:54
@djach7 djach7 force-pushed the loginWithPW branch 3 times, most recently from 252ec71 to d312892 Compare June 14, 2023 15:02
Copy link
Contributor

@7flying 7flying left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good, thank you!

djach7 added 2 commits June 14, 2023 17:27
Allows users to create a password for login when onboarding a device.
These passwords are optional and should be provided within the
'serviceinfo_api_server.yml' config file.

A user's password will be encrypted via SHA-256 if it is not already
encrypted when provided to the config.

Signed-off-by: djach7 <djachimo@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login via password for user created using fido
6 participants