Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2777233003:
Remove SecurityState::RevokeScheme() (Closed)
DescriptionRemove SecurityState::RevokeScheme()
This functionality is never used. As part of network service
refactoring, I'm trying to make things as immutable-as-possible, so I'd
like to remove this possibility.
BUG=624183
Review-Url: https://codereview.chromium.org/2777233003
Cr-Commit-Position: refs/heads/master@{#460174}
Committed: https://chromium.googlesource.com/chromium/src/+/7620a85012beed1c0462cc2fcb682d9d00516d02
Patch Set 1 #Patch Set 2 : . #Messages
Total messages: 23 (15 generated)
The CQ bit was checked by scottmg@chromium.org to run a CQ dry run
scottmg@chromium.org changed reviewers: + tsepez@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by scottmg@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Remove SecurityState::RevokeScheme() This functionality is never used. As part of network service refactoring, I'm trying to make things as immutable-as-possible, so I'd like to remove this possibility. BUG=624183 ========== to ========== Remove SecurityState::RevokeScheme() This functionality is never used. As part of network service refactoring, I'm trying to make things as immutable-as-possible, so I'd like to remove this possibility. BUG=624183 ==========
tsepez@chromium.org changed reviewers: + nasko@chromium.org
On 2017/03/28 00:33:42, scottmg wrote: Change itself looks good, but adding Nasko to see if this is expected given site-isolation changes or if maybe we dropped the ball somewhere.
nasko@chromium.org changed reviewers: + nick@chromium.org
Nick has done most of the work in that area, so I'll defer to him on this one.
lgtm from the "this is unused code" perspective Timing a revocation is hard with the current "write-anywhere, read-anywhere" design of the ChildProcessSecurityPolicy; we also don't consider revoking a privilege to be a useful operation from a security perspective, since a compromised process generally ought to be able to re-gain any revoked privilege by following the same steps it took to obtain the privilege the first time. Our road map does call for implementing revocation eventually, though -- but only to prevent unbounded memory growth once we have per-frame, per-origin privileges. We're still a ways away from that, and its form will be different enough that we don't need to keep this implementation around now.
lgtm
On 2017/03/28 18:12:47, ncarter wrote: > lgtm from the "this is unused code" perspective > > Timing a revocation is hard with the current "write-anywhere, read-anywhere" > design of the ChildProcessSecurityPolicy; we also don't consider revoking a > privilege to be a useful operation from a security perspective, since a > compromised process generally ought to be able to re-gain any revoked privilege > by following the same steps it took to obtain the privilege the first time. Thanks! The "write- & read-anywhere" you mention I'm finding tricky in trying to tease use of ChildProcessSecurityPolicy out of ResourceDispatcherHostImpl. If you're an expert on this area I'd be very happy to have your thoughts on https://docs.google.com/document/d/1KUi4tDIBBzeo2gzt0fv4J0NaRGTb9YoV4YgbRgsOu.... > > Our road map does call for implementing revocation eventually, though -- but > only to prevent unbounded memory growth once we have per-frame, per-origin > privileges. We're still a ways away from that, and its form will be different > enough that we don't need to keep this implementation around now.
The CQ bit was checked by scottmg@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490725803293970,
"parent_rev": "f1041970dd07cd46531600fa5fdfccbe8e6e6dce", "commit_rev":
"7620a85012beed1c0462cc2fcb682d9d00516d02"}
Message was sent while issue was closed.
Description was changed from ========== Remove SecurityState::RevokeScheme() This functionality is never used. As part of network service refactoring, I'm trying to make things as immutable-as-possible, so I'd like to remove this possibility. BUG=624183 ========== to ========== Remove SecurityState::RevokeScheme() This functionality is never used. As part of network service refactoring, I'm trying to make things as immutable-as-possible, so I'd like to remove this possibility. BUG=624183 Review-Url: https://codereview.chromium.org/2777233003 Cr-Commit-Position: refs/heads/master@{#460174} Committed: https://chromium.googlesource.com/chromium/src/+/7620a85012beed1c0462cc2fcb68... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/7620a85012beed1c0462cc2fcb68... |
