1355274 - PulseAudio libasyncns calls socketpair(AF_UNIX, SOCK_DGRAM, 0) in sandboxed content process

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Part of the PulseAudio stack is an asynchronous DNS resolver library, libasyncns (http://0pointer.de/lennart/projects/libasyncns/).  It seems to use Unix-domain datagram sockets, possibly for asynchronous inter-thread communication.

We forbid creating such sockets where it's possible to do so with seccomp-bpf (i.e., on platforms that aren't 32-bit x86) because they can be combined with sendto() or sendmsg() to send to any named datagram socket in the domain; see bug 1066750 for details.

Certainly this doesn't break PulseAudio for the basic case of using local sound hardware.  My guess is that this occurs only when it's configured as a client of a remote sound server, but I haven't tried testing that yet.

Longer-term we want to stop using PulseAudio directly in content processes, but we're not there yet.  Shorter-term we can either:

1. Allow socketpair for content until we're ready to take away socket() and connect(), because those are strictly worse than the socketpair+sendmsg thing; or

2. Substitute a SOCK_SEQPACKET socketpair instead and see if PulseAudio even notices the difference.

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

STR:
1. Start Firefox with the env var PULSE_SERVER set to the name of a host that's running PulseAudio but doesn't have module-native-protocol-tcp loaded
2. Load some non-WebAudio page to get a content process started.
3. On the PulseAudio server, do "load-module module-native-protocol-tcp auth-anonymous=1" (warning: this disables authentication and allows anything that can connect to the host to play or record audio; authenticating the client is left as an exercise for the reader)
4. Navigate to a page that uses WebAudio (e.g., https://www.youtube.com/watch?v=D2SoGHFM18I)

(A simpler STR for the crash is just to set PULSE_SERVER to a host that's not listening for PulseAudio-over-TCP, but that doesn't help test that fix makes audio actually work.)

More generally: we try to pre-initialize PulseAudio before starting sandboxing, so the simple case of using a remote audio server seems to work, but if the server isn't up when the content process is started, or the connection is dropped for some reason, the content process will try to reconnect while sandboxed.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1355274 - Polyfill SOCK_DGRAM socketpairs with SOCK_SEQPACKET, for libasyncns.

Review commit: https://reviewboard.mozilla.org/r/140774/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/140774/

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8869146 [details]
Bug 1355274 - Polyfill SOCK_DGRAM socketpairs with SOCK_SEQPACKET, for libasyncns.

https://reviewboard.mozilla.org/r/140774/#review146090

This doesn't seem like it would make anything worse, but does the socketpair call actually do anything useful? I mean, switching to SOCK_SEQPACKET will change the underlying protocol from UDP to SCTP, if I'm not misremembering. I would expect that not to be what the PA server on the other side expects. Does this trick actually work?

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #3)
> This doesn't seem like it would make anything worse, but does the socketpair
> call actually do anything useful? I mean, switching to SOCK_SEQPACKET will
> change the underlying protocol from UDP to SCTP, if I'm not misremembering.
> I would expect that not to be what the PA server on the other side expects.
> Does this trick actually work?

This applies only to socketpair() and Unix-domain sockets, which libasyncns seems to be using internally for thread synchronization.  When the resolver library uses socket()/connect()/etc. in the Internet domain to talk to nameservers, it will use UDP or TCP as usual.  This also doesn't affect the Internet-domain socket()/connect()/etc. for communicating with the PulseAudio server.

And I did confirm that this works — comment #1 has the test procedure.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9920ad022326
Polyfill SOCK_DGRAM socketpairs with SOCK_SEQPACKET, for libasyncns. r=gcp

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9920ad022326

Comment 7

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(I'll request uplift once this has had a few days on m-c.)

Comment 8

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8869146 [details]
Bug 1355274 - Polyfill SOCK_DGRAM socketpairs with SOCK_SEQPACKET, for libasyncns.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1337162 (Linux content sandboxing allowed to ride the trains)
[User impact if declined]: Using a remote audio server on Linux can unexpectedly stop working until Firefox is restarted.  This is probably a small part of the userbase, but we know from telemetry and crash-stats that it's nonzero.
[Is this code covered by automated tests?]: Not this specific case, because it needs system-level setup.  (We do have test coverage of sandboxing in general and non-remote audio, of course.)
[Has the fix been verified in Nightly?]: Yes; it's been on Nightly for a week now.
[Needs manual test from QE? If yes, steps to reproduce]: Comment #1 has STR but there's some setup needed (although it'd probably work on a single machine with "localhost" as the audio server).  I can verify it if need be.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: (1) If there's some other configuration where a library uses this system call with these arguments, it's uncommon enough that nobody on Nightly has filed a crash report in the past 6 months.  (2) While this patch doesn't restore exactly the unsandboxed behavior, it's equivalent for reasonable use cases.  (3) Anything affected by this patch would have failed (or, on Nightly, crashed) without it, so even if the previous two very unlikely conditions are satisfied, it's even more unlikely that this would make the regression worse.
[String changes made/needed]: None


(Yes, I just went through 35 crash reports by hand to make sure they were all about libasyncns.)

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8869146 [details]
Bug 1355274 - Polyfill SOCK_DGRAM socketpairs with SOCK_SEQPACKET, for libasyncns.

The volume of crashes in 54 is very low and it's very late in Beta54. Let's let it ride the train and won't fix in 54. Beta54-.

Comment 10

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

For reference, another workaround (besides restarting the browser, or switching to a fixed version like Beta 55) would be to use a local PulseAudio instance with the "tunnel" modules[1][2] as a relay.

[1] https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/User/Modules/#index14h3
[2] https://www.freedesktop.org/wiki/Software/PulseAudio/Documentation/User/Network/#index2h2