1744203 - FAIL Test connect_abstract_permit was permitted (security/sandbox/test/browser_sandbox_test.js)

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[:gerard-majax]

STR:

1. mach test --headless security/sandbox/test/browser_sandbox_test.js

Expected:
Test passes

Actual:

Unexpected Results
------------------
security/sandbox/test/browser_sandbox_test.js
  FAIL Test connect_abstract_permit was permitted.  | Succeeded -

Running with xvfb-run mach test security/sandbox/test/browser_sandbox_test.js the test is properly passing

Comment 1

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

I have patches that should fix this. I can't reproduce it with just ./mach test --headless, but setting the security.sandbox.content.headless pref breaks the test, because the broker rejects the call (EACCES) instead of permitting it and having the connection fail (ECONNREFUSED). The error message is incorrect in that case (was permitted and Succeeded instead of the actual error), and I've also fixed that.

Comment 2

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

Attachment: Bug 1744203 - Fix SandboxTesting when test cases expect a specific error.

The original SandboxTesting protocol assumed tests would just care about
whether operations succeeded or failed, but now we have tests that check
for specific error codes. Currently that doesn't work well: getting an
error with the wrong error code is misreported as the syscall succeeding.

This patch changes the protocol to simply indicate whether the test
passed and give an unstructured message about what happened, and fixes
the SandboxTestingChild::*Test methods to include the relevant
information in the message.

Comment 3

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

Attachment: Bug 1744203 - Fix the `connect_abstract_permit` test to handle X11 connections not being allowed.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/453b154f2cec
Fix SandboxTesting when test cases expect a specific error. r=gerard-majax
https://hg.mozilla.org/integration/autoland/rev/62c677d99313
Fix the `connect_abstract_permit` test to handle X11 connections not being allowed. r=gerard-majax

Comment 5

[Marian-Vasile Laza]

Backed out 2 changesets (Bug 1744203) for causing bc test failures.
Backout link
Push with failures
Failure Log

Comment 6

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

I don't understand this. I didn't change the .ini file, only some implementation details of the test.

But, I notice that the failed jobs are all for ASan, which doesn't use sandboxing; it shouldn't even have been trying to touch security/sandbox on that build type. Maybe there's some heuristic that's trying to run tests only for changed files, and it's being too clever for its own good….

Comment 7

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

The CI failure is bug 1667271, and the best workaround seems to be to re-land this (and warn the sheriffs, now that I know this is problem). But I'm going to wait until after the winter holidays to try that.

Comment 8

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

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jld, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d5a90dc653fb
Fix SandboxTesting when test cases expect a specific error. r=gerard-majax
https://hg.mozilla.org/integration/autoland/rev/47830047263a
Fix the `connect_abstract_permit` test to handle X11 connections not being allowed. r=gerard-majax

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/d5a90dc653fb
https://hg.mozilla.org/mozilla-central/rev/47830047263a