interfaces: add unconfined access to modem-manager

[alfonsosanchezbeato]

Let unconfined apps access modem-manager DBus interface, and also let
the plug access modem-manager in classic.

[jdstrand]

Thanks for the updates to this interface. There are a few requested changes. In addition, now that you are allowing unconfined to talk to modem-manager, you need to update modemManagerPermanentSlotDBus to not open a hole since policykit isn't available on all snaps. Please update modemManagerPermanentSlotDBus to have only:

<policy user="root">
    <allow own="org.freedesktop.ModemManager1"/>
    <allow send_destination="org.freedesktop.ModemManager1"/>
</policy>

Then create modemManagerConnectedPlugDBus to have:

<policy context="default">                                                      
    <deny own="org.freedesktop.ModemManager1"/>                                       
    <deny send_destination="org.freedesktop.ModemManager1"/>
</policy>   

Then adjust ConnectedPlugSnippet() to use modemManagerConnectedPlugDBus. See the udisks2 interface for an example.

Also, please add tests that search for the above udev snippets. These are currently lacking and we want to make sure that modemManagerPermanentSlotDBus is always applied and modemManagerConnectedPlugDBus conditionally applied on slot connection.

[jdstrand]

I liked the previous logic before-- if OnClassic, use the modemManagerConnectedPlugAppArmor policy, but with the unconfined label, otherwise use it with the slot label. What you have here is inflated policy on classic where you are allowed to talk to the modem-manager snap's label, which won't exist on classic.

AFAICT, you can drop the changes to ConnectedPlugSnippet, drop the modemManagerConnectedPlugAppArmorClassic variable and add this to modemManagerConnectedPlugAppArmor:

dbus (receive, send)
    bus=system
    path=/org/freedesktop/ModemManager1{,/**}
    interface=org.freedesktop.DBus.*
    peer=(label=###SLOT_SECURITY_TAGS###),

The changes to modemManagerPermanentSlotAppArmor can stay.

[alfonsosanchezbeato]

@jdstrand, thanks for the review. Branch refreshed with changes.

Additionally I had to change basedeclaration to allow connecting mmcli to MM daemon.

[jdstrand]

I realize you did this to work around https://bugs.launchpad.net/bugs/1640874, but this must be removed since the base declaration was working as intended (ie, it is meant to require a snap declaration for connections to be allowed to modem-manager).

[alfonsosanchezbeato]

Thanks for the review. I have removed the basedeclaration changes. Additionally, I have changed the policy again for plugs in classic, so it is now equivalent to the one in the ofono interface (that is, I consider it is possible to have a confined client using either the m-m snap or the deb on a classic system).

[jdstrand]

Can you add one more test TestConnectedPlugSnippedUsesUnconfinedLabelNotOnClassic that makes sure "peer=(label=unconfined)," is not present when release.OnClassic = false?

[alfonsosanchezbeato]

@jdstrand, test added

[morphis]

@mvo5 @niemeyer Not really sure where those autopkg test failures come from. Are they expected to be stable already? If not can you merge this PR?

[mvo5]

👍