1823458 - bug 1809518 triggers openbsd sandboxing

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Landry Breuil (:gaston)]

since 112 betas, utility process is crashing at startup with the following trace:

#0  sysctl () at /tmp/-:3
#1  0x5a9b5b7b921d1ad6 in ?? ()
#2  0x00000a94a544d158 in mozilla::TimeStamp::ComputeProcessUptime () at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/mozglue/misc/TimeStamp_posix.cpp:314
#3  0x00000a94a544ccaf in mozilla::TimeStamp::ProcessCreation () at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/mozglue/misc/TimeStamp.cpp:62
#4  0x00000a97614b779c in JS::detail::InitWithFailureDiagnostic (isDebugBuild=false) at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/js/src/vm/Initialization.cpp:140
#5  0x00000a9764c8af56 in JS_Init () at /usr/obj/ports/firefox-112.0beta2/build-amd64/dist/include/js/Initialization.h:69
#6  mozilla::ipc::UtilityProcessChild::Init (this=0xa9777816400, aEndpoint=..., aParentBuildID=..., aSandboxingKind=0) at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/ipc/glue/UtilityProcessChild.cpp:117
#7  mozilla::ipc::UtilityProcessImpl::Init (this=<optimized out>, aArgc=<optimized out>, aArgv=<optimized out>) at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/ipc/glue/UtilityProcessImpl.cpp:81
#8  0x00000a97617c98f1 in XRE_InitChildProcess (aArgc=12, aArgv=0x77e067cc2978, aChildData=<optimized out>) at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/toolkit/xre/nsEmbedFunctions.cpp:639
#9  0x00000a94a5449311 in content_process_main (bootstrap=0xa977179aac0, argc=14, argv=0x77e067cc2978) at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/browser/app/../../ipc/contentproc/plugin-container.cpp:57
#10 main (argc=<optimized out>, argv=0x77e067cc2978, envp=<optimized out>) at /usr/obj/ports/firefox-112.0beta2/firefox-112.0/browser/app/nsBrowserApp.cpp:353

due to tight sandboxing via pledge(), the utility process isnt allowed to call sysctl with KERN_PROC_PID argument in https://searchfox.org/mozilla-central/source/mozglue/misc/TimeStamp_posix.cpp#292 - the traceback leads to bug #1809518 which added js initialization to the utility process in 112.

an option to work around that would be to add ps to the pledge classes allowed for the utility process, but a much cleaner option would be to move the initialization of the utility process sandboxing (currently here: https://searchfox.org/mozilla-central/source/ipc/glue/UtilityProcessImpl.cpp#66) to after the JS_Init call that triggers the sysctl (eg in https://searchfox.org/mozilla-central/source/ipc/glue/UtilityProcessChild.cpp#121)

i'm currently building 112.0b4 with a patch that does this and will test runtime tomorrow and report back here.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1809518

:sefeng, since you are the author of the regressor, bug 1809518, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 2

[Sean Feng [:sefeng]]

Thanks for handling this gaston, let me know if you need anything from me.

Comment 3

[:gerard-majax]

is the suggestion reasonnable? or should ps pledge just be allowed?

Comment 4

[Landry Breuil (:gaston)]

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

is the suggestion reasonnable? or should ps pledge just be allowed?

i already know that'll get strongly frowned upon :)

Comment 5

[Landry Breuil (:gaston)]

112.0b4 runs fine with the below patch, without needing pledge config modifications:

Index: ipc/glue/UtilityProcessChild.cpp
--- ipc/glue/UtilityProcessChild.cpp.orig
+++ ipc/glue/UtilityProcessChild.cpp
@@ -20,6 +20,10 @@
 #  include "mozilla/Sandbox.h"
 #endif
 
+#if defined(XP_OPENBSD) && defined(MOZ_SANDBOX)
+#  include "mozilla/SandboxSettings.h"
+#endif
+
 #if defined(MOZ_SANDBOX) && defined(MOZ_DEBUG) && defined(ENABLE_TESTS)
 #  include "mozilla/SandboxTestingChild.h"
 #endif
@@ -118,6 +122,10 @@ bool UtilityProcessChild::Init(mozilla::ipc::UntypedEn
       return false;
     }
   }
+#if defined(__OpenBSD__) && defined(MOZ_SANDBOX)
+  StartOpenBSDSandbox(GeckoProcessType_Utility,
+                      mSandbox);
+#endif
 
   profiler_set_process_name(nsCString("Utility Process"));
 
Index: ipc/glue/UtilityProcessImpl.cpp
--- ipc/glue/UtilityProcessImpl.cpp.orig
+++ ipc/glue/UtilityProcessImpl.cpp
@@ -14,10 +14,6 @@
 #  include "WMFDecoderModule.h"
 #endif
 
-#if defined(XP_OPENBSD) && defined(MOZ_SANDBOX)
-#  include "mozilla/SandboxSettings.h"
-#endif
-
 namespace mozilla::ipc {
 
 UtilityProcessImpl::~UtilityProcessImpl() = default;
@@ -63,9 +59,6 @@ bool UtilityProcessImpl::Init(int aArgc, char* aArgv[]
 
   // Go for it
   mozilla::SandboxTarget::Instance()->StartSandbox();
-#elif defined(__OpenBSD__) && defined(MOZ_SANDBOX)
-  StartOpenBSDSandbox(GeckoProcessType_Utility,
-                      (SandboxingKind)*sandboxingKind);
 #endif
 
   Maybe<const char*> parentBuildID =

if deemed reasonable, i can send that to phabricator for proper review.

Comment 6

[Landry Breuil (:gaston)]

Attachment: Bug 1823458 - Call StartOpenBSDSandbox after JS_Init in generic utility process r=gerard-majax,sefeng

otherwise sandboxing is triggered by sysctl(KERN_PROC_PID) which
isnt permitted by the current sandboxing policy.

Comment 7

[:gerard-majax]

According to https://searchfox.org/mozilla-central/rev/f078cd02746b29652c134b144f0629d47e378166/mozglue/misc/TimeStamp_posix.cpp#286-331 this is code really specific to *BSD, so I dont think we really have a big issue here.

Comment 8

[Landry Breuil (:gaston)]

Fwiw i've tested the latest iteration of https://phabricator.services.mozilla.com/D173152 adressing review comments, and at runtime utility process doesnt crash. If new utility process type appear, i'll probably have to revisit it anway, but the patch is ready to land.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/204ed189899f
Call StartOpenBSDSandbox after JS_Init in generic utility process r=gerard-majax,sefeng

Comment 10

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/204ed189899f

Comment 11

[Landry Breuil (:gaston)]

Comment on attachment 9324194 [details]
Bug 1823458 - Call StartOpenBSDSandbox after JS_Init in generic utility process r=gerard-majax,sefeng

Beta/Release Uplift Approval Request

• User impact if declined: runtime crashes of utility process on OpenBSD
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Tier3
• String changes made/needed:
• Is Android affected?: No

Comment 12

[Dianna Smith [:diannaS]]

Comment on attachment 9324194 [details]
Bug 1823458 - Call StartOpenBSDSandbox after JS_Init in generic utility process r=gerard-majax,sefeng

Approved for 112.0b7

Comment 13

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/efc03a0d2b43