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

crypto/tls: remote error: tls:internal error #58434

Open
zhaozuowu opened this issue Feb 9, 2023 · 5 comments
Open

crypto/tls: remote error: tls:internal error #58434

zhaozuowu opened this issue Feb 9, 2023 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@zhaozuowu
Copy link

zhaozuowu commented Feb 9, 2023

What version of Go are you using (go version)?

$ go version
go version go1.18 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GO111MODULE="on"
GOARCH="arm64"
GOBIN="/Library/WebServer/Documents/yuebai/bin"
GOCACHE="/Users/zhaozuowu/Library/Caches/go-build"
GOENV="/Users/zhaozuowu/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/zhaozuowu/.gvm/pkgsets/go1.18/global/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/zhaozuowu/.gvm/pkgsets/go1.18/global"
GOPRIVATE=""
GOPROXY="https://goproxy.cn"
GOROOT="/Users/zhaozuowu/.gvm/gos/go1.18"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/zhaozuowu/.gvm/gos/go1.18/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Library/WebServer/Documents/yuebai/src/leak/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pl/tz0tvgq578x0zqh5q6bd41jh0000gn/T/go-build1005337739=/tmp/go-build -gno-record-gcc-switches -fno-common"

go env Output
$ go env

What did you do?

I launched an http get request for an https redirect short link, expecting to get a long redirected connection. As a result, the http client class library directly raised an error and returned remote error: tls: internal error, the service will generate a large number of Ctrip leaks at a moment, the following is used to demonstrate and reproduce the error:

package main

import (
	"crypto/tls"
	"fmt"
	"io/ioutil"
	"net"
	"net/http"
	"net/url"
	"time"
)

var DefaultTransports *http.Transport

func init() {
	DefaultTransports = http.DefaultTransport.(*http.Transport).Clone()
	DefaultTransports.MaxIdleConns = 0
	DefaultTransports.MaxIdleConnsPerHost = 200
	//DefaultTransports.MaxConnsPerHost = 200
	DefaultTransports.IdleConnTimeout = 90 * time.Second
	DefaultTransports.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
	//DefaultTransports.DisableKeepAlives = true
	DefaultTransports.DialContext = (&net.Dialer{
		Timeout:   time.Second * 3,
		KeepAlive: time.Second * 15,
	}).DialContext
}
func main() {

	location, err := GetHeaderLocation("https://p.pinduoduo.com/TGvnEO7z")
	if err != nil {
		switch err.(type) {
		case *url.Error:
			urlErr := err.(*url.Error)
			if urlErr != nil && urlErr.URL != "" {
				location = urlErr.URL
			}
		default:

		}
	}

	fmt.Println(location, err)

}

func GetHeaderLocation(url string) (string, error) {
	req, err := http.NewRequest("GET", url, nil)
	if err != nil {
		//logger.Warnf("http get::url:%s ,request:%+v  err:%v", urlStr, params, err)
		return "", err
	}

	client := &http.Client{
		Timeout:   3 * time.Second,
		Transport: DefaultTransports,
	}
	resp, err := client.Do(req)
	if err != nil {
		//logger.Warnf("http get::url:%s ,request:%+v  err:%v", urlStr, params, err)
		return "", err
	}

	defer resp.Body.Close()

	_, _ = ioutil.ReadAll(resp.Body)
	if resp.Request != nil && resp.Request.Response != nil {
		return resp.Request.Response.Header.Get("Location"), nil

	}
	return "", nil

}


What did you expect to see?

I expect to return a correct http response, not tls:internal error, and not reference (*url.Error). Urls will cause Ctrip to jam, which will lead to memory leaks

What did you see instead?

Get "https://mobile.yangkeduo.com/duo_theme_activity.html?__page=duo_tenmillion_coupon&_pdd_fs=1&pid=1999884_259916104&goods_id=399541643053&goods_sign=E9b2_2zHWblGZC4hzrTbjAEaelPSGvxY_JQvGjmqUmh&customParameters=%7B%22sid%22%3A%221999884_259916104%22%2C%22uid%22%3A%2236245%22%7D&cpsSign=TMC_230207_1999884_259916104_8f28882a69e9c452c5a2761bf264b904&_x_ddjb_act=%7B%22st%22%3A%22128%22%7D&duoduo_type=2": remote error: tls: internal error

@AlexanderYastrebov
Copy link
Contributor

By default http.Client follows redirects so it tries to query "long" link after it receives 302 and gets remote error: tls: internal error.

You may disable redirect following like so:

	client := &http.Client{
		Timeout:   3 * time.Second,
		Transport: DefaultTransports,
		CheckRedirect: func(*http.Request, []*http.Request) error {
			return http.ErrUseLastResponse
		},
	}

Then the part to get Location header value looks weird, it should be just (after redirects are disabled):

	defer resp.Body.Close()

	return resp.Header.Get("Location"), nil

@AlexanderYastrebov
Copy link
Contributor

I am not sure what is going on here but this client:

package main

import (
	"log"
	"net/http"
)

func main() {
	client := &http.Client{
		Transport: &http.Transport{
			//TLSClientConfig: &tls.Config{},
		},
	}

	_, err := client.Get("https://mobile.yangkeduo.com/")
	if err != nil {
		log.Fatalf("error: %v", err)
	}
}

fails with error: Get "https://mobile.yangkeduo.com/": remote error: tls: internal error
and works if //TLSClientConfig: &tls.Config{}, is uncommented.

The actual error comes from here

go/src/crypto/tls/conn.go

Lines 712 to 713 in aa5b225

case alertLevelError:
return c.in.setErrorLocked(&net.OpError{Op: "remote error", Err: alert(data[1])})

Looks like server returns alert record with internal error code:

Screenshot from 2023-02-10 19-04-43

recordTypeAlert recordType = 21

alertInternalError alert = 80

@dr2chase
Copy link
Contributor

@golang/security @neild
Not 100% sure if this is "crypto" or some http/s handshake problem.

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 10, 2023
@zhaozuowu
Copy link
Author

By default http.Client follows redirects so it tries to query "long" link after it receives 302 and gets remote error: tls: internal error.

You may disable redirect following like so:

	client := &http.Client{
		Timeout:   3 * time.Second,
		Transport: DefaultTransports,
		CheckRedirect: func(*http.Request, []*http.Request) error {
			return http.ErrUseLastResponse
		},
	}

Then the part to get Location header value looks weird, it should be just (after redirects are disabled):

	defer resp.Body.Close()

	return resp.Header.Get("Location"), nil

I am not sure what is going on here but this client:

package main

import (
	"log"
	"net/http"
)

func main() {
	client := &http.Client{
		Transport: &http.Transport{
			//TLSClientConfig: &tls.Config{},
		},
	}

	_, err := client.Get("https://mobile.yangkeduo.com/")
	if err != nil {
		log.Fatalf("error: %v", err)
	}
}

fails with error: Get "https://mobile.yangkeduo.com/": remote error: tls: internal error and works if //TLSClientConfig: &tls.Config{}, is uncommented.

The actual error comes from here

go/src/crypto/tls/conn.go

Lines 712 to 713 in aa5b225

case alertLevelError:
return c.in.setErrorLocked(&net.OpError{Op: "remote error", Err: alert(data[1])})

Looks like server returns alert record with internal error code:

Screenshot from 2023-02-10 19-04-43

recordTypeAlert recordType = 21

alertInternalError alert = 80

But I want to know why this kind of error, will lead to Ctrip number of millions of instant surge, resulting in memory leakage

@AlexanderYastrebov
Copy link
Contributor

But I want to know why this kind of error

The difference between transport with nil tls.Config and empty tls.Config{} is that empty tls config disables ALPN so the problem is that mobile.yangkeduo.com does not like default set of TLS extensions default go client uses.

mohammed90 added a commit to caddyserver/caddy that referenced this issue Oct 24, 2023
mohammed90 added a commit to caddyserver/caddy that referenced this issue Oct 24, 2023
mohammed90 added a commit to caddyserver/caddy that referenced this issue Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants