Local provider - isolating sudo usage

[roger peppe]

On 24 January 2014 10:59, Andrew Wilkins <andrew.wilkins at canonical.com> wrote:
> On Fri, Jan 24, 2014 at 11:38 PM, roger peppe <roger.peppe at canonical.com>
> wrote:
>>
>> On 24 January 2014 01:14, Andrew Wilkins <andrew.wilkins at 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?

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.