New issue
Advanced search Search tips

Issue 754280 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Time zone specifications have moved in 10.13, need to update sandbox profile?

Project Member Reported by mark@chromium.org, Aug 10 2017

Issue description

Split from  bug 754053  comment 6:

content/renderer/renderer.sb has sandbox holes to allow renderers to read the time zone specification.

https://chromium.googlesource.com/chromium/src/+/a2e4f7f5c265cc867f21564d027afaf6a31550a9/content/renderer/renderer.sb#34

34  ; https://crbug.com/288697
35  (allow file-read*
36    (path "/private/etc/localtime")
37    (subpath "/usr/share/zoneinfo"))
38
39  (allow file-read-metadata (path "/private/etc"))

This works for the time zone specification structure on 10.12 and earlier:

% ls -ld /etc
lrwxr-xr-x@ 1 root  wheel  11 Sep 20  2016 /etc -> private/etc
% ls -l /etc/localtime
lrwxr-xr-x  1 root  wheel  36 Sep 20  2016 /etc/localtime -> /usr/share/zoneinfo/America/New_York
% ls -ld /usr/share/zoneinfo
drwxr-xr-x  67 root  wheel  2278 Jul 19 15:50 /usr/share/zoneinfo
% ls -l /usr/share/zoneinfo/America/New_York
-rw-r--r--  3 root  wheel  1267 Aug 29  2016 /usr/share/zoneinfo/America/New_York
% sw_vers
ProductName:	Mac OS X
ProductVersion:	10.12.6
BuildVersion:	16G29

The structure has changed in 10.13:

$ ls -ld /etc
lrwxr-xr-x@ 1 root  wheel  11 Aug  7 18:43 /etc -> private/etc
$ ls -l /etc/localtime
lrwxr-xr-x  1 root  wheel  29 Aug  7 18:55 /etc/localtime -> /var/db/timezone/zoneinfo/UTC
$ ls -ld /var
lrwxr-xr-x@ 1 root  wheel  11 Aug  7 18:43 /var -> private/var
$ ls -ld /var/db/timezone/zoneinfo
lrwxr-xr-x  1 root  wheel  27 Aug  7 18:43 /var/db/timezone/zoneinfo -> /usr/share/zoneinfo.default
$ ls -l /var/db/timezone/zoneinfo/UTC
-rw-r--r--  6 root  wheel  118 Aug  2 01:13 /var/db/timezone/zoneinfo/UTC
$ ls -ld /usr/share/zoneinfo
lrwxr-xr-x  1 root  wheel  25 Aug  7 18:43 /usr/share/zoneinfo -> /var/db/timezone/zoneinfo
$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.13
BuildVersion:	17A330h

such that we probably now need to broaden our sandbox holes to allow this to work properly.

Do we still do OS version-specific sandbox rules?
 

Comment 1 by rsesek@chromium.org, Aug 10 2017

> Do we still do OS version-specific sandbox rules?

Yes, we do. Greg already fixed this in the v2 sandbox:

https://cs.chromium.org/chromium/src/content/renderer/renderer_v2.sb?q=renderer_v2.sb&sq=package:chromium&dr&l=91
Yes I should have realized that this needed to be fixed in the current profiles as well, so thank you for this report. I will take care of this. Note that I don't understand why the symlink goes from /etc/localtime to /var/db/timezone to /usr/share/zoneinfo but, oh well.
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5045f3a803a8375502ab153ce61bc6944c0ee28

commit c5045f3a803a8375502ab153ce61bc6944c0ee28
Author: Greg Kerr <kerrnel@chromium.org>
Date: Fri Aug 11 16:38:41 2017

Update sandbox rules for timezone path on macOS 10.13.

macOS 10.13 changes the location of the timezone paths, so this updates
the profiles to support the path for the older and new OS versions.

Bug:  754280 
Change-Id: Icc0b837f500bda041f19cdf39405b9d9b9cf9311
Reviewed-on: https://chromium-review.googlesource.com/611409
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Greg Kerr <kerrnel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493772}
[modify] https://crrev.com/c5045f3a803a8375502ab153ce61bc6944c0ee28/content/common/common.sb
[modify] https://crrev.com/c5045f3a803a8375502ab153ce61bc6944c0ee28/content/common/sandbox_mac.h
[modify] https://crrev.com/c5045f3a803a8375502ab153ce61bc6944c0ee28/content/common/sandbox_mac.mm
[modify] https://crrev.com/c5045f3a803a8375502ab153ce61bc6944c0ee28/content/renderer/renderer.sb

Status: Fixed (was: Started)
Cc: dalecur...@chromium.org
Is this safe to merge to M62? We need it for  issue 767037 
Labels: Merge-Request-62
It's a trivial change, merge requested.
Project Member

Comment 8 by sheriffbot@chromium.org, Sep 27 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Approving merge to M62. Branch:3202
Labels: -Merge-Review-62 Merge-Approved-62
Labels: -Merge-Approved-62 Merge-Rejected-62
Hah, this is actually already in M62. I should have checked more closely!
Heh I didn't even think to check other I've been so busy, I guess I'd have noticed when I did the merge. Thanks!

Sign in to add a comment