-
Notifications
You must be signed in to change notification settings - Fork 439
updater: allow accepting invalid TLS certs/hostnames via config #3057
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
base: v2
Are you sure you want to change the base?
updater: allow accepting invalid TLS certs/hostnames via config #3057
Conversation
Package Changes Through fce85baThere are 26 changes which include barcode-scanner with patch, barcode-scanner-js with patch, biometric with patch, biometric-js with patch, clipboard-manager with patch, clipboard-manager-js with patch, deep-link with patch, deep-link-js with patch, dialog with patch, dialog-js with patch, fs with patch, fs-js with patch, geolocation with patch, geolocation-js with patch, haptics with patch, haptics-js with patch, nfc with patch, nfc-js with patch, notification with patch, notification-js with patch, opener with patch, opener-js with patch, shell with patch, shell-js with patch, updater with minor, updater-js with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
Would resolve the error (Issue 2579) caused by strict TLS certificate validation. |
|
Thanks for the PR! would you mind adding a small changefile as well? There are some examples in the .changes dir :) |
|
argh, i just noticed that we export the Config struct which means that this is a breaking change........ Before thinking about alternative implementations, let us wait a few days and see how the discussions about an early v3 release we're currently having goes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
|
Thanks, I understand. I’ll wait for the v3 discussion and follow whatever the outcome is. If you prefer not to introduce a breaking change now, I can revert the Config edits and keep the functionality available only via the UpdaterBuilder (which should be non-breaking). Tell me which you’d like and I’ll update the PR accordingly. |
|
yeah, give us a few days to see if the v3 discussion is going anywhere. The UpdaterBuilder idea would be a good alternative indeed though a bit unfortunate that we'd then have 2 places for dangerous settings. |
Problem: updater fails when contacting servers with self-signed or internal TLS certs and error message is generic.
Solution: add two plugin config flags
dangerousAcceptInvalidCertsanddangerousAcceptInvalidHostnamesand builder methods to override them. These mirror http-plugin and reqwest'sdanger_accept_invalid_certsanddanger_accept_invalid_hostnames.Security: these settings are dangerous and should only be used in trusted environments or testing.