Re: Local provider - isolating sudo usage
On 25 January 2014 01:58, Andrew Wilkins andrew.wilk...@canonical.com wrote: On Sat, Jan 25, 2014 at 1:24 AM, roger peppe roger.pe...@canonical.com wrote: On 24 January 2014 10:59, Andrew Wilkins andrew.wilk...@canonical.com wrote: On Fri, Jan 24, 2014 at 11:38 PM, roger peppe roger.pe...@canonical.com wrote: On 24 January 2014 01:14, Andrew Wilkins andrew.wilk...@canonical.com wrote: I removed this bits that chown to the user from the local provider. I can't, unfortunately, easily remove the only other remaining part: chowning the ~/.juju/ssh dir and keys. Suggestions welcome. There's also a Chown in environs/configstore that I'd very much like to see go. Thanks, I missed that one. Could you expand on why it's hard to avoid chowning the ~/.juju/ssh dir for someone that's not that familiar with this area? AFAICS the writeAuthorizedKeys function that creates the directory is called by AddKeys, which is called directly from cmd/juju, which will be running as the correct user. What am I missing? writeAuthorisedKeys is not the problem, it's utils/ssh.LoadClientKeys that causes grief. This function will create ~/.juju/ssh and a key pair inside it if they don't exist. This function is called by juju.InitJujuHome, so very early on in the process. Doing it in InitJujuHome felt dirty, but I couldn't think of a better place at the time. More on this in a moment... Ah, InitJujuHome definitely seems like a not-so-great place for this. I use that function in various client programs, and I would not expect it to have side-effects. When I encountered this problem, I wondered whether we could just prevent root from executing the CLI at all (by erroring out, not by any OS mechanism). This won't work with the local provider as it is, as Destroy must be run as root. Destroy calls back into the CLI via sudo. This could be changed, at the cost of making destruction more complicated. On reflection, after you mentioned configstore, I'm thinking that perhaps LoadClientKeys could be called in environs.ConfigForName (or nearby), with a sync.Once. We could then disallow preparing an environment as the root user, which covers both the configstore case and the LoadClientKeys one. What do you think about that option? Rather than add side-effects onto existing functions, could we not add an explicit call, say EnsureClientKeys, and call it from juju bootstrap? I'm happy to rename. Calling it at bootstrap time doesn't help, though, I think. authorized_keys is determined prior to calling Prepare, and the environments dir is created even before that, I think. I'd check for sure, but about to hop on a plane... I think I wasn't clear there. When I said call it from juju bootstrap, meant call it directly from BootstrapCommand.Run, just before the call to environs.PrepareFromName. We'd add the same call to SyncToolsCommand too. An alternative might be to add it as another side-effect to PrepareFromName, but that seems wrong - its only side-effects are currently on the storage interface that's passed in. cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Local provider - isolating sudo usage
On 24 January 2014 01:14, Andrew Wilkins andrew.wilk...@canonical.com wrote: I removed this bits that chown to the user from the local provider. I can't, unfortunately, easily remove the only other remaining part: chowning the ~/.juju/ssh dir and keys. Suggestions welcome. There's also a Chown in environs/configstore that I'd very much like to see go. Could you expand on why it's hard to avoid chowning the ~/.juju/ssh dir for someone that's not that familiar with this area? AFAICS the writeAuthorizedKeys function that creates the directory is called by AddKeys, which is called directly from cmd/juju, which will be running as the correct user. What am I missing? cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Local provider - isolating sudo usage
On Sat, Jan 25, 2014 at 1:24 AM, roger peppe roger.pe...@canonical.comwrote: On 24 January 2014 10:59, Andrew Wilkins andrew.wilk...@canonical.com wrote: On Fri, Jan 24, 2014 at 11:38 PM, roger peppe roger.pe...@canonical.com wrote: On 24 January 2014 01:14, Andrew Wilkins andrew.wilk...@canonical.com wrote: I removed this bits that chown to the user from the local provider. I can't, unfortunately, easily remove the only other remaining part: chowning the ~/.juju/ssh dir and keys. Suggestions welcome. There's also a Chown in environs/configstore that I'd very much like to see go. Thanks, I missed that one. Could you expand on why it's hard to avoid chowning the ~/.juju/ssh dir for someone that's not that familiar with this area? AFAICS the writeAuthorizedKeys function that creates the directory is called by AddKeys, which is called directly from cmd/juju, which will be running as the correct user. What am I missing? writeAuthorisedKeys is not the problem, it's utils/ssh.LoadClientKeys that causes grief. This function will create ~/.juju/ssh and a key pair inside it if they don't exist. This function is called by juju.InitJujuHome, so very early on in the process. Doing it in InitJujuHome felt dirty, but I couldn't think of a better place at the time. More on this in a moment... Ah, InitJujuHome definitely seems like a not-so-great place for this. I use that function in various client programs, and I would not expect it to have side-effects. When I encountered this problem, I wondered whether we could just prevent root from executing the CLI at all (by erroring out, not by any OS mechanism). This won't work with the local provider as it is, as Destroy must be run as root. Destroy calls back into the CLI via sudo. This could be changed, at the cost of making destruction more complicated. On reflection, after you mentioned configstore, I'm thinking that perhaps LoadClientKeys could be called in environs.ConfigForName (or nearby), with a sync.Once. We could then disallow preparing an environment as the root user, which covers both the configstore case and the LoadClientKeys one. What do you think about that option? Rather than add side-effects onto existing functions, could we not add an explicit call, say EnsureClientKeys, and call it from juju bootstrap? I'm happy to rename. Calling it at bootstrap time doesn't help, though, I think. authorized_keys is determined prior to calling Prepare, and the environments dir is created even before that, I think. I'd check for sure, but about to hop on a plane... I think EnsureClientKeys should be called just before the call to utils/ssh.PublicKeyFiles. That way running juju help doesn't automatically create a ~/.juju/ssh directory, for one. And no sync.Once necessary. Might that work? cheers, rog. -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Local provider - isolating sudo usage
That sounds awesome. On Wed, Jan 22, 2014 at 6:12 PM, Andrew Wilkins andrew.wilk...@canonical.com wrote: Hi folks, I'm working on changing the local provider so that sudo is not needed from outside Juju; juju bootstrap and juju destroy-environment will prompt as necessary. I would like to also prevent Juju from allowing the user to run with sudo from the outside. This will allow us to remove all of the code pathways that change ownership to the sudo caller, and avoid future breakages. Does anyone have any strong reasons for not doing this? Cheers, Andrew -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Local provider - isolating sudo usage
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14-01-22 06:12 PM, Andrew Wilkins wrote: I would like to also prevent Juju from allowing the user to run with sudo from the outside. This will allow us to remove all of the code pathways that change ownership to the sudo caller, and avoid future breakages. As a user who has to work around the chown bug frequently, that sounds great to me. https://bugs.launchpad.net/juju-core/+bug/1245647 Aaron -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlLhIawACgkQ0F+nu1YWqI0x9QCfa/zZu9j67p6KQ+C7T6/4qxvw Zq0AniDjFTalu0P4RC48htZ+1IX2Z/oQ =a+OP -END PGP SIGNATURE- -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Local provider - isolating sudo usage
Thank you! Let's get rid of every chown in the code base. On 22 January 2014 23:12, Andrew Wilkins andrew.wilk...@canonical.com wrote: Hi folks, I'm working on changing the local provider so that sudo is not needed from outside Juju; juju bootstrap and juju destroy-environment will prompt as necessary. I would like to also prevent Juju from allowing the user to run with sudo from the outside. This will allow us to remove all of the code pathways that change ownership to the sudo caller, and avoid future breakages. Does anyone have any strong reasons for not doing this? Cheers, Andrew -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Local provider - isolating sudo usage
The core local provider changes have been merged now. For a summary of how the local provider bootstraps now, see: https://codereview.appspot.com/55880043/ I removed this bits that chown to the user from the local provider. I can't, unfortunately, easily remove the only other remaining part: chowning the ~/.juju/ssh dir and keys. Suggestions welcome. Cheers, Andrew On Fri, Jan 24, 2014 at 4:34 AM, roger peppe roger.pe...@canonical.comwrote: Thank you! Let's get rid of every chown in the code base. On 22 January 2014 23:12, Andrew Wilkins andrew.wilk...@canonical.com wrote: Hi folks, I'm working on changing the local provider so that sudo is not needed from outside Juju; juju bootstrap and juju destroy-environment will prompt as necessary. I would like to also prevent Juju from allowing the user to run with sudo from the outside. This will allow us to remove all of the code pathways that change ownership to the sudo caller, and avoid future breakages. Does anyone have any strong reasons for not doing this? Cheers, Andrew -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
