Bug 173457 – Consider allow gUM to be called from localhost without https

• Component: Media
• Status: RESOLVED FIXED

[Jon Lee]

https://stackoverflow.com/questions/44523609/getusermedia-is-not-allowed-in-localhost-safari-11

[David Zulaica]

getUserMedia is also not available with the file:/// protocol.

[Eric Carlson]

Note that it is possible to enable/disable gUM on insecure sites with the Safari Develop menu. See https://webkit.org/blog/7763/a-closer-look-into-webrtc for details.

[Eric Carlson]

Created attachment 318060 [details]
Proposed patch.

[youenn fablet]

Comment on attachment 318060 [details]
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=318060&action=review

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:105
> +    return equalLettersIgnoringASCIICase(response.url().host(), "localhost");

So only localhost and not 127.0.0.1?
Do you know what other browsers are doing?

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:111
> +    if (requiresSecureConnection && !isSecure(*document.loader()) && !isLocalhost(*document.loader())) {

Geolocation is not allowing localhost/127.0.0.1 but it is accepting file URLs.
I wonder whether we should not make these two consistent.
If so, we should come up with a common routine.

[Radar WebKit Bug Importer]

<rdar://problem/33900527>

[Eric Carlson]

Created attachment 318159 [details]
Patch for landing.

[youenn fablet]

Comment on attachment 318159 [details]
Patch for landing.


View in context: https://bugs.webkit.org/attachment.cgi?id=318159&action=review

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:98
> +    return SchemeRegistry::shouldTreatURLSchemeAsSecure(response.url().protocol().toStringWithoutCopying())

I did not know about toStringWithoutCopying method.
Maybe shouldTreatURLSchemeAsSecure should take a StringView though?

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:107
> +    if (requiresSecureConnection && !isSecure(documentLoader) && !SecurityOrigin::isLocalHostOrLoopbackIPAddress(documentLoader.response().url())) {

Why not placing SecurityOrigin::isLocalHostOrLoopbackIPAddress call in isSecure function?

> Source/WebCore/page/SecurityOrigin.cpp:134
> +    if (SecurityOrigin::isLocalHostOrLoopbackIPAddress(url))

Sounds good although unrelated to the initial purpose of that change.
Looking at https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy, it also considers www.localhost as potentially trustworthy.
Maybe we should split out this part and try realigning our implementation with the spec.

> Source/WebCore/page/SecurityOrigin.cpp:592
> +    // FIXME: Ensure that localhost resolves to the loopback address.

As per the spec, this FIXME might no longer be something we might care about.

[Eric Carlson]

Comment on attachment 318159 [details]
Patch for landing.

View in context: https://bugs.webkit.org/attachment.cgi?id=318159&action=review

>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:107
>> +    if (requiresSecureConnection && !isSecure(documentLoader) && !SecurityOrigin::isLocalHostOrLoopbackIPAddress(documentLoader.response().url())) {
> 
> Why not placing SecurityOrigin::isLocalHostOrLoopbackIPAddress call in isSecure function?

I am not an expert in this area, but I assume SecurityOrigin::isPotentionallyTrustworthy allows localhost/loopback and SecurityOrigin::isSecure doesn't by design, so I will leave this as is for now and talk to @dbates about it.

>> Source/WebCore/page/SecurityOrigin.cpp:134
>> +    if (SecurityOrigin::isLocalHostOrLoopbackIPAddress(url))
> 
> Sounds good although unrelated to the initial purpose of that change.
> Looking at https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy, it also considers www.localhost as potentially trustworthy.
> Maybe we should split out this part and try realigning our implementation with the spec.

This will have implications for other parts of WebKit, so lets leave that for another change.

>> Source/WebCore/page/SecurityOrigin.cpp:592
>> +    // FIXME: Ensure that localhost resolves to the loopback address.
> 
> As per the spec, this FIXME might no longer be something we might care about.

I think we do for now because https://w3c.github.io/webappsec-secure-contexts/#localhost says that user agents MAY treat localhost names as having potentially trustworthy origins "if and only if ... localhost never resolves to a non-loopback address". Others are following discussions elsewhere about changing this, so this comment will be removed if/when that changes.

[WebKit Commit Bot]

Comment on attachment 318159 [details]
Patch for landing.

Rejecting attachment 318159 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 318159, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
.webkit.org/git/WebKit
   4ef9ce9..1fb5568  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 220758 = 4ef9ce9da4ece36525787f3029fc0747c27790c8
r220759 = 1fb556876104fc0d5aaf782e9e62313e62d0acde
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/4319604

[Eric Carlson]

Comment on attachment 318159 [details]
Patch for landing.

View in context: https://bugs.webkit.org/attachment.cgi?id=318159&action=review

>>> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:107
>>> +    if (requiresSecureConnection && !isSecure(documentLoader) && !SecurityOrigin::isLocalHostOrLoopbackIPAddress(documentLoader.response().url())) {
>> 
>> Why not placing SecurityOrigin::isLocalHostOrLoopbackIPAddress call in isSecure function?
> 
> I am not an expert in this area, but I assume SecurityOrigin::isPotentionallyTrustworthy allows localhost/loopback and SecurityOrigin::isSecure doesn't by design, so I will leave this as is for now and talk to @dbates about it.

Oops, Dan pointed out that you were suggesting I do this check inside of isSecure in *this* file. I didn't do that because semantically localhost/loopback isn't secure, but it is something we allow as an exception.

[Eric Carlson]

Created attachment 318177 [details]
Rebased patch.

[WebKit Commit Bot]

Comment on attachment 318177 [details]
Rebased patch.

Rejecting attachment 318177 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 318177, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
    -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 220761 = 0f5ef5f5c48ee19395f265454cec76a57a23306f
r220762 = 70a4f6f95592494e9aa0c812d3a03e08630ecc0f
r220763 = abc7aa56fed15ca049b6c3cd6c35949bf1412e48
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/4320064

[WebKit Commit Bot]

Comment on attachment 318177 [details]
Rebased patch.

Clearing flags on attachment: 318177

Committed r220805: <http://trac.webkit.org/changeset/220805>

[Sam Sneddon [:gsnedders]]

This appears to have been fixed by the above commit?