New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 649800 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Replace cc::Surfaceid::nonce_ with base::UnguessableToken

Project Member Reported by staraz@chromium.org, Sep 23 2016

Issue description

cc::SurfaceId should use base::UnguessableToken for nonce_ instead of an uint64_t for better security


 

Comment 1 by staraz@chromium.org, Sep 23 2016

Cc: fsam...@chromium.org
Cc: sadrul@chromium.org vollick@chromium.org enne@chromium.org danakj@chromium.org mfomitchev@chromium.org varkha@chromium.org piman@chromium.org
Cc: tsepez@chromium.org
and to unify usage of an unguessable token.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 10 2016

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

commit 6b01e58ff708c17f74b560fe1b57f7733b78da53
Author: staraz <staraz@chromium.org>
Date: Thu Nov 10 18:19:11 2016

Replaced cc::SurfaceId::nonce_ with base::UnguessableToken

base::SurfaceId::nonce_ is an uint64 generated at random to make the entire
surface id unguessable.
base::UnguessableToken is a wrapper for 2 uint64 (high_ and low_).
high_ and low_ are generated at random using base::UnguessableToken::Create().
base::UnguessableToken achieves the same goal as the original
cc::SurfaceId::nonce_ with better security and uses more common code.

BUG= 649800 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2379653006
Cr-Commit-Position: refs/heads/master@{#431294}

[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/base/unguessable_token.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/ipc/DEPS
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/ipc/cc_param_traits_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/ipc/local_frame_id.mojom
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/ipc/local_frame_id_struct_traits.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/quads/draw_quad_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/display_scheduler_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/local_frame_id.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_aggregator_perftest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_aggregator_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_hittest_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_id.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_id_allocator.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_manager_ref_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/cc/surfaces/surface_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/modules/canvas/HTMLCanvasElementModule.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeTest.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h
[modify] https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53/ui/compositor/layer_unittest.cc

Comment 6 by staraz@chromium.org, Nov 10 2016

Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 10 2016

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

commit d75809ef13714bcc4e960779d2178d9f99783147
Author: dpranke <dpranke@chromium.org>
Date: Thu Nov 10 20:04:44 2016

Revert of Replaced cc::SurfaceId::nonce_ with base::UnguessableToken (patchset #25 id:480001 of https://codereview.chromium.org/2379653006/ )

Reason for revert:
Looks like this added a static initializer to surface_manager.cc via the change to unguessable_token

https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/28400/steps/sizes/logs/stdio

Original issue's description:
> Replaced cc::SurfaceId::nonce_ with base::UnguessableToken
>
> base::SurfaceId::nonce_ is an uint64 generated at random to make the entire
> surface id unguessable.
> base::UnguessableToken is a wrapper for 2 uint64 (high_ and low_).
> high_ and low_ are generated at random using base::UnguessableToken::Create().
> base::UnguessableToken achieves the same goal as the original
> cc::SurfaceId::nonce_ with better security and uses more common code.
>
> BUG= 649800 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
>
> Committed: https://crrev.com/6b01e58ff708c17f74b560fe1b57f7733b78da53
> Cr-Commit-Position: refs/heads/master@{#431294}

TBR=danakj@chromium.org,tsepez@chromium.org,fsamuel@chromium.org,jbroman@chromium.org,haraken@chromium.org,vmpstr@chromium.org,sky@chromium.org,dcheng@chromium.org,staraz@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 649800 

Review-Url: https://codereview.chromium.org/2494833002
Cr-Commit-Position: refs/heads/master@{#431330}

[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/base/unguessable_token.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/ipc/DEPS
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/ipc/cc_param_traits.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/ipc/cc_param_traits_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/ipc/local_frame_id.mojom
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/ipc/local_frame_id_struct_traits.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/quads/draw_quad_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/display_scheduler_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/local_frame_id.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_aggregator_perftest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_aggregator_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_hittest_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_id.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_id_allocator.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_manager_ref_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/cc/surfaces/surface_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/modules/canvas/HTMLCanvasElementModule.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridgeTest.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h
[modify] https://crrev.com/d75809ef13714bcc4e960779d2178d9f99783147/ui/compositor/layer_unittest.cc

Sign in to add a comment