feat: allow insecure https #68
Open
Conversation
This file contains 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
stapelberg
reviewed
Jun 5, 2024
| @@ -1467,7 +1467,7 @@ func (pack *Pack) logic(programName string) error { | |||
| done := measure.Interactively("probing https") | |||
| remoteScheme, err := httpclient.GetRemoteScheme(updateBaseUrl) | |||
| done("") | |||
| if remoteScheme == "https" && !tlsflag.Insecure() { | |||
There was a problem hiding this comment.
I don’t think we can make this change. The -insecure flag is supposed to prevent HTTPS probing to allow folks to first enable TLS.
Based on the error message you provided, it seems to me like the updater.NewTarget() call in line 1494 is what fails, so I’m not sure how your suggested change helps with that…?
There was a problem hiding this comment.
GetRemoteScheme does not return https if TLS isn't enabled.
And this insecure check prevents to keep setting the address scheme to https when the server actually supports https. So it fallbacks to http (when passing insecure), which doesn't work with TLS enabled.
The original https error is unrelated to this fix.
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.
When changing machine and forgetting to back up
~/.config/gokrazy, you can end up in a locked state if you have SSL enabled.Basically, running
gok update --insecurewill give, as the instance has TLS enabled:2024/06/03 00:17:18 updating root file system: unexpected HTTP status code: got 400 Bad Request, want 200 (body "expected a PUT request\n")On the other hand, not using the insecure flag will fail at certificate verification:
2024/06/03 00:07:36 checking target partuuid support: Get "https://gokrazy:***@rpi-gokrazy/update/features": tls: failed to verify certificate: x509: certificate signed by unknown authority (possibly because of "x509: invalid signature: parent certificate cannot sign this kind of certificate" while trying to verify candidate authority certificate "gokrazy")If TLS is enabled,
GetRemoteSchemegets already the correct scheme, so we should still set it, regardless of the insecure flag, so we can query the correct endpoint (without cert verification).