1784521 - Replace the use of PR_CreateProcess in the Samba NTLM auth code

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

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

See bug 1658072 and bug 227246; PR_CreateProcess doesn't cooperate with other code that handles child processes, and nsAuthSambaNTLM.cpp is one of the few remaining users (and in particular this case is causing an assertion failure with the patch for bug 1658072).

It looks like all the code is using is redirecting stdin/stdout to pipes, and it's not used on Windows so it should be safe to just use Unix-specific APIs; this can all be done with IPC (LaunchApp and ProcessWatcher).

Comment 1

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

It looks like I called this out 5 years ago in bug 1406971 and then forgot about it.

Comment 2

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

Martin, do you have any idea of whether anyone might still be using this NTLM auth backend with Firefox? I noticed RHBZ 1173156 (in which it apparently had at least one user, 9 years ago), and it occurs to me that if anyone is still using it then they're likely to be on RHEL.

Specifically, I'd like to either remove this code entirely if it's not being used, or else replace the use of PR_CreateProcess in which case it would help to know how to test any changes to it.

Mozilla has telemetry which covers this, and the last time we checked it there were no users of this code, but it's opt-in so we don't have data from most release/ESR users.

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Hello, AFAIK ntlm_auth is still shipped and used.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1784521 - Stop using PR_CreateProcess in nsAuthSambaNTLM, r=mccr8!

The NSPR process launching backend appears to attach its own SIGCHLD
handler, which can cause problems with our own SIGCHLD handling under
the hood.

This patch changes the behaviour to instead use base::LaunchProcess to
start the child process for nsAuthSambaNTLM, which is more aligned with
other places we launch code in Gecko.

This code is posix-specific, so we don't need to handle Windows or macOS
process launching.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6b7e5789f25
Stop using PR_CreateProcess in nsAuthSambaNTLM, r=mccr8

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/f6b7e5789f25