Ssh-auth script returns keys of deactivated users


#1

Hello Phabricator community,

we ran into the problem where newly added keys won’t be accepted due to an openssh buffer problem. (This is already known: see T11827 or Newly added SSH keys not working )
While fixing this via writing the returned keys from the ssh-auth script into an authorized_keys file (we’re stuck with Ubuntu LTS 16.04 at the moment), we discovered that the keys of deactivated users are also returned. Maybe this could have security implications.

Observed Behavior:

ssh-auth script returns keys of activated and! deactivated users.

Expected Behavior:

Only ssh keys of activated users should be returned.

Phabricator Version/ System-Setup:

  • Ubuntu 16.04.5 LTS
  • OpenSSH: OpenSSH_7.2p2
  • PHP 7.1.23-3+ubuntu16.04.1+deb.sury.org+1
  • Libraries (2018-11-05):
    • phabricator master hash: 24a061f84 (HEAD -> master, origin/master, origin/HEAD)
    • arcanist master hash: 83661809 (HEAD -> master, origin/master, origin/HEAD)
    • libphutil master hash: cf96fd6 (HEAD -> master, origin/master, origin/HEAD)

Reproduction Steps:

  • Deactivate users with ssh keys (which where not revoked by the user)
  • execute the ssh-auth script <phabricator_install_dir>/bin/ssh-auth
  • the keys of activated and deactivated users will be returned

This can also be verified via the database:

  • db: phabricator_auth
  • table: auth_sshkey

Possible Solution

The keys are extracted from the user objects. The condition in the script to check the instance of the user object, can be extended to check the status of the user. There is already an function which can be used to check if the user is activated:

- if ($object instanceof PhabricatorUser) {
+ if ($object instanceof PhabricatorUser && $object->isUserActivated()) {

Another valid option is to remove the isActive status of the user-keys when users are deactivated.

I hope this helps,
greetings


#2

This is intentional, so we can give the user an explicit error message and disambiguate key-related errors from account-status-related errors. Deactivated users can not establish useful sessions, but they can get an error message which makes it clear that the problem is an issue with their account, not an issue with their private key:

$ git pull
phabricator-ssh-exec: Your account (“epriestley”) does not have permission to establish SSH sessions. Visit the web interface for more information.


#3

Ah ok, this is good to know and thanks for the quick reply.