-
Notifications
You must be signed in to change notification settings - Fork 95
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.
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
Create worker code from module instead of from string #563
Comments
@jeshua-clipchamp, Thank you for using Speech SDK, and writing up this issue. This admittedly obtuse method of setting timeouts is a workaround for allowing continued recognition when the browser is minimized. (See #74 for more information.) Regressing this behavior is not tenable, obviously, and I can't make any promises yet, but adding an option not to use the worker seems doable, as long as the browser-minimized regression is not something you care about when that option is set. I'll update this issue when I get a chance to investigate further. |
@jeshua-clipchamp Having investigated this issue, I'm curious why the worker in PCMRecorder (created from a string) is acceptable, but the timeout worker is not. I'm also curious about the issue you're describing with Content-Security-Policy, as you can specify the allowed URL of any workers using the worker-src tag, described here. Would you mind elaborating a bit on why worker-src doesn't work for your use case? |
Closing this on lack of response to follow-up questions, may re-open on response. |
Whoops, sorry about the lack of response Github decided to not notify me about your replies 😅 For your first question, I assume that For your second point, Having the worker code as a separate file and loading it from that should be enough to fix this (because then we could do |
@jeshua-clipchamp When looking at this before, using a data URL seemed to be a way forward that works with our current build processes. See this branch's Timeout.ts for an example of loading a base64 string as a worker. Let me know your thoughts. |
I suspect that would have the same kind of problem (it's still loading a worker from a string after all). Another option might be to allow users to supply the worker themselves, then we could do some shenanigans to build the worker from a file or something? |
@jeshua-clipchamp From Using Web Workers: The exception to this is if the worker script's origin is a globally unique identifier (for example, if its URL has a scheme of data or blob). In this case, the worker does inherit the CSP of the document or worker that created it." |
Right, but this means you have to include |
@jeshua-clipchamp In the upcoming release, the code will be included as a data url, as shown in the merged PR above. |
Hmmm I'll give that a go when it comes out, thanks for the fix :) I'm interested as to whether you can only allow data URLs from specific domains with CSP but I guess we'll see :D |
@jeshua-clipchamp JS Speech SDK v1.24 has been released, with this fix included. Thanks again for submitting this issue! |
I'm not sure about security, but isn't there any difference between blobs and data other than the encoding method? |
From what I can tell the provided fix might not resolve the original concern, as the Would it be possible to please get an option to disable this worker in high-security environments, with the caveat that accurate timing information will be lost when the tab is minimised? For our specific use-case we're already patching this out and just using a standard browser |
@me4502 Thanks for looking at and thinking about this issue. Can the "hash-algorithm"-"base64-value" attribute not being applied to this worker? |
After testing that, it does appear to work. Thanks for pointing that one out, I was just looking at the My only concern with this is that when the library updates it's an extra step to ensure that the base64'd hash is updated alongside it, but that's not too major an issue. |
Actually, we've found that while the hash I setup works on macOS (the OS I initially tested on), the value appears to differ on Windows. Unsure if this is something encoding related, but given it appears to differ here there's a possibility it'll differ further based on other factors. Due to this, this doesn't seem to be a viable fix |
What's the problem?
In Timeout.ts a worker is loaded from a hard-coded string within the file. This doesn't play nicely with the
Content-Security-Policy
, where if you allow creating workers from strings you have to let anyone create a worker from a string (as far as I could tell there is no way to limit it to specific libraries, worker code with specific hashes, from files with specific hashes etc.). This is a pretty big security hole for something which is relatively unimportant (my understanding of this worker is to provide accurate timing if the user changes tabs, is that correct?).We are currently working around this by patching the source code on checkout to just use the standard
setTimeout
:Possible solutions
worker-src 'self'
should work)The text was updated successfully, but these errors were encountered: