Skip to content

Conversation

mgjm
Copy link
Contributor

@mgjm mgjm commented Apr 9, 2021

The Dockerfile contained a random password to unlock the git user account. Otherwise the OpenSSH server would complain about a locked account (#377).

A locked account is marked by a ! at the start off the password hash. An alternative is the password hash *. Both are invalid crypt password hashes and therefore no login via password is possible. The ! has the additional meaning of an locked account, whereas * just means no login. See shadow(5).

Additionally the rootless Dockerfile uses the embedded SSH server. And I think the embedded SSH server does not depend on the git user password at all. Therefore we can leave the password at the default (disabled and locked via !).

ToDo:

  • Verify that the ssh access to the (root) docker image still works
  • Verify that the ssh access to the rootless docker image still works
    • If the rootless image does not work, revert 04390f5

@mgjm mgjm changed the title Remove random passwords in Dockerfile Remove random password in Dockerfiles Apr 9, 2021
@silverwind silverwind added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Apr 9, 2021
@techknowlogick techknowlogick added this to the 1.15.0 milestone Apr 10, 2021
@mgjm
Copy link
Contributor Author

mgjm commented Apr 17, 2021

I just verified that the login in the main Docker image works with the new * password hash and that the rootless image works with a locked account.

From my side this PR is now ready for review / merge.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 17, 2021
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 29, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 29, 2021
@6543
Copy link
Member

6543 commented Apr 29, 2021

🚀

@6543 6543 merged commit d576126 into go-gitea:master Apr 29, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants