Skip to content
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

fix invalid HTTPS check #58

Closed
wants to merge 1 commit into from

Conversation

grische
Copy link
Contributor

@grische grische commented Nov 20, 2023

The old code ran wget and checked only if exit code was '1' and then used HTTP as a fallback.

wget returns several different exit code:
https://www.gnu.org/software/wget/manual/html_node/Exit-Status.html

In case for example an "SSL verification failure." occured (expired certs) the fallback did not trigger and it tried but failed to establish an HTTPS connection.

The old code ran wget and checked only if exit code was '1' and then used
HTTP as a fallback.

wget returns several different exit code:
https://www.gnu.org/software/wget/manual/html_node/Exit-Status.html

In case for example an "SSL verification failure." occured (expired certs)
the fallback did not trigger and it tried but failed to establish an HTTPS
connection.
@grische grische marked this pull request as draft November 20, 2023 22:14
@grische
Copy link
Contributor Author

grische commented Nov 20, 2023

This needs some more discussion as this logic change might have unintended side-effects.
Currently, an existing Gluon router does:

root@ffmuc-d88xxxxxxxxxx:~# wget -q https://[[::1]]
Failed to send request: Operation not permitted
root@ffmuc-d88xxxxxxxxxx:~# echo $?
4

@maurerle
Copy link
Member

If no ssl lib is installed,
wget -q "https://[::1] returns 1 (do not know what to do - generic) -> use http
if any lib is installed:
wget -q "https://[::1] returns 4 (network error) -> https is available and can be used

So checking for 0 or not equals 0 does not help here.
Maybe there is a better way to check if any ssl lib is installed

@grische
Copy link
Contributor Author

grische commented Nov 20, 2023

Yeah, that's also we just figured out in ffmuc. So either checking for -ne 4 or -eq 1. I will close this for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants