| Message ID | 20220420084724.3514-1-levenson@mmer.org |
|---|---|
| Headers | show |
| Series | Let openssh trust /gnu/store | expand |
Hi, Alexey Abramov <levenson@mmer.org> skribis: > This patch allows users to use /gnu/store objects for AuthorizedKeysCommand > and similar options. According to the sshd_config(5): > >> The program must be owned by root, not writable by group or others, and >> specified by an absolute path. That’s the case with programs in /gnu/store. Why isn’t it working? > However, this is not the case for Guix, even though it is RO. OpenSSH doesn't > check if the location mounted or ended up on the RO mount point. > > I think implementing a check for RO location is much harder here, rather > than to trust /gnu/store path. The same way OpenSSH does with users' home > directory. (RO = read-only, right?) I’m not sure why checking whether a file is read-only is much harder. Am I overlooking something? Thanks, Ludo’.
On 20 April 2022 08:47:24 UTC, Alexey Abramov via Guix-patches via <guix-patches@gnu.org> wrote: >This patch allows users to use /gnu/store objects for AuthorizedKeysCommand >and similar options. According to the sshd_config(5): > >> The program must be owned by root, not writable by group or others, and >> specified by an absolute path. > >However, this is not the case for Guix, even though it is RO. OpenSSH doesn't >check if the location mounted or ended up on the RO mount point. The RO bind mount is not a hard guarantee, and a footgun protector against accidental writes, not primarily a security feature (IMO). By design, *anyone* can write *anything* to the store by talking to the daemon. They just can't choose the file name. A much weaker guarantee than OpenSSH assumes, at the very least. With that in mind, could this highly intrusive patch be used to compromise a system? It seems so very likely. If it is, Guix will be rightly derided for what amounts to ifdeffing out the securities, even if OpenBSD's can be frustratingly theatrical at times. >I think implementing a check for RO location is much harder here Why is 'RO location' relevant here? If the snippet you quote above is complete, which requirement does the un-bind-mounted store not meet? I can't think of one off the top o' me head? > , rather >than to trust /gnu/store path. That's a lot of trust. Tens of gigabytes on average. We explicitly rejected that idea in IceCat for example, instead whitelisting only specific store subdirectories. Why is OpenSSH different? > The same way OpenSSH does with users' home >directory. > >Let me know what you think. The rationale and its assumptions (also) belong in the patch itself, not just a separate mail. Hi Alexey, Thanks for the patch suggestion! Kind regards, T G-R Sent on the go. Excuse or enjoy my brevity.
> $EVERYTHING > >Hi Alexey, > >Thanks for the patch suggestion! > >Kind regards, > >T G-R is a very annoying thing my MUA does. Apologies. Kind regards, T G-R Sent on the go. Excuse or enjoy my brevity.
Hi Ludo, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > Alexey Abramov <levenson@mmer.org> skribis: > >> This patch allows users to use /gnu/store objects for AuthorizedKeysCommand >> and similar options. According to the sshd_config(5): >> >>> The program must be owned by root, not writable by group or others, and >>> specified by an absolute path. > > That’s the case with programs in /gnu/store. Why isn’t it working? The reason is that safe_path in openssh takes a full path of the file and checks every directory one by one. The constrain fails on /gnu/store directory due to write permissions for group. openssh reports the following message: Unsafe AuthorizedKeysCommand "/gnu/store/xxxx-echo-sshkey.sh": bad ownership or modes for directory /gnu/store. >> However, this is not the case for Guix, even though it is RO. OpenSSH doesn't >> check if the location mounted or ended up on the RO mount point. >> >> I think implementing a check for RO location is much harder here, rather >> than to trust /gnu/store path. The same way OpenSSH does with users' home >> directory. > > (RO = read-only, right?) Yes. > I’m not sure why checking whether a file is read-only is much harder. > Am I overlooking something? As I mentioned before, the check not just checking the file path itself, but also follows down to the root and check every single directory for the constrain. Me dunno, was thinking about an extra check against mount locations, and in case it has read-only mount options along the way, I could trust the executable. It also implies cross-compilation... May be I overthink the thing? Maybe it is me who overlooking something?
Hi Tobias, Thanks for the review. Tobias Geerinckx-Rice <me@tobias.gr> writes: [...] > The RO bind mount is not a hard guarantee, and a footgun protector > against accidental writes, not primarily a security feature (IMO). > > By design, *anyone* can write *anything* to the store by talking to > the daemon. They just can't choose the file name. A much weaker > guarantee than OpenSSH assumes, at the very least. Even though I knew how the daemon works, I find your explanation very nice and clear. Thank you. [...] > > Why is 'RO location' relevant here? > > If the snippet you quote above is complete, which requirement does the > un-bind-mounted store not meet? I can't think of one off the top o' > me head? Here is a comment from safe_path function --8<---------------cut here---------------start------------->8--- /* * Check a given path for security. This is defined as all components of * the path to the file must be owned by either the owner of of the file * or root and no directories must be group or world writable. * * XXX Should any specific check be done for sym links ? * * Takes a file name, its stat information (preferably from fstat() to * avoid races), the uid of the expected owner, their home directory and * an error buffer plus max size as arguments. * * Returns 0 on success and -1 on failure / --8<---------------cut here---------------end--------------->8--- I probably had to post it first, to avoid misunderstanding. sshd_config(5) is not that clear unfortunately. Due to group write permissions on the /gnu/store directory, safe_path doesn't allow openssh execute it. Couple of months ago I posted this problem on IRC, and you mentioned the read-only mount thingy. So I was trying to take advantage of that. What other options do I have? > That's a lot of trust. Tens of gigabytes on average. =) > We explicitly rejected that idea in IceCat for example, instead > whitelisting only specific store subdirectories. Why is OpenSSH > different? I didn't know that. I don't treat OpenSSH any different than other software either. Whitelist some specific directory is a really good option here, even though It introduces some secret knowledge. > The rationale and its assumptions (also) belong in the patch itself, > not just a separate mail. True. Let me put some more context on what I am trying to do. We have LDAP server which also holds users' ssh keys. I package a simple wrapper for LDAP search which returns them. I would like to use it with OpenSSH, however due to the way it checks executable in the configuration, I don't see the way to use it. I assume it is possible to copy that store object somewhere, but it doesn't look right to me.
Hi, Alexey Abramov <levenson@mmer.org> skribis: > Ludovic Courtès <ludo@gnu.org> writes: > >> Hi, >> >> Alexey Abramov <levenson@mmer.org> skribis: >> >>> This patch allows users to use /gnu/store objects for AuthorizedKeysCommand >>> and similar options. According to the sshd_config(5): >>> >>>> The program must be owned by root, not writable by group or others, and >>>> specified by an absolute path. >> >> That’s the case with programs in /gnu/store. Why isn’t it working? > > The reason is that safe_path in openssh takes a full path of the file > and checks every directory one by one. The constrain fails on /gnu/store > directory due to write permissions for group. Oh I see, makes sense. [...] >> Also note that the strcmp above is incorrect: it would accept >> /gnu/storesomethinglese. You probably need to add a trailing slash to >> be sure. > > Let me correct myself. In the previous email I wrote that the safe_path > goes from top to bottom, but actually it walking upwards. This is an > actual loop [...] > As you can see, buffer is holding the result of dirname already, hence > I used "/gnu/store". Sounds good then! Thanks for explaining, Ludo’.