Skip to content

[fix](sdk) Enable TLS certificate verification by default in Go SDK#64925

Open
arpitjain099 wants to merge 1 commit into
apache:masterfrom
arpitjain099:fix/go-sdk-tls-verify
Open

[fix](sdk) Enable TLS certificate verification by default in Go SDK#64925
arpitjain099 wants to merge 1 commit into
apache:masterfrom
arpitjain099:fix/go-sdk-tls-verify

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #64570

Related PR: N/A

Problem Summary:
The Go SDK HTTP client (pkg/load/util/http_client.go) hardcodes InsecureSkipVerify: true in its default TLS configuration. All SDK API calls, including those carrying authentication credentials, skip TLS certificate verification. This exposes Doris database credentials to man-in-the-middle attacks on any Go SDK client connection (CWE-295).

The fix enables TLS verification by default (the Go standard library default behavior) and adds a configurable InsecureSkipVerify field to Config for environments that use self-signed certificates.

Since InsecureSkipVerify is a bool, its zero value is false, so existing callers that don't set the field get the secure default without code changes.

Release note

Fixed a security issue in the Go SDK where TLS certificate verification was disabled by default, exposing credentials to potential MITM attacks. TLS verification is now enabled by default. Users who need to connect to Doris clusters with self-signed certificates can set InsecureSkipVerify: true in their Config.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
      • Verified that the test in http_client_test.go still passes (it uses httptest.NewServer which is plain HTTP, so TLS config is not exercised). The InsecureSkipVerify field was also removed from the test helper since it is not needed for the HTTP test server.
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • Yes. TLS certificate verification is now enabled by default. Clusters using self-signed certificates will need to set InsecureSkipVerify: true in their SDK config to maintain the previous behavior.
  • Does this need documentation?

    • Yes. The new InsecureSkipVerify config field should be documented in the Go SDK README.

The HTTP client in the Go SDK hardcodes InsecureSkipVerify: true,
which disables TLS certificate verification for all connections.
This exposes database credentials to man-in-the-middle attacks
on any Go SDK client connection (CWE-295).

This patch enables TLS verification by default and adds an
InsecureSkipVerify option to the Config struct for environments
where self-signed certificates are expected.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

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.

CWE-295: Go SDK HTTP client hardcodes InsecureSkipVerify:true by default

2 participants