ChildProcessSecurityPolicy: Add DeleteFromFileSystem permission.

ChildProcessSecurityPolicy: Add DeleteFromFileSystem permission.

BUG=275980
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230055

[tommycli, 2013-10-21 14:16:09]

Looks like you're trying to separate the WRITE grant from the DELETE grant.
Would we ever want to grant one and not the other?

Could a WRITE not effectively delete by emptying the file of its contents?

https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
File content/browser/child_process_security_policy_impl.cc (right):

https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
content/browser/child_process_security_policy_impl.cc:41:
DELETE_FROM_FILE_PERMISSION      = 1 << 4,
probably just DELETE_FILE right?

https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
content/browser/child_process_security_policy_impl.cc:64: DELETE_FROM_FILE_GRANT
      = DELETE_FROM_FILE_PERMISSION,
here too

https://codereview.chromium.org/31663002/diff/1/content/browser/fileapi/filea...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/31663002/diff/1/content/browser/fileapi/filea...
content/browser/fileapi/fileapi_message_filter.cc:466: if
(!security_policy_->CanWriteFileSystemFile(process_id_, url)) {
Should Truncate be considered a WRITE op or a DELETE op?

[Tom Sepez, 2013-10-21 16:45:24]

Could you point me at the spot(s) in the existing code where we grant write but
not delete?

[tommycli, 2013-10-21 17:11:03]

On 2013/10/21 16:45:24, Tom Sepez wrote:
> Could you point me at the spot(s) in the existing code where we grant write
but
> not delete?

Is this question for me or thestig?

[Tom Sepez, 2013-10-21 17:13:07]

> Is this question for me or thestig?
Primarily for 'stig, but you could answer it if you wanted.  In short, I'd like
to see what's actually driving this.

[Lei Zhang, 2013-10-21 20:57:08]

On 2013/10/21 17:13:07, Tom Sepez wrote:
> > Is this question for me or thestig?
> Primarily for 'stig, but you could answer it if you wanted.  In short, I'd
like
> to see what's actually driving this.

Bug 275980 of course. I have a follow up CL to add media galleries delete
support, where we don't want to allow write. Separated this out to make it
easier to review.

[Tom Sepez, 2013-10-21 21:05:29]

lgtm

[Lei Zhang, 2013-10-21 21:23:08]

+jam for contents/

[Lei Zhang, 2013-10-21 21:35:37]

https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
File content/browser/child_process_security_policy_impl.cc (right):

https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
content/browser/child_process_security_policy_impl.cc:41:
DELETE_FROM_FILE_PERMISSION      = 1 << 4,
On 2013/10/21 14:16:09, tommycli wrote:
> probably just DELETE_FILE right?

Done.

https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
content/browser/child_process_security_policy_impl.cc:64: DELETE_FROM_FILE_GRANT
      = DELETE_FROM_FILE_PERMISSION,
On 2013/10/21 14:16:09, tommycli wrote:
> here too

Done.

https://codereview.chromium.org/31663002/diff/1/content/browser/fileapi/filea...
File content/browser/fileapi/fileapi_message_filter.cc (right):

https://codereview.chromium.org/31663002/diff/1/content/browser/fileapi/filea...
content/browser/fileapi/fileapi_message_filter.cc:466: if
(!security_policy_->CanWriteFileSystemFile(process_id_, url)) {
On 2013/10/21 14:16:09, tommycli wrote:
> Should Truncate be considered a WRITE op or a DELETE op?

It's a write since we are not deleting the file entry.

[tommycli, 2013-10-21 21:35:49]

On 2013/10/21 21:35:37, Lei Zhang wrote:
>
https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
> File content/browser/child_process_security_policy_impl.cc (right):
> 
>
https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
> content/browser/child_process_security_policy_impl.cc:41:
> DELETE_FROM_FILE_PERMISSION      = 1 << 4,
> On 2013/10/21 14:16:09, tommycli wrote:
> > probably just DELETE_FILE right?
> 
> Done.
> 
>
https://codereview.chromium.org/31663002/diff/1/content/browser/child_process...
> content/browser/child_process_security_policy_impl.cc:64:
DELETE_FROM_FILE_GRANT
>       = DELETE_FROM_FILE_PERMISSION,
> On 2013/10/21 14:16:09, tommycli wrote:
> > here too
> 
> Done.
> 
>
https://codereview.chromium.org/31663002/diff/1/content/browser/fileapi/filea...
> File content/browser/fileapi/fileapi_message_filter.cc (right):
> 
>
https://codereview.chromium.org/31663002/diff/1/content/browser/fileapi/filea...
> content/browser/fileapi/fileapi_message_filter.cc:466: if
> (!security_policy_->CanWriteFileSystemFile(process_id_, url)) {
> On 2013/10/21 14:16:09, tommycli wrote:
> > Should Truncate be considered a WRITE op or a DELETE op?
> 
> It's a write since we are not deleting the file entry.

lgtm

[jam, 2013-10-22 00:00:35]

lgtm

[commit-bot: I haz the power, 2013-10-22 00:16:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/31663002/140001

[commit-bot: I haz the power, 2013-10-22 00:36:20]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-10-22 00:50:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/31663002/140001

[commit-bot: I haz the power, 2013-10-22 05:15:38]

Change committed as 230055