Local provider - isolating sudo usage

[roger peppe]

On 25 January 2014 01:58, Andrew Wilkins <andrew.wilkins at canonical.com> wrote:
> On Sat, Jan 25, 2014 at 1:24 AM, roger peppe <roger.peppe at canonical.com>
> wrote:
>>
>> 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?
>
>
> 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.