Automatically use secure WebSocket on HTTPS pages #371
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, driver used in browser used secure WebSocket with 'wss'
scheme only when encryption was explicitly turned on in the driver
config. On HTTP web pages it is perfectly fine to use either plane
or secure WebSocket ('ws' and 'wss' respectively). On HTTPS web
pages only 'wss' is allowed. So explicit `{encrypted: true}`
configuration was always required for drivers used on HTTPS web pages.
This commit makes driver automatically use secure WebSocket when driver
is used on a HTTPS web page.
lutovich
force-pushed
the
1.7-wss-in-https
branch
from
26669bb to
e8b4bcd
Compare
ali-ince
approved these changes
May 16, 2018
| * @param {function(): string} protocolSupplier - function that detects protocol of the web page. | ||
| * @return {{scheme: string|null, error: Neo4jError|null}} object containing either scheme or error. | ||
| */ | ||
| function determineWebSocketScheme(config, protocolSupplier) { |
There was a problem hiding this comment.
Based on the information given in Security Considerations section here, it may make sense generating warning message about mixing protocols may not work - if encryption is explicitly turned off or on (i.e. https/ws or http/wss).
There was a problem hiding this comment.
Added a warn print in c623d8a
src/v1/internal/ch-websocket.js
Outdated
|
|
||
| if (encrypted === true || encrypted === ENCRYPTION_ON) { | ||
| // encryption explicitly requested in the config | ||
| if (!trust || trust === 'TRUST_CUSTOM_CA_SIGNED_CERTIFICATES') { |
There was a problem hiding this comment.
This is totally out of scope of this PR, but I find TRUST_CUSTOM_CA_SIGNED_CERTIFICATES setting to be confusing and would prefer TRUST_SYSTEM_CA_SIGNED_CERTIFICATES as the default. We may consider revising naming / behaviour in upcoming releases?
There was a problem hiding this comment.
I'll create a card for this
Encryption can be explicitly turned on or off in the driver config.
Driver can be used on both HTTP and HTTPS web pages. Configured
encryption has to match the security protocol of the web page. Insecure
WebSockets ('ws') have to be used on insecure pages ('http'). Secure
WebSockets ('wss') have to be used on secure pages ('https').
WebSockets might not work in a mixed content environment,
i.e. 'ws' + 'https' or 'wss' + 'http'.
This commit makes driver print a warning when its configuration is
inconsistent with the web page.
ali-ince
approved these changes
May 23, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, driver used in browser used secure WebSocket with 'wss' scheme only when encryption was explicitly turned on in the driver config. On HTTP web pages it is perfectly fine to use either plane or secure WebSocket ('ws' and 'wss' respectively). On HTTPS web pages only 'wss' is allowed. So explicit
{encrypted: true}configuration was always required for drivers used on HTTPS web pages.This PR makes driver automatically use secure WebSocket when driver is used on a HTTPS web page.
Resolves #255