Update write checks for external extension file on mac.

Update write checks for external extension file on mac.


BUG=100565
TEST=VerifyPathControlledByUserTest.*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105923

[Mark Mentovai, 2011-10-17 18:53:57]

http://codereview.chromium.org/8318011/diff/2001/base/file_util_posix.cc
File base/file_util_posix.cc (right):

http://codereview.chromium.org/8318011/diff/2001/base/file_util_posix.cc#newc...
base/file_util_posix.cc:1037: const char* kAdminGroupNames[] = {
In order for this to be properly treated as constant data, you need one more
const, otherwise the compiler may assume that someone may change the char*
pointers in the array. You need this one to be

  const char* const kAdminGroupNames[] = {

http://codereview.chromium.org/8318011/diff/2001/base/file_util_posix.cc#newc...
base/file_util_posix.cc:1047: struct group *group_record =
getgrnam(kAdminGroupNames[i]);
This does a name lookup which may block. On which thread does this happen?

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
File chrome/common/extensions/docs/external_extensions.html (right):

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
chrome/common/extensions/docs/external_extensions.html:448: <p
class="note"><b>Note:</b> The external extensions file is read only if every
directory in the path is owned by the user <code>root</code>, has the group
<code>admin</code> or <code>wheel</code>, and is not world readable.  The path
must also be free of symbolic links.  These restrictions prevent an unprivileged
user from causing extensions to be installed for all users.  See <a
href="#troubleshooting">troubleshooting</a> for details.</p>
“not word readable” is wrong.

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
File chrome/common/extensions/docs/static/external_extensions.html (right):

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
chrome/common/extensions/docs/static/external_extensions.html:106: <p
class="note"><b>Note:</b> The external extensions file is read only if every
directory in the path is owned by the user <code>root</code>, has the group
<code>admin</code> or <code>wheel</code>, and is not world readable.  The path
must also be free of symbolic links.  These restrictions prevent an unprivileged
user from causing extensions to be installed for all users.  See <a
href="#troubleshooting">troubleshooting</a> for details.</p>
As before.

[Sam Kerner (Chrome), 2011-10-17 19:35:16]

http://codereview.chromium.org/8318011/diff/2001/base/file_util_posix.cc
File base/file_util_posix.cc (right):

http://codereview.chromium.org/8318011/diff/2001/base/file_util_posix.cc#newc...
base/file_util_posix.cc:1037: const char* kAdminGroupNames[] = {
On 2011/10/17 18:53:57, Mark Mentovai wrote:
> In order for this to be properly treated as constant data, you need one more
> const, otherwise the compiler may assume that someone may change the char*
> pointers in the array. You need this one to be
> 
>   const char* const kAdminGroupNames[] = {

Done.

http://codereview.chromium.org/8318011/diff/2001/base/file_util_posix.cc#newc...
base/file_util_posix.cc:1047: struct group *group_record =
getgrnam(kAdminGroupNames[i]);
On 2011/10/17 18:53:57, Mark Mentovai wrote:
> This does a name lookup which may block. On which thread does this happen?

This function runs on the file thread.  To be sure no one else uses it on
another thread, I call base::ThreadRestrictions::AssertIOAllowed(); just before
the loop.

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
File chrome/common/extensions/docs/external_extensions.html (right):

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
chrome/common/extensions/docs/external_extensions.html:448: <p
class="note"><b>Note:</b> The external extensions file is read only if every
directory in the path is owned by the user <code>root</code>, has the group
<code>admin</code> or <code>wheel</code>, and is not world readable.  The path
must also be free of symbolic links.  These restrictions prevent an unprivileged
user from causing extensions to be installed for all users.  See <a
href="#troubleshooting">troubleshooting</a> for details.</p>
On 2011/10/17 18:53:57, Mark Mentovai wrote:
> “not word readable” is wrong.

Done.

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
File chrome/common/extensions/docs/static/external_extensions.html (right):

http://codereview.chromium.org/8318011/diff/2001/chrome/common/extensions/doc...
chrome/common/extensions/docs/static/external_extensions.html:106: <p
class="note"><b>Note:</b> The external extensions file is read only if every
directory in the path is owned by the user <code>root</code>, has the group
<code>admin</code> or <code>wheel</code>, and is not world readable.  The path
must also be free of symbolic links.  These restrictions prevent an unprivileged
user from causing extensions to be installed for all users.  See <a
href="#troubleshooting">troubleshooting</a> for details.</p>
On 2011/10/17 18:53:57, Mark Mentovai wrote:
> As before.

Done.

[Mark Mentovai, 2011-10-17 19:38:54]

Great, LGTM.

[commit-bot: I haz the power, 2011-10-17 19:58:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skerner@chromium.org/8318011/7001

[commit-bot: I haz the power, 2011-10-17 21:30:30]

Change committed as 105923