Auto-connection request for Moonlight `network-observe`

store-requests
1

Moonlight is an open source implementation of NVIDIA’s GameStream protocol. It can stream games from a GameStream-compatible PC to any supported device and play them remotely.

We’ve recently moved to core20 on the edge channel, which broke PC autodiscovery via mDNS. Autodiscovery is the first step in setting up streaming, and saves users from looking up and manually entering the IP address of the remote PC.

The issue seems caused by the update to a newer Qt, as described in Moonlight - mDNS broken with AppArmor denials for AF_NETLINK since update to Qt 5.12. Based on that discussion, I’d like to request auto-connection of the network-observe interface, please.

Thanks in advance!

2

+1 for auto-connection. @reviewers - can others please vote?

3

@jdstrand - I wonder if this will start to affect more snaps that move to core20 - should the desktop interface be updated to allow this common access rather than lots of snaps potentially having to request network-observe?

1 Like
4

+1 for auto-connect from me, as well.

5

I second this question, it makes me a little uncomfortable granting network-observe for a use-case that feels like it should be covered under a less privileged interface.

6

I don’t think this is core20 specific. The old api is still there for apps that want it, and they can choose this one that needs network-observe or the other one that has always needed network-manager-observe. Not all Qt apps need this so it wouldn’t be right to put it in desktop or desktop-legacy. Qt is a large set of APIs that snaps may choose and IMO network-observe is still the right choice for snap that use this API. If this becomes onerous, we can revisit (but it shouldn’t be, we’re accustomed to granting network-manager-observe for publishers that request it).

7

Hello, I’m the upstream developer of Moonlight.

I think there is a bit of confusion regarding the actual change between Qt 5.9 and Qt 5.14. The API that seems to be the problem here is QNetworkInterface. The code on Moonlight’s end is identical between Qt 5.9 and 5.14. There are no new Qt APIs or anything that we’re using on 5.14. We’re using QNetworkInterface just like we did before, and it broke because Qt 5.14 in core20 was built with the AF_NETLINK backend for QNetworkInterface.

We use QNetworkInterface in 3 places.

The first one is to send Wake-on-LAN broadcast packets via all connected interfaces to ensure it reaches the target PC on multi-homed systems. We do this by find the local IP address and subnet of each interface and calculating the subnet broadcast address to use when sending our WOL packets.

The second is a VPN detection heuristic that lets us reduce our MTU slightly to ensure we’re not causing IP fragmentation when the VPN encapsulates our video traffic. The code there uses the MTU, MAC address (some VPN interfaces have common prefixes), interface type, and interface name to determine if the interface we’re connected through is likely a VPN.

The third (and most critical one) is in our QMdnsEngine submodule. mDNS is a multicast protocol and in order to join a multicast group, you should specify an interface. Without an interface, you’re not guaranteed where your mDNS packets will go out on multi-homed systems. Because QNetworkInterface::allInterfaces() returns an empty list, we can’t join a multicast group or specify the outbound multicast interface and the mDNS packets don’t get sent/received.

None of these things (except maybe #2) really seem like something normal apps shouldn’t be doing. Getting local interface IP addresses doesn’t seem like something that should be a privileged operation (especially since one can simply call getsockname() on a connected socket to accomplish the same thing). It’s basic stuff that getifaddrs() could do. I think it’s likely that other Qt apps will run into this issue as well.

The bigger risk in my opinion is that Qt apps will subtly break when updating to core20. We caught this because the mDNS breakage was obvious, but imagine if the only breakage was something less clear, like our VPN heuristic or Wake-on-LAN multi-homed support. It might be that the app now connects via an internet relay rather than directly via LAN, or that your streaming performance over a VPN gets slightly worse, or that Wake-on-LAN doesn’t work consistently for some users. It’s just functional enough to escape the developer’s attention, but the user experience will suffer.

Apologies, I wanted to provide more links to the Moonlight source and Qt documentation, but I can’t post more than 2 links since I’m a new user.

8

Thanks for the additional information. While I don’t think that this should be part of the desktop interface, it could maybe be added to the network interface. I’ll investigate and consider this for the 2.45 batch of policy updates.

9

Wrt getifaddrs(), that requires use of NETLINK_ROUTE with the apparmor rule network netlink raw, which is allowed by the ‘network’ interface. AIUI from your description this addresses your use case ‘1’ and ‘3’ and parts of ‘2’. Qt’s change to netlink uses qt_safe_socket(AF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE), and the issue isn’t actually the seccomp rule for NETLINK_ROUTE, but for the missing apparmor rule of network netlink dgram,. When I was looking at Moonlight - mDNS broken with AppArmor denials for AF_NETLINK since update to Qt 5.12, I didn’t consider this fully.

network-observe does allow network netlink dgram, which is why plugging it works.

Before the netlink change, Qt used qt_safe_socket(AF_INET, SOCK_STREAM, IPPROTO_IP) and the seccomp policy allows socket AF_INET - - while apparmor allows network inet stream,. Combine this with ioctls on the socket like SIOCGIFHWADDR and SIOCGIFMTU, and you can get the mac address and mtu with only the ‘network’ interface.

Our ioctl mediation currently relies a lot on Linux capabilities (and the initial syscall for the the fd to pass into ioctl() of course). In an ideal world, we would’ve only allowed SIOCGIFHWADDR in the network-observe interface all along, but if we had, we likely would’ve had this conversation sooner.

Finally, man 7 netlink states "Netlink is a datagram-oriented service. Both SOCK_RAW and SOCK_DGRAM are valid values for socket_type. However, the netlink protocol does not distinguish between datagram and raw sockets." It is therefore IMO a bug in the policy that we allow netlink raw and not netlink dgram in the network interface.

Altogether, we need to update the network interface to have network netlink dgram,.

10

Considering this, I’m changing my vote to -1 for auto-connecting network-observe (sorry @maxiberta for the hoop jumping, but thanks for bringing this up!). IMO, for the time being it is fine to continue to plugs network-observe for people to test your new builds until 2.45 is out.

@alexmurray, @Igor and @kyrofa - can you please consider this new information and recast your vote?

11

Agreed. -1 to auto-connection for network-observe, let’s fix network.

12

Ah yes, network is much more appropriate than desktop :slight_smile:

1 Like
13

-1 from me also for network-observe since snapd 2.45 will allow this with just network. -3 votes for network-observe for moonlight. Not processing this request.

14

Fyi, https://github.com/snapcore/snapd/pull/8793

2 Likes