(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #0)

[...]

• socketpair with SOCK_DGRAM, which we handled by transparently substituting
  SOCK_SEQPACKET (bug 1355274)

It seems like we have code triggering that as well now, it is not just from pulse/libasyncns:

(gdb) bt
#0  mozilla::SandboxPolicyCommon::SocketpairDatagramTrap(sandbox::arch_seccomp_data const&, void*) (aArgs=..., aux=0x0) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/linux/SandboxFilter.cpp:449
#1  0x00007f8a0b2ee04b in sandbox::Trap::SigSys(int, siginfo_t*, ucontext_t*) (this=<optimized out>, nr=<optimized out>, info=<optimized out>, ctx=<optimized out>) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/chromium/sandbox/linux/seccomp-bpf/trap.cc:251
#2  0x00007f8a0b322db4 in mozilla::SigSysHandler(int, siginfo_t*, void*) (nr=31, info=0x7f89fc26c4f0, void_context=0x7f89fc26c3c0) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/linux/Sandbox.cpp:135
#3  0x00007f8a0b28b140 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#4  0x00007f8a0ae7e4aa in socketpair () at ../sysdeps/unix/syscall-template.S:120
#5  0x00007f8a0b32b8c8 in mozilla::ContentSandboxPolicy::FakeSocketTrapCommon(int, int, int) (domain=<optimized out>, type=524290, protocol=0) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/linux/SandboxFilter.cpp:1007
#6  mozilla::ContentSandboxPolicy::FakeSocketTrap(sandbox::arch_seccomp_data const&, void*) (aArgs=<optimized out>, aux=<optimized out>) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/linux/SandboxFilter.cpp:1015
#7  0x00007f8a0b2ee04b in sandbox::Trap::SigSys(int, siginfo_t*, ucontext_t*) (this=<optimized out>, nr=<optimized out>, info=<optimized out>, ctx=<optimized out>) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/chromium/sandbox/linux/seccomp-bpf/trap.cc:251
#8  0x00007f8a0b322db4 in mozilla::SigSysHandler(int, siginfo_t*, void*) (nr=31, info=0x7f89fc26d1f0, void_context=0x7f89fc26d0c0) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/security/sandbox/linux/Sandbox.cpp:135
#9  0x00007f8a0b28b140 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#10 0x00007f8a0ae7e477 in socket () at ../sysdeps/unix/syscall-template.S:120
#11 0x00007f8a0ae7e593 in __opensock () at ../sysdeps/unix/sysv/linux/opensock.c:65
#12 0x00007f8a0ae96744 in __if_indextoname (ifindex=ifindex@entry=4294967295, ifname=ifname@entry=0x7f89fc26d720 "") at ../sysdeps/unix/sysv/linux/if_index.c:226
#13 0x00007f8a01524a89 in sctp_userspace_get_mtu_from_ifn (if_index=1, if_index@entry=4294967295, af=123) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/netwerk/sctp/src/netinet/sctp_userspace.c:105
#14 0x00007f8a01516f9e in sctp_add_addr_to_vrf (vrf_id=vrf_id@entry=0, ifn=ifn@entry=0x0, ifn_index=ifn_index@entry=4294967295, ifn_type=<optimized out>, ifn_type@entry=0, if_name=0x7f89fe42a454 "conn", ifa=ifa@entry=0x0, addr=0x7f89fc26d808, ifa_flags=0, dynamic_add=0)
    at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/netwerk/sctp/src/netinet/sctp_pcb.c:594
#15 0x00007f8a0153d29f in usrsctp_register_address (addr=<optimized out>) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/netwerk/sctp/src/user_socket.c:3153
#16 0x00007f8a0154a5d5 in mozilla::DataChannelConnection::Init(unsigned short, unsigned short, mozilla::Maybe<unsigned long> const&)::$_1::operator()() const (this=0x7f89fa923420) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/netwerk/sctp/datachannel/DataChannel.cpp:608
#17 mozilla::detail::RunnableFunction<mozilla::DataChannelConnection::Init(unsigned short, unsigned short, mozilla::Maybe<unsigned long> const&)::$_1>::Run() (this=0x7f89fa923400) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/obj-browser/dist/include/nsThreadUtils.h:534
#18 0x00007f8a00f3429e in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f8a0ab9abb0, aMayWait=<optimized out>, aResult=<optimized out>) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/xpcom/threads/nsThread.cpp:1153
#19 0x00007f8a00f37e38 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x1, aMayWait=true) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/xpcom/threads/nsThreadUtils.cpp:548
#20 0x00007f8a01052bbf in mozilla::net::nsSocketTransportService::Run() (this=0x7f89fcb1f580) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/netwerk/base/nsSocketTransportService2.cpp:1200
#21 0x00007f8a010537fd in non-virtual thunk to mozilla::net::nsSocketTransportService::Run() () at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/netwerk/base/nsSocketTransportService2.cpp:1279
#22 0x00007f8a00f3429e in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f8a0ab9abb0, aMayWait=<optimized out>, aResult=<optimized out>) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/xpcom/threads/nsThread.cpp:1153
#23 0x00007f8a00f37e38 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x1, aThread@entry=0x7f8a0ab9abb0, aMayWait=false) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/xpcom/threads/nsThreadUtils.cpp:548
#24 0x00007f8a015d6868 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (this=0x7f89fcb82f00, aDelegate=0x7f89fc26dc70) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/ipc/glue/MessagePump.cpp:300
#25 0x00007f8a0155cfa6 in MessageLoop::RunInternal() (this=0x80002) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/ipc/chromium/src/base/message_loop.cc:335
#26 MessageLoop::RunHandler() (this=0x80002) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/ipc/chromium/src/base/message_loop.cc:328
#27 MessageLoop::Run() (this=0x80002) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/ipc/chromium/src/base/message_loop.cc:310
#28 0x00007f8a00f31b57 in nsThread::ThreadFunc(void*) (aArg=0x7f89fcb85940) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/xpcom/threads/nsThread.cpp:395
#29 0x00007f8a0ad77cd5 in _pt_root (arg=0x7f89fc08edc0) at /home/alexandre/Documents/codaz/Mozilla/MiscWork/mozilla-source/mozilla-unified/nsprpub/pr/src/pthreads/ptthread.c:201
#30 0x00007f8a0b27fea7 in start_thread (arg=<optimized out>) at pthread_create.c:477
#31 0x00007f8a0ae7cdef in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Adding bug 1709617 because a quick local hack shows the sandbox violation triggered by the socket() call issued is indeed not happening anymore if I hardcode a MTU instead of poking for it via socket().

So I have this patch that removes many of the code, except the SOCK_DGRAM instance because as mentionned in the stack it is breaking on webrtc tests (at least):

• try with SOCK_DGRAM removed: https://treeherder.mozilla.org/jobs?repo=try&revision=34b2a5802bee5528d9c935849f137a0aa92b1837&selectedTaskRun=DTEaV1IeRuyJxF0CvcHNVQ.2
• try with SOCk_DGRAM kept: https://treeherder.mozilla.org/jobs?repo=try&revision=870343e1792f4b6914acee9abdc83b428503d353

As per discussing on matrix #media, I filed blocking bug 1709617 because it looks like we should not even have a socket() call made at all. Locally, removing (hack) the MTU query via socket() the failing webrtc test is passing.

(In reply to Alexandre LISSY :gerard-majax from comment #4)

So I have this patch that removes many of the code, except the SOCK_DGRAM instance because as mentionned in the stack it is breaking on webrtc tests (at least):

• try with SOCK_DGRAM removed: https://treeherder.mozilla.org/jobs?repo=try&revision=34b2a5802bee5528d9c935849f137a0aa92b1837&selectedTaskRun=DTEaV1IeRuyJxF0CvcHNVQ.2
• try with SOCk_DGRAM kept: https://treeherder.mozilla.org/jobs?repo=try&revision=870343e1792f4b6914acee9abdc83b428503d353

As per discussing on matrix #media, I filed blocking bug 1709617 because it looks like we should not even have a socket() call made at all. Locally, removing (hack) the MTU query via socket() the failing webrtc test is passing.

And since bug 1709617 has been updated, I've pushed a new try with its fix as well as removal of SOCK_DGRAM from the sandbox: https://treeherder.mozilla.org/jobs?repo=try&revision=570cc23ecb13791d34ebf40d85c2e9899f274c0d

Sorry Jed, the question I was asking is:

• do we want to proceed with the present bug without removing SOCK_DGRAM for now, or
• do we want to wait for bug 1709617 to land and fold the SOCK_DGRAM removal in the present bug?

I don't think we can do this as long as the media.cubeb.sandbox pref exists — if it's set to false then we'll still try to use audio libraries directly. We don't test that case, so it's possible we've already accidentally broken it somehow, but if we're going to deliberately break it then:

1. We'll need sign-off from a suitable media peer
2. I'd prefer that the pref itself be ifdef'ed so that it no longer exists on Linux
3. I think we could also remove the ALSA parts of the sandbox rules (and other uses of the pref, like this)

Also, I can confirm that content processes shouldn't be trying to determine MTU themselves: if permitted by the distro, they'll be in isolated network namespaces with no access to the real network interfaces.

(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #7)

I don't think we can do this as long as the media.cubeb.sandbox pref exists — if it's set to false then we'll still try to use audio libraries directly. We don't test that case, so it's possible we've already accidentally broken it somehow, but if we're going to deliberately break it then:

1. We'll need sign-off from a suitable media peer
2. I'd prefer that the pref itself be ifdef'ed so that it no longer exists on Linux
3. I think we could also remove the ALSA parts of the sandbox rules (and other uses of the pref, like this)

Also, I can confirm that content processes shouldn't be trying to determine MTU themselves: if permitted by the distro, they'll be in isolated network namespaces with no access to the real network interfaces.

Thanks, I'll check with media peer on the pref and ifdef'ing it. I'll also look into ALSA removal as well.

So, after discussing a bit with :kinetik, the pref is still useful for debugging purpose and it might be better we change a bit the strategy by only opening pulseaudio holes when the pref is set to false. I think it is still a win in the default case.

We should be safe to rip all ALSA parts so.

So, I've implemented the alternative suggested:

• keep the code but only use it when media.cubeb.sandbox=false
• remove MOZ_ALSA bits

I am unsure though if we can get rid of things like https://searchfox.org/mozilla-central/rev/e3c34d34cc7dce56df377f05183ad379ac1de123/security/sandbox/common/SandboxSettings.cpp#33-34

Removing the handling of media.cubeb.sandbox depends on bug 1362220 to be complete ; it is my understanding that currently the remaining parts are macOS and Android support.

The following patch is waiting for review from an inactive reviewer:

:gerard-majax, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Sorry, there was a problem with the detection of inactive users.