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

cmd/tailscale: disable HTTPS verification for QNAP auth. #6919

Merged
merged 1 commit into from Jan 11, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion cmd/tailscale/cli/web.go
Expand Up @@ -247,7 +247,14 @@ func qnapAuthnSid(r *http.Request, user, sid string) (string, *qnapAuthResponse,
}

func qnapAuthnFinish(user, url string) (string, *qnapAuthResponse, error) {
resp, err := http.Get(url)
// QNAP Force HTTPS mode uses a self-signed certificate. Even importing
// the QNAP root CA isn't enough, the cert doesn't have a usable CN nor
// SAN. See https://github.com/tailscale/tailscale/issues/6903
tr := &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a well-meaning bot. The point of this PR is that QNAP's feature forces us to use HTTPS and then ignore the certificate it presents.

I'm looking for a way to add a comment or otherwise suppress this finding, we don't want it firing every time someone modifies this file.


From github/codeql#7937 it looks like there isn't a way to do this inline using code comments.
github/codeql-action#1098 provides a way to modify the GitHub Action.
Update: github/codeql#4786 implies that it does not work for Go.

}
client := &http.Client{Transport: tr}
resp, err := client.Get(url)
if err != nil {
return "", nil, err
}
Expand Down