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

[Bug] TLS is not verified during OAuth callback #9373

Closed
mattbostock opened this issue Sep 28, 2017 · 3 comments · Fixed by #9378
Closed

[Bug] TLS is not verified during OAuth callback #9373

mattbostock opened this issue Sep 28, 2017 · 3 comments · Fixed by #9378
Milestone

Comments

@mattbostock
Copy link
Contributor

  • What Grafana version are you using?

4.5.1

  • What OS are you running grafana on?

Linux

See:

tr := &http.Transport{
TLSClientConfig: &tls.Config{
InsecureSkipVerify: true,
Certificates: []tls.Certificate{cert},
RootCAs: caCertPool,
},
}
sslcli := &http.Client{Transport: tr}
oauthCtx = context.Background()
oauthCtx = context.WithValue(oauthCtx, oauth2.HTTPClient, sslcli)
}

TLS should be verified by default.

Related: #8812

@bergquist
Copy link
Contributor

💯

But this would have to be configurable for those who don't want it. Not sure if it should be one global setting for all outgoing requests or one per oauth, datasources, alert notifications. etc

@mattbostock
Copy link
Contributor Author

I made a start here: #9378

I haven't added a UI option, but wanted to get something out so that at least others can patch their deployment if they wish to.

#9377 tackles this more comprehensively for datasources and adds an explicit 'skip verify' option.

@mattbostock
Copy link
Contributor Author

Seems that TLS verification is only skipped if TLS client authentication is enabled: #9419, which mitigates the issue for some installations.

@bergquist bergquist added this to the 4.6 milestone Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants