Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

Expose interesting information as environment variables to SSH_ASKPASS #9

Open
martinpaljak opened this issue Dec 17, 2016 · 4 comments

Comments

@martinpaljak
Copy link

I don't like the message and would like to build my own in a custom SSH_ASKPASS script, but the interesting informations (like host) is not available as a separate variable for the askpass app. It would be nice to export all variables to the askpass script.

@tiwe-de
Copy link
Owner

tiwe-de commented Dec 17, 2016

The ssh-agent or ssh-agent-filter gets to sign the message specified in RFC 4252 Section 7. There is no usable information on the target host in there, it is already hashed in the session identifier. We already put all the interesting stuff in the string that is passed to SSH_ASKPASS.

Nevertheless I'm open for a patch (but not a q&d one) adding environment variables.

@tiwe-de tiwe-de closed this as completed Dec 17, 2016
@martinpaljak
Copy link
Author

So my current implementation is to populate a map in dissect_auth_data_ssh() and to give this as a parameter to confirm().

I still have to make pam_ssh_agent_auth to compile with OpenSSL 1.1.0, as that seems even more interesting with visual notification.

The main reason for this change is that combined with Yubikey, that already has a touch confirmation for signatures, the very long and verbose message is not really usable on OSX with a notification (instead a confirmation dialog). Instead of making the message "statically" inside ssh-agent-filter, environment variables can be populated and actual UI be dynamically created in $SSH_ASKPASS as required.

Would this be sufficient as a non-q&d approach?

@martinpaljak
Copy link
Author

Also, localization is something that seems easier to manage outside of ssh-agent-filter instead of inside it.

@tiwe-de tiwe-de reopened this Dec 18, 2016
@tiwe-de
Copy link
Owner

tiwe-de commented Dec 18, 2016

Your approach sounds reasonable. I never did environment changes from a C(++) program, but I guess it should be done in the fork()ed child process just before the exec(). Please leave the confirmation string intact.

Beware of C string termination attacks and while you're at it please also add detection and/or escaping for the confirmation string.

I didn't even think about l10n as this tool was intended for admins.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants