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

Changing PAM behaviour to stop ugly output on first connection #19

Merged
merged 1 commit into from
Jul 16, 2017
Merged

Changing PAM behaviour to stop ugly output on first connection #19

merged 1 commit into from
Jul 16, 2017

Conversation

FutureSharks
Copy link
Contributor

Currently I get this on first connection with a valid IAM user:

$ ssh my.user@52.208.162.66
Keymaker: Your user account has been replicated onto this host, but SSH will not recognize it until you reconnect.
Keymaker: Connect again to log in to your account.
/usr/local/bin/keymaker-create-account-for-iam-user failed: exit code 75

/usr/local/bin/keymaker-create-account-for-iam-user failed: exit code 77

/usr/local/bin/keymaker-create-account-for-iam-user failed: exit code 77

Permission denied (publickey,keyboard-interactive).

It looks a bit ugly. Even worse for a user that does not exist in IAM:

$ ssh bad.user@52.208.162.66
Error while retrieving IAM SSH keys for max.williamsasdasd: An error occurred (NoSuchEntity) when calling the ListSSHPublicKeys operation: The user with name max.williamsasdasd cannot be found.
/usr/local/bin/keymaker-create-account-for-iam-user failed: exit code 22

Error while retrieving IAM SSH keys for max.williamsasdasd: An error occurred (NoSuchEntity) when calling the ListSSHPublicKeys operation: The user with name max.williamsasdasd cannot be found.
/usr/local/bin/keymaker-create-account-for-iam-user failed: exit code 22

Error while retrieving IAM SSH keys for max.williamsasdasd: An error occurred (NoSuchEntity) when calling the ListSSHPublicKeys operation: The user with name max.williamsasdasd cannot be found.
/usr/local/bin/keymaker-create-account-for-iam-user failed: exit code 22

Permission denied (publickey,keyboard-interactive).

So with this PR I change it to:

  1. Be silent if user with key does not exist in IAM.
  2. Be silent if user is already present on the local system.
  3. A nicer message on user creation.

@FutureSharks
Copy link
Contributor Author

Also, you are adding a requisite entry to the sshd PAM stack. Is this really desired? It means it desire stop any SSH authentication that does not return PAM_SUCCESS by this entry.

What if there is a local user with password authentication enabled?
Would it not make more sense to add an optional PAM entry and always return PAM_SUCCESS in order to let the existing PAM configuration deal with authentication?

@kislyuk
Copy link
Owner

kislyuk commented Jun 9, 2017

You raise good points, but it will take me a while to review this. I'll have to test this thoroughly on both Ubuntu and Red Hat.

At least going by the PAM docs, it does seem that an optional PAM module is probably more appropriate for auto-vivifying the user from the IAM account. But I'll need to try to remember why I chose to use requisite in the first place.

@FutureSharks
Copy link
Contributor Author

Fair enough.

Here is the behaviour I would expect from keymaker in regards to PAM, if I have understood the project intentions correctly:

  1. Be in the ssh service.
  2. If user is already present, do nothing.
  3. If the user is not present and the user is not in IAM, do nothing.
  4. If the user is not present and is in IAM but without a key, do nothing.
  5. If the user is not present and is in IAM with a key, add the user, print message about re-login.
  6. In any and every scenario return PAM_SUCCESS or PAM_IGNORE regardless of outcome.

Is that correct? In this way it won't interfere with any other PAM configuration (like password login etc).

@kislyuk
Copy link
Owner

kislyuk commented Jun 10, 2017

Yes, that is correct.

@FutureSharks
Copy link
Contributor Author

OK cool. I think maybe leave the change from requisite to optional for another PR as it's a separate topic.

In this PR I'm making cosmetic changes and also preventing keymaker from affecting any login by making it always return PAM_SUCCESS as I think is the desired behaviour.

@kislyuk kislyuk merged commit dd8b506 into kislyuk:master Jul 16, 2017
@kislyuk
Copy link
Owner

kislyuk commented Jul 16, 2017

OK, tested this and it seems to all work as expected. @FutureSharks, could you file another PR for changing the PAM module from requisite to optional?

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.

3 participants