Skip to content

Restrict SpeechRecognition and friends to secure contexts #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 13, 2025
Merged

Restrict SpeechRecognition and friends to secure contexts #31

merged 1 commit into from
May 13, 2025

Conversation

foolip
Copy link
Collaborator

@foolip foolip commented Aug 6, 2018 •
edited by pr-preview bot
Loading

@yoavweiss yoavweiss changed the base branch from master to main March 5, 2021 21:38
@padenot
Copy link
Member

padenot commented Feb 28, 2025

I think we want to merge this, despite being of a certain vintage?

@evanbliu, @youennf what's your respective opinion on this. Mozilla would rather only do this on SecureContext.

@padenot
Copy link
Member

padenot commented Apr 23, 2025

@evanbliu, @youennf, any comment on this?

@youennf
Copy link

youennf commented Apr 23, 2025

LGTM, this is worth a try.
PR seems to need a refresh.

@padenot padenot force-pushed the recognition-SecureContext branch from e7ef1f2 to 71bfe55 Compare April 23, 2025 09:07
@padenot
Copy link
Member

padenot commented Apr 23, 2025

Done.

@youennf
Copy link

youennf commented Apr 23, 2025

SpeechRecognition uses microphone so that makes sense to restrict.

For SpeechSynthesis, I am less sure, do we have some numbers there?
Maybe we could split the issue.

@padenot
Copy link
Member

padenot commented Apr 23, 2025

SpeechRecognition uses microphone so that makes sense to restrict.

Not in the current state of the spec, no, it doesn't have to. It can take its input from a MediaStream.

@youennf
Copy link

youennf commented Apr 24, 2025

Not in the current state of the spec

This is a new API so the web compat risk is very low.
The initial intent of the PR was based on Chrome's change to disable SpeechRecognition, which is not functional in insecure websites.
On the other hand, SpeechSynthesis is functional in insecure websites so the web compat story is less clear.

@padenot
Copy link
Member

padenot commented Apr 24, 2025

This is a new API so the web compat risk is very low.

It is not, it has been shipped for about 11 years in Chrome, says MDN.

@evanbliu, what's Chrome opinion on this matter?

@youennf
Copy link

youennf commented Apr 24, 2025

It is not, it has been shipped for about 11 years in Chrome, says MDN.

SpeechRecognition with microphone is not new, but SpeechRecoginition with any MediaStreamTrack is new AFAIK and is probably not a web compat issue.

@evanbliu
Copy link
Collaborator

Oops, sorry I missed this. Restricting SpeechRecognition to secure contexts sounds fine to me! But it seems like this isn't necessary for SpeechSynthesis.

@evanbliu
Copy link
Collaborator

@padenot & @youennf - What are your thoughts on gating the Web Speech API (or at least the on-device parts of it) behind a Permission Policy? Without this limitation, if a legitimate site using this API embeds a third-party iframe (e.g. an ad), that ad could read the same fingerprinting bits through availableOnDevice().

@youennf
Copy link

youennf commented Apr 29, 2025

What are your thoughts on gating the Web Speech API (or at least the on-device parts of it) behind a Permission Policy?

This might deserve its own bug.
In general, if availableOnDevice() is opening new fingerprinting, the spec should say so and we should evaluate how to mitigate this/remove this. In general, I do not see Permission Policy as the right tool against fingerprinting.

I would instead tend to reduce what is being exposed to the bare minimum, do we have a bug tracker for that, or should I file one?
For instance, how about a boolean stating that website wants speech recognition to be done on device.

Getting back to SecureContext, @padenot, what are your thoughts?

@padenot
Copy link
Member

padenot commented Apr 30, 2025

Mozilla's SecureContext policy is still https://blog.mozilla.org/security/2018/01/15/secure-contexts-everywhere/.

SpeechRecognition should be SecureContext for obvious reasons. Synthesis should be SecureContet if we can make it so web-compat wise.

@youennf
Copy link

youennf commented Apr 30, 2025

Can we then only tackle SpeechRecognition in this PR and open a separate issue to further study SpeechSynthesis?
I'd like to update WebKit when the spec is updated for SpeechRecognition.

@youennf
Copy link

youennf commented Apr 30, 2025

Also, it would be nice to have a WPT legacy test to check that the constructs no longer appear in non secure contexts.

@padenot padenot force-pushed the recognition-SecureContext branch from 71bfe55 to b94c977 Compare May 5, 2025 09:24
@padenot padenot force-pushed the recognition-SecureContext branch from b94c977 to da41368 Compare May 5, 2025 09:36
@padenot
Copy link
Member

padenot commented May 5, 2025

Can we then only tackle SpeechRecognition in this PR and open a separate issue to further study SpeechSynthesis? I'd like to update WebKit when the spec is updated for SpeechRecognition.

Done. We can write a idlharness test after merging.

youennf
youennf reviewed May 5, 2025
View reviewed changes
Copy link

@youennf youennf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unofficial LGTM (it seems I cannot approve PRs here).

@padenot padenot requested a review from evanbliu May 5, 2025 11:18
@padenot
Copy link
Member

padenot commented May 5, 2025

I've formally asked @evanbliu for a review, assuming Google is fine w/ the change.

@padenot padenot merged commit ff12a0d into main May 13, 2025
1 check passed
github-actions bot added a commit that referenced this pull request May 13, 2025
@foolip @github-actions
Make SpeechRecognition objects [SecureContext] (#31)
f69ef74 
SHA: ff12a0d
Reason: push, by padenot

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
youennf added a commit to youennf/WebKit that referenced this pull request May 13, 2025
@youennf
SpeechRecognition interface and friends should be SecureContext
4f37a72 
rdar://151240414
https://bugs.webkit.org/show_bug.cgi?id=292941

Reviewed by NOBODY (OOPS!).

Align implementation with WebAudio/web-speech-api#31.

* LayoutTests/http/wpt/mediastream/speechrecognition-insecure-expected.txt: Added.
* LayoutTests/http/wpt/mediastream/speechrecognition-insecure.html: Added.
* Source/WebCore/Modules/speech/SpeechRecognition.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionAlternative.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionErrorEvent.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionResult.idl:
@youennf youennf mentioned this pull request May 13, 2025
Merged
webkit-commit-queue pushed a commit to youennf/WebKit that referenced this pull request May 14, 2025
@youennf
SpeechRecognition interface and friends should be SecureContext
f60ba77 
rdar://151240414
https://bugs.webkit.org/show_bug.cgi?id=292941

Reviewed by Brady Eidson, Per Arne Vollan, and Sihui Liu.

Align implementation with WebAudio/web-speech-api#31.

* LayoutTests/http/wpt/mediastream/speechrecognition-insecure-expected.txt: Added.
* LayoutTests/http/wpt/mediastream/speechrecognition-insecure.html: Added.
* Source/WebCore/Modules/speech/SpeechRecognition.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionAlternative.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionErrorEvent.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionEvent.idl:
* Source/WebCore/Modules/speech/SpeechRecognitionResult.idl:

Canonical link: https://commits.webkit.org/294887@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youennf youennf youennf left review comments

@evanbliu evanbliu evanbliu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@foolip @padenot @youennf @evanbliu