Blowing Off Steam: Let's plan steam-support interface

We’re actually not back to square one; the conflicting x modifiers is part of the reason why I suggested not jumping on this PR just yet. :wink: We’ll need to carve out the paths in at least SNAP_USER_COMMON. removable-media should be ok though since we don’t have ix rules in there. Does the steam client actually need home? My understanding was it actually only needed SNAP_USER_COMMON, but if it does, we need to do the same for home.

I’m not sure what you mean by “carve out the paths” - can you elaborate please?

And we’ll need to allow home for now for people having setups like ~/SteamLibrary that are migrating to the snap

Or we allow ~/SteamLibrary in steam-support and steam-game (eg, is all of home actually needed?).

And by carve out, I mean adjust the default policy/affected interfaces that cause the conflicting x modifiers to specify ix for those paths so that steam-support can specify Px for them.

This is what I was going to provide to you and was trying to shield you from. :wink:

Well ~/SteamLibrary was a totally arbitrary path, basically we care about:

@{HOME}/**/steamapps/common/*/** ixm
{,run/}media/**/steamapps/common/*/** ixm

Exact bits might be different but you get the idea. :]

I’ll provide the carving out to you (still have some investigating to do; also why I wanted to wait for comment), but I’ll need the exact bits. If I can count on something like ‘/steamapps/common/’ then it should be ok, but if it can be literally anything then no. If it can be literally anything, we can also make it so it works for where the snap expects it to be, and allow steam-support to migrate to the expected locations (this is actually closer to what we do for other snaps).

The games being launched will always be under /steamapps/common/ paths. Steam Client itself will follow normal rules for living inside SNAP_USER_COMMON

1 Like

Hrmm, I forgot about https://launchpad.net/bugs/1696552 which prevents us from using the px rule approach (the cx rule approach is still valid). I really like the separation of the plugs though so I’m going to think through this and see if I can make this work with cx rules.

2 Likes

Ok, I think this is workable if we have yaml like this:

name: steam
plugs:
  steam-game:
    steam-client: client 
apps:
  client:
    plugs:
    - steam-support
    - network
    - removable-media
  child:
    plugs:
    - steam-game
    - network
    - removable-media
    - joystick
    - ...

The apparmor profile for ‘client’ works like always. The apparmor profile for ‘game’ is slightly different in that the ###PROFILEATTACH### looks like this:

profile snap.steam.client//game { ... }

where client comes from the steam-game interface attribute steam-client and game is hard-coded in the interface. Put another way, we need to tell snapd that the child command is actually a child profile for client, so we use an interface attribute for that.

@ikey, please don’t implement this yet. When we got feedback on the design, I will create a branch off of your PR and do the heavy lifting so you can run with it.

This is all super crazy stuff! I will certainly hold back until the smoke has cleared, thank you! :slight_smile:

1 Like

Getting back to this now.

<TLDR;>

Due to limitations with the AppArmor parser and the design of snapd, we (IMO) have essentially two options:

  1. The Px/px idea is the cleanest implementation, but requires patching apparmor parser to fix bug #16965522. The problem with this approach is propagating that fix everywhere. It is possible for Solus to pick it up and for Ubuntu to SRU, but far less likely Ubuntu derivatives, Debian, SUSE, etc will pick it up. snapd could perform a runtime detection and do something smart-- eg distros with full AppArmor support only expose the interface if the fix is in place (partial AppArmor support doesn’t end up with the rule at all so no problem)
  2. We utilize a child profile with a Cx/cx rule and modify snapd to accommodate this. The least intrusive option is to put the child profile (ie what plugs: [steam-game]) in a separate file '#include’d by the parent (ie, what plugs: [steam-support]) and adjust snapd to load/unload the parent profile with interface connect/disconnect for the child. We also adjust ‘command’ in snap.yaml to either require ‘null’ assignment or to be omitted when used with ‘steam-game’

I prefer ‘1’ since it is the cleanest implementation, but need an architect to decide if it is ok from a cross-distro perspective. ‘2’ is not as clean, but I think it can be done in a way that isn’t terrible from an implementation perspective (with a little design for the finer points (which can be done in PR review)).

Full context

jjohansen and I discussed options with apparmor wrt Px and Cx rules.

Because of the apparmor bug with Px/px, we can’t use the Px/px idea today. We could fix the parser for the bug, but that won’t propagate to everywhere snapd is supported (we could SRU for Ubuntu, but Solus, SUSE, Arch, Debian, Ubuntu derivatives, etc may not have the fix), but then the snap would operate differently on different distros. To remedy this, we could surface the interfaces on if the parser had the fix (it could be a runtime check).

The alternative is to use a child profile in some fashion. In practice, this results in variations on two choices:

  1. embed the child profile in the parent (eg profile snap.snap-client.snap-client { game {} })
  2. put the child profile in a separate file

‘1’ might work with snap.yaml like the following:

name: steam-client
apps:
  steam-client:
    command: $SNAP/bin/client
    plugs:
    - steam-support
    - network
    - removable-media
    - home
    - desktop
    - browser-support
    - ...

With this, we have steam-support put one set of rules in the parent profile and another set in the child profile and we adjust snapd so that all the other interfaces are added to both. This is a somewhat complicated proposition within snapd in terms of adjusting interface connections, but not impossible. It also suffers from the fact that the client and the game have all the same interfaces (differing only in what stream-support conditionally puts in each).

‘2’ might work with snap.yaml like the following:

name: steam-client
apps:
  steam-client:
    command: $SNAP/bin/client
    plugs:
    - steam-support
    - network
    - desktop
    - home
    - removable-media
  game:
    command: ???
    plugs:
    - steam-game
    - network
    - desktop
    - home
    - ...

With this, the client and the game can have different plugs. Because the child profile for ‘game’ is in another profile, we either need to:

  • be cognizant of load order (parent then child) and unload order (child then parent) and lexically name the profile such that the cache files load in the right order. This would require fairly involced changes to snapd profile load/unload
  • have the parent ‘#include’ the child and have snapd ignore loading/unloading the child. This requires simpler changes to snapd

With ‘2’ , in order to avoid more complicated changes to snap run, snap-confine, etc for the profile name being snap.steam-client.steam-client//game, we also have to decide how to deal with the fact that ‘steam-game’ is not actually a command and that it is a child profile of ‘steam-client’. One way to handle this is by ignoring command when used with steam-game. Eg:

name: steam-client
plugs:
  steam-support:
    child-command: game
apps:
  steam-client:
    command: $SNAP/bin/client
    plugs:
    - steam-support
    - ...
  game:
    command: null
    plugs:
    - steam-game
    ...

@niemeyer - if you could decide on the general approach, I can run with it and discuss implementation details in a PR.

Anyone running WINE apps are likely to need ptrace, similar to steam, because of the way that WINE operates. In my opinion, therefore, it would be better to come up with a generic interface which allows ptrace-of anything within a snap app’s label, e.g. ‘snap.cncredalert.cncredalert’ for my current work using WINE which dies a horrid death from lack-of-mortality (signal 31).

Restricting to app label will prevent other apps in the same snap from ptracing each other (good) but allow a hierarchy started from each app definition to ptrace within their own chain:

apps:
  ptracable:
    command: foo
    plugs: [ptrace]
  other-ptracable:
    command: bar
    plugs: [ptrace]
  • ptracable can ptrace itself only
  • other-ptracable can ptrace itself only
  • both can ptrace other processes started directly by them, but not by each other

ptrace in the general case needs to be explored, and it is something on my list. The problem is that on <4.8 kernels, you can abuse ptrace to break out of the seccomp sandbox. solus uses a 4.9 kernel, so they are not affected, and this interface, today, is meant for them. Once we figure the steam-support interface out for Solus, I’ll circle back to the general ptrace case.

Your idea may very well be what we do, but the problem with steam and why I’m exploring all these options is that the steam client launches steam games, therefore, because of the ix rules, the steam client and the games all have the same label, so allowing ptrace of the same label doesn’t work for this case. We need to come up with a method for steam where we can wrap an unmodified steam client in apparmor such that when it launches unmodified games, there is a profile transition to the game profile, which has a different label than the client profile.

3 Likes

@niemeyer and I had a long and productive conversation on steam-support specifically and also how the ideas and concerns we’ve been discussing might apply generally for other interfaces and how it all fits into the larger snapd roadmap.

The essence of my input in this topic prior to the meeting today with @niemeyer is to have the steam-support interface perform profile exec transitions based on game installation paths so that we could have different policy for the client and the games. Thinking about how that maps generally to arbitrary interfaces, we agreed that a potential medium-term snapd feature would allow interfaces to do something interface-specific for declared cases. For example, consider the following snap.yaml (not approved, not final and still to-be-designed; specifically, the ‘paths’ attribute will almost certainly not be the finalized design):

plugs:
  plug-name:
    paths:
    - $SNAP/bin/bar
    - $SNAP_USER_DATA/baz/*
apps:
  foo:
    plugs: [ plug-name ]

What the above is saying is that upon interface connection, most things for the foo command get the defaults for the plug-name interface, but things matching paths will get something different that is interface-dependent (eg, generate another profile and setup a profile exec transition to it).

Because the above needs design and careful consideration for the medium term, we can unblock the steam-support interface today and work through the submitted PR without implementing profile transitions. When the above feature is in place, we can revisit steam-support to use it. In terms of security I’d like to note that Solus can consider (if they haven’t already) enabling YAMA in the kernel and setting kernel.yama.ptrace_scope=1 via sysctl by default (this is the default in Ubuntu since 10.10), which will prevent games launched by the steam client from ptracing the client but allow the games to ptrace their children.

@ikey - is unblocking the existing PR for the short-term and revisiting in the medium term acceptable?

I still need help with that PR because we ran into issues with it. I need it so that it doesn’t break core basically. Its “just” another interface. In the mean time I’ll get the kernels ptrace stuff sorted and post back once that part is done.

1 Like

Kernel changes going into Solus now:

https://dev.solus-project.com/R1966:a14974909befe074b84d031e9ebb54be14d5c22f

https://dev.solus-project.com/R3571:111a2f96513e702f6f8384855864a830caccd8d9

We’ll need to ensure the tooling is adjusted for yama and then make it default

Followed up by finalising that initial PR so we can make steam-support a thing and move off of devmode.

1 Like

I plan to take a look at this tomorrow.

For others following along, did an extensive review today. After this it should be small iterations until we’re done.

1 Like