Don't allow to use sandbox primitives from anywhere but global scope and templates

RESOLVED FIXED in Firefox 48

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 2 bugs)

unspecified
mozilla48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

3 years ago
No description provided.
Assignee

Updated

3 years ago
Depends on: 1259960
Assignee

Comment 1

3 years ago
We've done this for set_config, set_define, imply_option, and
add_old_configure_assignment. Do it for add_old_configure_arg as well.

Review commit: https://reviewboard.mozilla.org/r/42697/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42697/
Attachment #8735320 - Flags: review?(nalexander)
Attachment #8735321 - Flags: review?(nalexander)
Attachment #8735322 - Flags: review?(nalexander)
Assignee

Comment 3

3 years ago
The initial goal of templates was to provide a way to write shorter
constructs for some generic tasks during configure. But the limitations
of the sandbox and the properties of templates made them used for more
general functions.

Consequently, this led to templates having to be available from
anywhere, which, in turn, led to difficult to introspect constructs.
With bug 1257823, we've made almost everything use set_config and
similar functions from the global scope, but we don't enforce that
those primitives are only used at the global scope.

This change does that: it enforces that primitives are only used at
the global scope. Or in templates.

Now, since templates were used for other purposes than generic uses
of other primitives, we now allow non-template functions to be declared.
Those can be used everywhere, but don't have access to the sandbox
primitives.

Review commit: https://reviewboard.mozilla.org/r/42701/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42701/
Comment on attachment 8735320 [details]
MozReview Request: Bug 1260066 - Move add_old_configure_arg to the global scope. r?nalexander

https://reviewboard.mozilla.org/r/42697/#review39341
Attachment #8735320 - Flags: review?(nalexander) → review+
Comment on attachment 8735321 [details]
MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

https://reviewboard.mozilla.org/r/42699/#review39343

::: moz.configure:159
(Diff revision 1)
> -@depends(target)
> +        )
> -def linux_programs(target):
>      if target.os == 'GNU' and target.kernel == 'Linux':
> -        check_prog('RPMBUILD', ('rpmbuild',), allow_missing=True)
> +        return namespace(RPMBUILD=('rpmbuild',))
> +
> +check_prog('DSYMUTIL', delayed_getattr(extra_programs, 'DSYMUTIL'),

How can this succeed?  Suppose we're not on Darwin, so extra_programs returns a namespace not including 'DSYMUTIL'.  Then the `delayed_getattr` returns `None`, and `check_prog` should fail: https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/checks.configure#58.

What am I missing?
Attachment #8735321 - Flags: review?(nalexander)
Attachment #8735322 - Flags: review?(nalexander) → review+
Comment on attachment 8735322 [details]
MozReview Request: Bug 1260066 - Don't allow to use sandbox primitives from anywhere but global scope and templates. r?nalexander

https://reviewboard.mozilla.org/r/42701/#review39347

This looks okay to me, but help me understand the scope of declared functions.  Suppose `a.configure` defines `f` and `b.configure` includes `a.configure`.  Is `f` available in `b.configure`?  I assume yes (there's an `exec_file` in there)?
Assignee

Comment 7

3 years ago
(In reply to Nick Alexander :nalexander from comment #5)
> How can this succeed?  Suppose we're not on Darwin, so extra_programs
> returns a namespace not including 'DSYMUTIL'.  Then the `delayed_getattr`
> returns `None`, and `check_prog` should fail:
> https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/checks.
> configure#58.
> 
> What am I missing?

bug 1259960
Assignee

Comment 8

3 years ago
(In reply to Nick Alexander :nalexander from comment #6)
> Comment on attachment 8735322 [details]
> MozReview Request: Bug 1260066 - Don't allow to use sandbox primitives from
> anywhere but global scope and templates. r?nalexander
> 
> https://reviewboard.mozilla.org/r/42701/#review39347
> 
> This looks okay to me, but help me understand the scope of declared
> functions.  Suppose `a.configure` defines `f` and `b.configure` includes
> `a.configure`.  Is `f` available in `b.configure`?  I assume yes (there's an
> `exec_file` in there)?

Same as templates and depends, yes.
Assignee

Comment 9

3 years ago
Comment on attachment 8735321 [details]
MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

See comment 7
Attachment #8735321 - Flags: review?(nalexander)
Comment on attachment 8735321 [details]
MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

https://reviewboard.mozilla.org/r/42699/#review39389
Attachment #8735321 - Flags: review?(nalexander) → review+

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.