From b36aaa819382f870c2369102e4c55b9f456e090f Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Wed, 29 Sep 2021 11:37:21 -0400 Subject: [PATCH] fix: persist permission granted to serial ports --- .../browser/api/electron_api_web_contents.cc | 48 +++++++++++++ shell/browser/api/electron_api_web_contents.h | 24 +++++++ shell/browser/electron_permission_manager.cc | 67 +++++++++++++++++++ shell/browser/electron_permission_manager.h | 11 +++ .../serial/electron_serial_delegate.cc | 3 +- .../browser/serial/serial_chooser_context.cc | 62 ++++++++++------- shell/browser/serial/serial_chooser_context.h | 33 ++++++--- .../serial/serial_chooser_controller.cc | 10 +-- .../serial/serial_chooser_controller.h | 6 +- .../browser/web_contents_permission_helper.cc | 40 +++++++++++ .../browser/web_contents_permission_helper.h | 18 +++++ 11 files changed, 279 insertions(+), 43 deletions(-) diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 551b25837c810..53f4e147e8b23 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -911,6 +911,12 @@ void WebContents::InitWithWebContents(content::WebContents* web_contents, } WebContents::~WebContents() { + // clear out objects that have been granted permissions so that when + // WebContents::RenderFrameDeleted is called as a result of WebContents + // destruction it doesn't try to clear out a granted_devices_ + // on a destructed object. + granted_devices_.clear(); + MarkDestroyed(); // The destroy() is called. if (inspectable_web_contents_) { @@ -1415,6 +1421,12 @@ void WebContents::RenderFrameDeleted( // - Cross-origin navigation creates a new RFH in a separate process which // is swapped by content::RenderFrameHostManager. // + + // clear out objects that have been granted permissions + if (!granted_devices_.empty()) { + granted_devices_.erase(render_frame_host->GetFrameTreeNodeId()); + } + // WebFrameMain::FromRenderFrameHost(rfh) will use the RFH's FrameTreeNode ID // to find an existing instance of WebFrameMain. During a cross-origin // navigation, the deleted RFH will be the old host which was swapped out. In @@ -3258,6 +3270,42 @@ v8::Local WebContents::TakeHeapSnapshot( return handle; } +void WebContents::GrantDevicePermission( + const url::Origin& origin, + const base::Value* device, + content::PermissionType permissionType, + content::RenderFrameHost* render_frame_host) { + granted_devices_[render_frame_host->GetFrameTreeNodeId()][permissionType] + [origin] + .push_back( + std::make_unique(device->Clone())); +} + +std::vector WebContents::GetGrantedDevices( + const url::Origin& origin, + content::PermissionType permissionType, + content::RenderFrameHost* render_frame_host) { + const auto& devices_for_frame_host_it = + granted_devices_.find(render_frame_host->GetFrameTreeNodeId()); + if (devices_for_frame_host_it == granted_devices_.end()) + return {}; + + const auto& current_devices_it = + devices_for_frame_host_it->second.find(permissionType); + if (current_devices_it == devices_for_frame_host_it->second.end()) + return {}; + + const auto& origin_devices_it = current_devices_it->second.find(origin); + if (origin_devices_it == current_devices_it->second.end()) + return {}; + + std::vector results; + for (const auto& object : origin_devices_it->second) + results.push_back(object->Clone()); + + return results; +} + void WebContents::UpdatePreferredSize(content::WebContents* web_contents, const gfx::Size& pref_size) { Emit("preferred-size-changed", pref_size); diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 3ed91c9f4fcf2..a70af3e24264a 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -20,6 +20,7 @@ #include "content/common/frame.mojom.h" #include "content/public/browser/devtools_agent_host.h" #include "content/public/browser/keyboard_event_processing_result.h" +#include "content/public/browser/permission_type.h" #include "content/public/browser/render_widget_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_delegate.h" @@ -90,6 +91,11 @@ class OffScreenWebContentsView; namespace api { +using DevicePermissionMap = std::map< + int, + std::map>>>>; + // Wrapper around the content::WebContents. class WebContents : public gin::Wrappable, public gin_helper::EventEmitterMixin, @@ -428,6 +434,21 @@ class WebContents : public gin::Wrappable, void DoGetZoomLevel( electron::mojom::ElectronBrowser::DoGetZoomLevelCallback callback); + // Grants |origin| access to |device|. + // To be used in place of ObjectPermissionContextBase::GrantObjectPermission. + void GrantDevicePermission(const url::Origin& origin, + const base::Value* device, + content::PermissionType permissionType, + content::RenderFrameHost* render_frame_host); + + // Returns the list of devices that |origin| has been granted permission to + // access. To be used in place of + // ObjectPermissionContextBase::GetGrantedObjects. + std::vector GetGrantedDevices( + const url::Origin& origin, + content::PermissionType permissionType, + content::RenderFrameHost* render_frame_host); + private: // Does not manage lifetime of |web_contents|. WebContents(v8::Isolate* isolate, content::WebContents* web_contents); @@ -786,6 +807,9 @@ class WebContents : public gin::Wrappable, service_manager::BinderRegistryWithArgs registry_; + // In-memory cache that holds objects that have been granted permissions. + DevicePermissionMap granted_devices_; + base::WeakPtrFactory weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(WebContents); diff --git a/shell/browser/electron_permission_manager.cc b/shell/browser/electron_permission_manager.cc index be66f3c09b5e8..928ec6483e976 100644 --- a/shell/browser/electron_permission_manager.cc +++ b/shell/browser/electron_permission_manager.cc @@ -15,9 +15,17 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" +#include "gin/data_object_builder.h" +#include "shell/browser/api/electron_api_web_contents.h" #include "shell/browser/electron_browser_client.h" #include "shell/browser/electron_browser_main_parts.h" +#include "shell/browser/serial/serial_chooser_context.h" +#include "shell/browser/web_contents_permission_helper.h" #include "shell/browser/web_contents_preferences.h" +#include "shell/common/gin_converters/content_converter.h" +#include "shell/common/gin_converters/frame_converter.h" +#include "shell/common/gin_converters/value_converter.h" +#include "shell/common/gin_helper/event_emitter_caller.h" namespace electron { @@ -277,6 +285,65 @@ bool ElectronPermissionManager::CheckPermissionWithDetails( mutable_details); } +bool ElectronPermissionManager::CheckDevicePermission( + content::PermissionType permission, + const url::Origin& origin, + const base::Value* device, + content::RenderFrameHost* render_frame_host) const { + auto* web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + api::WebContents* api_web_contents = api::WebContents::From(web_contents); + + if (api_web_contents) { + std::vector granted_devices = + api_web_contents->GetGrantedDevices(origin, permission, + render_frame_host); + + for (const auto& granted_device : granted_devices) { + if (permission == + static_cast( + WebContentsPermissionHelper::PermissionType::SERIAL)) { +#if defined(OS_WIN) + if (device->FindStringKey(kDeviceInstanceIdKey) == + granted_device.FindStringKey(kDeviceInstanceIdKey)) + return true; +#else + if (device->FindIntKey(kVendorIdKey) != + granted_device.FindIntKey(kVendorIdKey) || + device->FindIntKey(kProductIdKey) != + granted_device.FindIntKey(kProductIdKey) || + *device->FindStringKey(kSerialNumberKey) != + *granted_device.FindStringKey(kSerialNumberKey)) { + continue; + } + +#if defined(OS_MAC) + if (*device->FindStringKey(kUsbDriverKey) != + *granted_device.FindStringKey(kUsbDriverKey)) { + continue; + } +#endif // defined(OS_MAC) + return true; +#endif // defined(OS_WIN) + } + } + } + return false; +} + +void ElectronPermissionManager::GrantDevicePermission( + content::PermissionType permission, + const url::Origin& origin, + const base::Value* device, + content::RenderFrameHost* render_frame_host) const { + auto* web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + api::WebContents* api_web_contents = api::WebContents::From(web_contents); + if (api_web_contents) + api_web_contents->GrantDevicePermission(origin, device, permission, + render_frame_host); +} + blink::mojom::PermissionStatus ElectronPermissionManager::GetPermissionStatusForFrame( content::PermissionType permission, diff --git a/shell/browser/electron_permission_manager.h b/shell/browser/electron_permission_manager.h index 40013b3e89ca8..f425d89557781 100644 --- a/shell/browser/electron_permission_manager.h +++ b/shell/browser/electron_permission_manager.h @@ -13,6 +13,7 @@ #include "base/containers/id_map.h" #include "base/values.h" #include "content/public/browser/permission_controller_delegate.h" +#include "gin/dictionary.h" namespace content { class WebContents; @@ -77,6 +78,16 @@ class ElectronPermissionManager : public content::PermissionControllerDelegate { const GURL& requesting_origin, const base::DictionaryValue* details) const; + bool CheckDevicePermission(content::PermissionType permission, + const url::Origin& origin, + const base::Value* object, + content::RenderFrameHost* render_frame_host) const; + + void GrantDevicePermission(content::PermissionType permission, + const url::Origin& origin, + const base::Value* object, + content::RenderFrameHost* render_frame_host) const; + protected: void OnPermissionResponse(int request_id, int permission_id, diff --git a/shell/browser/serial/electron_serial_delegate.cc b/shell/browser/serial/electron_serial_delegate.cc index 468148a07d28a..cf68867ac3639 100644 --- a/shell/browser/serial/electron_serial_delegate.cc +++ b/shell/browser/serial/electron_serial_delegate.cc @@ -71,8 +71,7 @@ bool ElectronSerialDelegate::HasPortPermission( auto* chooser_context = SerialChooserContextFactory::GetForBrowserContext(browser_context); return chooser_context->HasPortPermission( - frame->GetLastCommittedOrigin(), - web_contents->GetMainFrame()->GetLastCommittedOrigin(), port); + web_contents->GetMainFrame()->GetLastCommittedOrigin(), port, frame); } device::mojom::SerialPortManager* ElectronSerialDelegate::GetPortManager( diff --git a/shell/browser/serial/serial_chooser_context.cc b/shell/browser/serial/serial_chooser_context.cc index 5497fb114bcd3..37d8525c77af7 100644 --- a/shell/browser/serial/serial_chooser_context.cc +++ b/shell/browser/serial/serial_chooser_context.cc @@ -10,22 +10,25 @@ #include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "content/public/browser/device_service.h" +#include "content/public/browser/web_contents.h" #include "mojo/public/cpp/bindings/pending_remote.h" +#include "shell/browser/web_contents_permission_helper.h" namespace electron { constexpr char kPortNameKey[] = "name"; constexpr char kTokenKey[] = "token"; + #if defined(OS_WIN) -constexpr char kDeviceInstanceIdKey[] = "device_instance_id"; +const char kDeviceInstanceIdKey[] = "device_instance_id"; #else -constexpr char kVendorIdKey[] = "vendor_id"; -constexpr char kProductIdKey[] = "product_id"; -constexpr char kSerialNumberKey[] = "serial_number"; +const char kVendorIdKey[] = "vendor_id"; +const char kProductIdKey[] = "product_id"; +const char kSerialNumberKey[] = "serial_number"; #if defined(OS_MAC) -constexpr char kUsbDriverKey[] = "usb_driver"; +const char kUsbDriverKey[] = "usb_driver"; #endif // defined(OS_MAC) -#endif // defined(OS_WIN) +#endif // defined(OS_WIN std::string EncodeToken(const base::UnguessableToken& token) { const uint64_t data[2] = {token.GetHighForSerialization(), @@ -81,30 +84,51 @@ base::Value PortInfoToValue(const device::mojom::SerialPortInfo& port) { } SerialChooserContext::SerialChooserContext() = default; + SerialChooserContext::~SerialChooserContext() = default; void SerialChooserContext::GrantPortPermission( - const url::Origin& requesting_origin, - const url::Origin& embedding_origin, - const device::mojom::SerialPortInfo& port) { + const url::Origin& origin, + const device::mojom::SerialPortInfo& port, + content::RenderFrameHost* render_frame_host) { base::Value value = PortInfoToValue(port); port_info_.insert({port.token, value.Clone()}); - ephemeral_ports_[{requesting_origin, embedding_origin}].insert(port.token); + if (CanStorePersistentEntry(port)) { + auto* web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + auto* permission_helper = + WebContentsPermissionHelper::FromWebContents(web_contents); + permission_helper->GrantSerialPortPermission(origin, std::move(value), + render_frame_host); + return; + } + + ephemeral_ports_[origin].insert(port.token); } bool SerialChooserContext::HasPortPermission( - const url::Origin& requesting_origin, - const url::Origin& embedding_origin, - const device::mojom::SerialPortInfo& port) { - auto it = ephemeral_ports_.find({requesting_origin, embedding_origin}); + const url::Origin& origin, + const device::mojom::SerialPortInfo& port, + content::RenderFrameHost* render_frame_host) { + auto it = ephemeral_ports_.find(origin); if (it != ephemeral_ports_.end()) { const std::set ports = it->second; if (base::Contains(ports, port.token)) return true; } - return false; + if (!CanStorePersistentEntry(port)) { + return false; + } + + auto* web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + auto* permission_helper = + WebContentsPermissionHelper::FromWebContents(web_contents); + base::Value value = PortInfoToValue(port); + return permission_helper->CheckSerialPortPermission(origin, std::move(value), + render_frame_host); } // static @@ -168,14 +192,6 @@ void SerialChooserContext::OnPortRemoved( for (auto& observer : port_observer_list_) observer.OnPortRemoved(*port); - std::vector> revoked_url_pairs; - for (auto& map_entry : ephemeral_ports_) { - std::set& ports = map_entry.second; - if (ports.erase(port->token) > 0) { - revoked_url_pairs.push_back(map_entry.first); - } - } - port_info_.erase(port->token); } diff --git a/shell/browser/serial/serial_chooser_context.h b/shell/browser/serial/serial_chooser_context.h index 258307d9ff3cc..59d61cfa29988 100644 --- a/shell/browser/serial/serial_chooser_context.h +++ b/shell/browser/serial/serial_chooser_context.h @@ -30,6 +30,20 @@ class Value; namespace electron { +extern const char kHidVendorIdKey[]; +extern const char kHidProductIdKey[]; + +#if defined(OS_WIN) +extern const char kDeviceInstanceIdKey[]; +#else +extern const char kVendorIdKey[]; +extern const char kProductIdKey[]; +extern const char kSerialNumberKey[]; +#if defined(OS_MAC) +extern const char kUsbDriverKey[]; +#endif // defined(OS_MAC) +#endif // defined(OS_WIN) + class SerialChooserContext : public KeyedService, public device::mojom::SerialPortManagerClient { public: @@ -39,12 +53,12 @@ class SerialChooserContext : public KeyedService, ~SerialChooserContext() override; // Serial-specific interface for granting and checking permissions. - void GrantPortPermission(const url::Origin& requesting_origin, - const url::Origin& embedding_origin, - const device::mojom::SerialPortInfo& port); - bool HasPortPermission(const url::Origin& requesting_origin, - const url::Origin& embedding_origin, - const device::mojom::SerialPortInfo& port); + void GrantPortPermission(const url::Origin& origin, + const device::mojom::SerialPortInfo& port, + content::RenderFrameHost* render_frame_host); + bool HasPortPermission(const url::Origin& origin, + const device::mojom::SerialPortInfo& port, + content::RenderFrameHost* render_frame_host); static bool CanStorePersistentEntry( const device::mojom::SerialPortInfo& port); @@ -69,11 +83,8 @@ class SerialChooserContext : public KeyedService, blink::mojom::SerialService::GetPortsCallback callback, std::vector ports); - // Tracks the set of ports to which an origin (potentially embedded in another - // origin) has access to. Key is (requesting_origin, embedding_origin). - std::map, - std::set> - ephemeral_ports_; + // Tracks the set of ports to which an origin has access to. + std::map> ephemeral_ports_; // Holds information about ports in |ephemeral_ports_|. std::map port_info_; diff --git a/shell/browser/serial/serial_chooser_controller.cc b/shell/browser/serial/serial_chooser_controller.cc index 01489ba2e6bad..aca9fe0b82cd4 100644 --- a/shell/browser/serial/serial_chooser_controller.cc +++ b/shell/browser/serial/serial_chooser_controller.cc @@ -67,9 +67,9 @@ SerialChooserController::SerialChooserController( : WebContentsObserver(web_contents), filters_(std::move(filters)), callback_(std::move(callback)), - serial_delegate_(serial_delegate) { - requesting_origin_ = render_frame_host->GetLastCommittedOrigin(); - embedding_origin_ = web_contents->GetMainFrame()->GetLastCommittedOrigin(); + serial_delegate_(serial_delegate), + render_frame_host_id_(render_frame_host->GetGlobalFrameRoutingId()) { + origin_ = web_contents->GetMainFrame()->GetLastCommittedOrigin(); chooser_context_ = SerialChooserContextFactory::GetForBrowserContext( web_contents->GetBrowserContext()) @@ -125,8 +125,8 @@ void SerialChooserController::OnDeviceChosen(const std::string& port_id) { return ptr->token.ToString() == port_id; }); if (it != ports_.end()) { - chooser_context_->GrantPortPermission(requesting_origin_, - embedding_origin_, *it->get()); + auto* rfh = content::RenderFrameHost::FromID(render_frame_host_id_); + chooser_context_->GrantPortPermission(origin_, *it->get(), rfh); RunCallback(it->Clone()); } else { RunCallback(/*port=*/nullptr); diff --git a/shell/browser/serial/serial_chooser_controller.h b/shell/browser/serial/serial_chooser_controller.h index 85c3759082529..342035d36f2f3 100644 --- a/shell/browser/serial/serial_chooser_controller.h +++ b/shell/browser/serial/serial_chooser_controller.h @@ -10,6 +10,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" +#include "content/public/browser/global_routing_id.h" #include "content/public/browser/serial_chooser.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" @@ -53,8 +54,7 @@ class SerialChooserController final : public SerialChooserContext::PortObserver, std::vector filters_; content::SerialChooser::Callback callback_; - url::Origin requesting_origin_; - url::Origin embedding_origin_; + url::Origin origin_; base::WeakPtr chooser_context_; @@ -62,6 +62,8 @@ class SerialChooserController final : public SerialChooserContext::PortObserver, base::WeakPtr serial_delegate_; + content::GlobalFrameRoutingId render_frame_host_id_; + base::WeakPtrFactory weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(SerialChooserController); diff --git a/shell/browser/web_contents_permission_helper.cc b/shell/browser/web_contents_permission_helper.cc index 82273f6adc73d..3f2609d0d0d41 100644 --- a/shell/browser/web_contents_permission_helper.cc +++ b/shell/browser/web_contents_permission_helper.cc @@ -94,6 +94,28 @@ bool WebContentsPermissionHelper::CheckPermission( details); } +bool WebContentsPermissionHelper::CheckDevicePermission( + content::PermissionType permission, + const url::Origin& origin, + const base::Value* device, + content::RenderFrameHost* render_frame_host) const { + auto* permission_manager = static_cast( + web_contents_->GetBrowserContext()->GetPermissionControllerDelegate()); + return permission_manager->CheckDevicePermission(permission, origin, device, + render_frame_host); +} + +void WebContentsPermissionHelper::GrantDevicePermission( + content::PermissionType permission, + const url::Origin& origin, + const base::Value* device, + content::RenderFrameHost* render_frame_host) const { + auto* permission_manager = static_cast( + web_contents_->GetBrowserContext()->GetPermissionControllerDelegate()); + permission_manager->GrantDevicePermission(permission, origin, device, + render_frame_host); +} + void WebContentsPermissionHelper::RequestFullscreenPermission( base::OnceCallback callback) { RequestPermission( @@ -168,6 +190,24 @@ bool WebContentsPermissionHelper::CheckSerialAccessPermission( static_cast(PermissionType::SERIAL), &details); } +bool WebContentsPermissionHelper::CheckSerialPortPermission( + const url::Origin& origin, + base::Value device, + content::RenderFrameHost* render_frame_host) const { + return CheckDevicePermission( + static_cast(PermissionType::SERIAL), origin, + &device, render_frame_host); +} + +void WebContentsPermissionHelper::GrantSerialPortPermission( + const url::Origin& origin, + base::Value device, + content::RenderFrameHost* render_frame_host) const { + return GrantDevicePermission( + static_cast(PermissionType::SERIAL), origin, + &device, render_frame_host); +} + WEB_CONTENTS_USER_DATA_KEY_IMPL(WebContentsPermissionHelper) } // namespace electron diff --git a/shell/browser/web_contents_permission_helper.h b/shell/browser/web_contents_permission_helper.h index d0732941ffef1..16e137c2e94c1 100644 --- a/shell/browser/web_contents_permission_helper.h +++ b/shell/browser/web_contents_permission_helper.h @@ -40,6 +40,14 @@ class WebContentsPermissionHelper bool CheckMediaAccessPermission(const GURL& security_origin, blink::mojom::MediaStreamType type) const; bool CheckSerialAccessPermission(const url::Origin& embedding_origin) const; + bool CheckSerialPortPermission( + const url::Origin& origin, + base::Value device, + content::RenderFrameHost* render_frame_host) const; + void GrantSerialPortPermission( + const url::Origin& origin, + base::Value device, + content::RenderFrameHost* render_frame_host) const; private: explicit WebContentsPermissionHelper(content::WebContents* web_contents); @@ -53,6 +61,16 @@ class WebContentsPermissionHelper bool CheckPermission(content::PermissionType permission, const base::DictionaryValue* details) const; + bool CheckDevicePermission(content::PermissionType permission, + const url::Origin& origin, + const base::Value* device, + content::RenderFrameHost* render_frame_host) const; + + void GrantDevicePermission(content::PermissionType permission, + const url::Origin& origin, + const base::Value* device, + content::RenderFrameHost* render_frame_host) const; + content::WebContents* web_contents_; WEB_CONTENTS_USER_DATA_KEY_DECL();