Changes to interfaces API

The changes to interfaces API landed in master last week (24/03) and all affected PRs (implementing new interfaces) were updated to use the new API. This was a massive change that affected (internally) all currently available interfaces.

With this change we removed ConnectedSlotSnippet, ConnectedPlugSnippet, PermanendSlotSnippet and PermanentSlotSnippet methods and replaced them with per-security backend methods such as AppArmorConnectedSlot, AppArmorConnectedPlug, AppArmorPermanentSlot, AppArmorPermanentPlug (sames goes for SecComp, KMod, DBus, UDev, Systemd, Mount backends). These methods receive a Specification object of a respective backend and call its Add* method to define/add new snippets or policy definitions (e.g. for apparmor it’s AddSnippet(string) method; for kmod backend it is AddModule(string)).

Below is a short depiction of the difference between the old and new approach:

Old approach - four methods for permanent plug/slot snippet and connected plug/slot snippet, serving different security backends via switch/case statements inside the methods (all four methods need to be present even if not needed by given interface):

    func (iface *FooBarInterface) ConnectedPlugSnippet(
                plug *interfaces.Plug,
                slot *interfaces.Slot,
                securitySystem interfaces.SecuritySystem) ([]byte, error) {

        switch securitySystem {
        case interfaces.SecurityAppArmor:
                ...
                return apparmorSnippet, nil
        case interfaces.SecurityDBus:
                ....
                return dbusSnippet, nil
        }
        return nil, nil
    }

New approach - separate methods for each security backend and permanent/connected plug/slot as needed (unused backend do not need to be defined!):

    func (iface *FooBarInterface) AppArmorConnectedPlug(
                spec *apparmor.Specification,
                plug *interfaces.Plug,
                slot *interfaces.Slot) error {

	    spec.AddSnippet(snippet)
        return nil
    }

    func (iface *FooBarInterface) DBusPermanentSlot(
                spec *apparmor.Specification,
                slot *interfaces.Slot) error {

	    spec.AddSnippet(snippet)
	    return nil
    }

The other methods of interface definition (ValidatePlug, ValidateSlot, AutoConnect etc. remain in the new API and are not affected by the changes).

This topic is meant to collect any discussions around these changes, including potential tweaks or bugs found. Possible enhancements we will consider in near future is making some of the specifications strongly-typed, e.g. UDev specification could take a typed rule rather than a string snippet.

2 Likes

@zyga-snapd you had some concerns about network manager apparmor policy being broken (the slot part of it) because of this change and want to fix it; is there a PR for that?

Not sure if that’s what you are referring to. I discussed the network-status interface. As it is written today it can only work if the slot is provided by a snap other than core. Since there are no plans to provide this via core yet I think that’s OK but if this changes the slot side has to be adapted.

Ah, yes, my bad, it was network-status indeed. Thanks for clarifying.

Some more rationale for the change: each security backend represents a subsystem such as apparmor, secccomp, dbus, etc. The previous model we had in place allowed interfaces to provide text snippets to enrich any one of those backends. That was great because it enabled us to make every backend and every interface look a lot like each other, and pretty much all of the subsystems we have operate on text files anyway.

After some time, though, we found specific cases which required taking a little bit more information in the backend to properly put in place the bits that each interface wanted to tweak about the particular backend. For example, we wanted to create unique systemd units that were valid for all occurrences of a given interface, no matter if there was one connection or many. So, we needed more structure, in those cases. We worked around that by providing json as a snippet and marshaling/unmarshaling it across the existing interface, but by then we knew that our approach would have to change at some point.

Recently then, we found other cases where we’d have to do that same sort of trick. So we decided to take a step back and refactor the underlying framework to accommodate those needs. This has just landed in master recently, and now we’re in the process of migrating pre-existing pull requests in GitHub to use the new API, so we don’t need to bother every single author that already had a review underway to do that.

@pstolowski Would you mind to provide some examples of how code looked like before and after this change, so we can point people here for migrations?

Thanks to @pstolowski for the great work handling this change.

1 Like

I’ve updated the 1st post in this topic with an example.

Fixed its formatting a bit.