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

Issue 765302 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

Do not cache device ID salts

Project Member Reported by guidou@chromium.org, Sep 14 2017

Issue description

MediaStreamDispatcherHost, MediaDevicesDispatcherHost and AudioOutputAuthorizationHandler use a salt from the browser context to hash device IDs.

However, this salt can be reset by users by clearing cookies, so new device IDs must be hashed with the new salt.
Caching them results in the reuse of old device IDs, which is a spec violation.

Even worse, since MSDH is per process and MDDH is per frame, their caches have different lifetimes and may return different device IDs when cookies are cleared, which leads to getUserMedia failures.
 

Comment 1 by guidou@chromium.org, Sep 15 2017

Cc: guidou@chromium.org
 Issue 764276  has been merged into this issue.

Comment 2 by guidou@chromium.org, Sep 15 2017

Cc: -guidou@chromium.org
Components: Blink>GetUserMedia
Owner: guidou@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by guidou@chromium.org, Sep 15 2017

Labels: -Pri-1 Pri-2

Comment 4 by guidou@chromium.org, Sep 15 2017

Labels: M-63

Comment 5 by guidou@chromium.org, Sep 15 2017

Description: Show this description
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 16 2017

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

commit 3298e19f82e0297653b4eb3a52e67ee87bacf371
Author: Guido Urdaneta <guidou@chromium.org>
Date: Mon Oct 16 11:08:08 2017

Do not cache device ID salt in MediaDevicesDispatcherHost.

This is the first in a series of CLs to remove caching of the device ID
salt in the following browser objects:
-MediaDevicesDispatcherHost
-MediaStreamDispatcherHost
-AudioRendererHost

The salt can be reset by the user at any time by clearing browsing data.
Caching it leads to using a stale salt, which results in noncompliance
with the spec, which requires user-visible device IDs to be reset when
cookies are cleared.
Furthermore, the objects mentioned above have different lifetimes, which
leads to inconsistencies if a user clears browsing data.

Bug:  765302 ,  765235 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: If6123b7991084ad92809b20faa5f672fcb3061f2
Reviewed-on: https://chromium-review.googlesource.com/708737
Reviewed-by: Henrik Boström <hbos@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509016}
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/content/browser/media/media_devices_util.cc
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/content/browser/media/media_devices_util.h
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/content/browser/renderer_host/media/media_devices_dispatcher_host.cc
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/content/browser/renderer_host/media/media_devices_dispatcher_host.h
[modify] https://crrev.com/3298e19f82e0297653b4eb3a52e67ee87bacf371/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc

Comment 7 by guidou@chromium.org, Oct 16 2017

Labels: -M-63 M-64
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 16 2017

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

commit d71bcc4f0e72abc9c1068498c964b87ef76ef6f7
Author: Guido Urdaneta <guidou@chromium.org>
Date: Mon Oct 16 14:16:13 2017

Remove Media Device Salt cache from MediaStreamDispatcherHost

This is part of a series to remove the caching of the Media Device salt
from various browser objects.

This CL replaces the salt cache with use of the
GetMediaDeviceSaltAndOrigin() utility function.

This CL also removes passing the security origin from the renderer
process since it is now obtained with the same utility function.

Bug:  765302 
Change-Id: Iaccffd7717690563f6bd5a90494490a7d26b628e
Reviewed-on: https://chromium-review.googlesource.com/709514
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509034}
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/browser/renderer_host/media/media_stream_dispatcher_host.h
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/common/media/media_stream.mojom
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/media_stream_dispatcher.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/media_stream_dispatcher.h
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/media_stream_dispatcher_unittest.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/mock_media_stream_dispatcher.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/mock_media_stream_dispatcher.h
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/mock_mojo_media_stream_dispatcher_host.h
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/media/user_media_processor.cc
[modify] https://crrev.com/d71bcc4f0e72abc9c1068498c964b87ef76ef6f7/content/renderer/pepper/pepper_media_device_manager.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 16 2017

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

commit 731b8f05d7f82aa403cd821ed8e224e12bba3977
Author: Guido Urdaneta <guidou@chromium.org>
Date: Mon Oct 16 15:46:42 2017

Remove caching of salt in AudioOutputAuthorizationHandler

The user can reset the salt at any time by clearing browsing data.
Keeping the salt cached can lead to failure to authorize valid device
IDs if the user clears browsing data.

This is the last CL in the series.

Bug:  765302 
Change-Id: I6460fcb5aba49e0a1cc9ee221256fd56ce898b1b
Reviewed-on: https://chromium-review.googlesource.com/718740
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509059}
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/audio_output_authorization_handler.h
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/audio_renderer_host.h
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/renderer_audio_output_stream_factory_context_impl.cc
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/media/renderer_audio_output_stream_factory_context_impl.h
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/731b8f05d7f82aa403cd821ed8e224e12bba3977/content/test/renderer_audio_output_stream_factory_context_impl_unittest.cc

Status: Fixed (was: Assigned)
Cc: guidou@chromium.org olka@chromium.org
 Issue 788528  has been merged into this issue.

Sign in to add a comment