ubuntu-appstore-developers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxx>]

On Wed, Jul 17, 2013 at 02:36:42PM -0700, Steve Beattie wrote:
> On Wed, Jul 17, 2013 at 05:55:17PM +0100, Colin Watson wrote:
> > On Wed, Jul 17, 2013 at 08:55:10AM -0500, Jamie Strandboge wrote:
> > > On 07/17/2013 08:01 AM, Colin Watson wrote:
> > > >  * By default, system hooks are expected to handle multiple versions.
> > > >    The %s substitution in a Pattern will expand to PACKAGE_KEY_VERSION,
> > > >    where KEY is a string given in the manifest for each instance of a
> > > >    hook (so for a package with multiple .desktop files it might be the
> > > >    base name of the .desktop file; I'll flesh this out more rigorously
> > > >    in the spec and give examples).  Target path handling is simplified
> > > >    accordingly.
> 
> Ah, if nothing else, the PACKAGE_KEY_VERSION %s substitution is an
> improvement over what I had initially (mis-?)interpreted the click
> documentation to indicate, that the symlink would only contain the
> top level PACKAGE name, which would be a symlink to the top level
> package directory, wherein I then believed the hook would need to
> walk the different versions beneath that.

I think that probably was a misinterpretation of my previous intent, but
it really wasn't very clearly specified anyway.

> > It's also occurred to me that, really, I've been talking about hooks the
> > wrong way up for the multi-application case.  We don't have a set of
> > hooks each of which can be applied to several applications; we have a
> > set of applications which can make use of several hooks.  This becomes
> > obvious when you think about what the current design would require for
> > the common case of a single-app package with an AppArmor profile and a
> > desktop file:
[...]
> > The duplication is clear, and it gets worse the more hooks you use.
> > This means that it ought to be the other way up; and to get rid of the
> > duplication entirely we need to be considering AppArmor profiles as more
> > like other hooks.
> 
> I agree, in that these things are really attributes of the
> applications: the security policy, the desktop file, etc.

Phew.

> This scheme looks good, as long as we can use the symlink name to pull
> out the package, key, and version, as they won't be repeated within
> the apparmor JSON files.

Right, you can safely do that.

> (And keeping JSON for the sub-entries is fine for our use.)
> 
> The only situation I can see where this would be problematic is
> if global level information was added to the manifest format that
> hooks might need or be interested in, or information that multiple
> hooks might want to know. For example, earlier you had proposed
> alternate install locations than /opt/click.ubuntu.com/ for things
> like vendor/carrier installed applications. The apparmor hook would
> want exposure to this somehow, but having the author/sdk repeat this
> information both at a global level and within the security json
> for each application within the package would potentially lead to
> inconsistencies.

Right.  For this reason, you mustn't hardcode /opt/click.ubuntu.com/ in
the AppArmor hook, or otherwise try to construct package unpack
directories yourself.  Instead, fully dereference ("readlink -f") the
symlink you're given (which will be to apparmor/myapp.json or whatever's
specified), step up until you find a directory containing a .click
subdirectory, and that's the package unpack directory.

(I have a pending merge proposal from Ted to add "click pkg-dir"; I want
to think about that in light of this and see if perhaps I can offer a
general helper.  But for the time being it's OK to do it by hand.)

If files referenced in hooks need to know any of this information, then
I think the right thing to do is to write them in a templated form and
for the hook code to apply substitutions.  I certainly agree that we
should apply a Don't Repeat Yourself rule.

> That in fact is why the apparmor hook would be parsing apart the
> symlink name, as it encapsulates the global information from the
> manifest that apparmor is interested in.

Is this obviated by the algorithm I described above for unpack directory
discovery, or do you still need to parse the symlink name for something
else?

> [One nice feature about this scheme from the apparmor perspective
>  is that if the package is for something like the ubuntu-terminal
>  where it's not really acceptable for it to be confined by apparmor,
>  the manifest can just not provide the hook entry, rather than
>  using the somewhat hacky 'unconfined' template like we do now.
>  Manual/automated inspection of manifests can red-flag those, to
>  detect app authors who try to do this. Similarly, secondary apps
>  that don't need/provide desktop files just won't provide that entry.]

Indeed.  That appealed to me too.

> > The one remaining non-generic thing here is as follows: my understanding
> > is that for 13.10 the server is not going to have any facilities for
> > scanning apps for problems and rejecting them.  This means that, IMO, we
> > need to reject such things at the client end.  But that at least is a
> > relatively small amount of code.
> 
> What would a hook be expected to do to reject it, without causing
> the install to fail?

A hook can't do this check because it wouldn't even be called.  I'll
have to do this in click for the time being.  This doesn't have to be a
permanent hack, though; it can be replaced with some configurable notion
of optional vs. required hooks when we have time.

Thanks for the feedback.  I will get going on this today.

-- 
Colin Watson                                       [cjwatson@xxxxxxxxxx]