Menu

#5 including authenticated action in signature request

None
closed-fixed
jbeverly
None
5
2014-03-24
2013-09-18
chrysn
No

i'd like to suggest that the action that is just about to be permitted be represented in the data chunk that gets actually signed and is generated in userauth_pubkey_from_id. in the case of sudo, that should include the user id to be assumed, the working directory and the command. (afaik environment variables are filtered by sudo).

the aim of this is to create a trust chain from the ssh-agent that signs the request to the pam library that requested it, where the agent can know what it will sign off. support for this kind of introspection is just being built in into ssh-agent-filter.

if there is no better way to extend the data structures involved, i propose that the data be put into the user field, which is currently filled with a static string of "root". i'd argue that while in some situations we're ssh-userauth just to authenticate doing anything as a user, but in other situations we are authenticating very particular actions we're requesting, so the user is not actually "root" but "root-as-restricted-to-do-exactly-this". given there'll be a path involved, '\0'-separated fields inside the user string could work, so a typical cd /srv/foo; sudo -u www apache2 -f my_config would end up as pamsshagentauth_buffer_put_string(&b, "www\0/srv/foo\0apache2 -f my_config", 33).

as far as i know, there are no clients that do anything else but blindly sign whatever gets requested here, in ssh-agent-filter this is just being considered.

for reference, the request originally came from timo lindfors on http://bugs.debian.org/595817.

Discussion

  • chrysn

    chrysn - 2013-09-18

    in order to prevent host spoofing, the host name should be part of the information snipplets that are requested for signing (think of a user running sudo shutdown -h now on server A, while his user account was compromised, the attacker logs in into server B, gets a challenge for running sudo shutdown -h now there, and lets the agent on server A sign it).

    tiwe suggested that the information could be also packed into session identifier, and that a dedicated service name be reserved. i'm unsure of whether a different service name would make things fail elsewhere; another option would be to have a magic number in the session identifier, whereupon the rest of the session identifier gets decoded by the client.

     
  • jbeverly

    jbeverly - 2013-09-19
    • assigned_to: jbeverly
    • Group: -->
     
  • jbeverly

    jbeverly - 2013-09-19

    This seems easy enough to implement, and its benefits are obvious. I would have loved a patch for this one; but I'll get to work on it ASAP.

     
  • chrysn

    chrysn - 2013-09-19

    i tried to write one, but it was not clear to me how to obtain the details of what was just being authorized.

    to write one, the questions open from my pov are:

    • in which data structure does pam store the very action that is to be permitted? (once it is found, it is easy to be passed into userauth_pubkey_from_id)

    • where do you think is that information best stored? more and more, i'm leaning towards the session identifier together with a magic number, but it would be great to get a statement from someone who understands the whole ssh architecture. (i don't).

     
  • chrysn

    chrysn - 2013-09-19

    some experimentation showed that using a dedicated service name does not harm anything in my setup. with that, i'd go for the session identifier and have it combined from some random data and the authorization statement.

     
  • Timo Weingärtner

    The contents of the data part is described on https://tools.ietf.org/html/rfc4252#page-10 .

    I don't think a magic number in a field normally containing pseudorandom data is a good way to tell requests from ssh vs. pam_ssh_agent_auth apart.

    Instead we should keep the same format, but use values for some field that ssh would never use, e.g. 0 in the byte field.

    There should also be some way to keep non-ssh requests apart, we could use the service field for that and put "pam_ssh_agent_auth" in there:

    string session identifier
    byte 0
    string target user name
    string "pam_ssh_agent_auth"
    string "publickey"
    boolean TRUE
    string public key algorithm name
    string public key to be used for authentication

    The session identifier could contain other stuff (in rfc4251 encoding):

    string command
    string working directory
    uint64 timestamp (against replay attacks)
    string hostname (against relay attacks)

     

    Last edit: Timo Weingärtner 2013-09-21
  • jbeverly

    jbeverly - 2013-09-24

    Unfortunately, getting the "command" in a portable way is not currently possible. I could look up on the stack by following EBP/RBP and find the arguments passed to main, but that requires fragile, and non-trivial platform specific implementations which rely on a lot of assumptions (for instance, compiler optimizations such as frame-pointer omission result in EBP/RBP not being pushed onto the stack; similarly on a.out/bundle based systems (like osx), the addresses appear to be disjoint in the dlopen'd module from the process loading it, and I'm not 100% sure how to work around that; and without research, I don't even know how to do this on sparc, pf, etc) More importantly, this would not be simple code; and in an authentication module, I would much prefer simple code.

    Because of this, I think it best to "hope" that whatever command your running sets some environment variable (and otherwise wipes the environment, so that you can trust the environment variable you get) telling you what the command it is executing is. sudo does this (SUDO_COMMAND) but unfortunately does not presently have this variable set when it starts pam authentication.

    In the hopes of possibly getting sudo to set that environment variable prior to starting pam_authenticate, I'll include all the rest of the information, and try to getenv SUDO_COMMAND (as well as a more generic "PAM_AUTHORIZED_ACTION") and plug that into the session-id.

    Also, in line with the comment about "use values ssh would never use", I'm going to request having a new message identifier be added to ssh SSH2_MSG_USERAUTH_TRUST_REQUEST = 54 (or similar) which would indicate that the session-id contains something interesting, in addition to random-data. (which starts with a leading integer to distinguish between different types of trust-chain type requests, should somebody else need similar)

    Revision 92 contains a first whack at it, but was written in haste, and hasn't been tested or vetted for mental acuity.

     
  • jbeverly

    jbeverly - 2014-03-22
    • status: open --> closed-fixed
     
  • chrysn

    chrysn - 2014-03-22

    thank you for fixing this; i didn't notice the dev/jamie branch until now.

    there are three issues with the solution, ranging from security critical (no cookie gets sent) to performance loss (4k signing requests instead of a few bytes).

    i think my patches are pretty self explanatory, please include them or similar ones before making a release.

     
  • chrysn

    chrysn - 2014-03-22

    it seems i can only upload a patch at a time, here's the second one.

     
  • chrysn

    chrysn - 2014-03-22

    and the last one.

    in case you are using git internally, you can cherry-pick the patches from my chrysn/patches branch at git://prometheus.amsuess.com/pam-ssh-agent-auth .

     
  • jbeverly

    jbeverly - 2014-03-24

    Applied all 3, plus some more of my own. I'm going to go ahead and do my platform testing, and if that goes well, I'll cut a release.

     

Log in to post a comment.