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 error wrapping for errors in HTTP2 server preface #6639

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions clientconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@ package grpc

import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"math"
"net"
"net/http"
"net/http/httptest"
"os"
"strings"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -254,6 +259,88 @@ func (s) TestDialWaitsForServerSettingsAndFails(t *testing.T) {
}
}

func (s) TestDialFailFastInServerPrefaceWhenIncapableTLSUsed(t *testing.T) {
// we do setup only an http server since we don't expect reach the gRPC server
httpSrv := httptest.NewUnstartedServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
}))

httpSrv.EnableHTTP2 = true
t.Cleanup(func() {
httpSrv.Close()
})

// configuring server TLS and add client's CA certificate
cert, err := tls.LoadX509KeyPair(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem"))
if err != nil {
t.Fatalf("tls.LoadX509KeyPair(x509/server1_cert.pem, x509/server1_key.pem) failed: %v", err)
}
ca, err := os.ReadFile(testdata.Path("x509/client_ca_cert.pem"))
if err != nil {
t.Fatalf("os.ReadFile(x509/client_ca_cert.pem) failed: %v", err)
}
certPool := x509.NewCertPool()
if !certPool.AppendCertsFromPEM(ca) {
t.Fatal("failed to append clients' CA certificates")
}

httpSrv.TLS = &tls.Config{
ClientAuth: tls.RequireAndVerifyClientCert,
Certificates: []tls.Certificate{cert},
ClientCAs: certPool,
}
httpSrv.StartTLS()

// configuring client TLS and add server's CA certificate
// an important part that client_bad.crt is the valid certificate,
// but it's limited to use only for email protection
cert, err = tls.LoadX509KeyPair(testdata.Path("x509/client_bad.crt"), testdata.Path("x509/client_bad_key.pem"))
if err != nil {
t.Fatalf("tls.LoadX509KeyPair(x509/client_bad.crt, x509/client_bad_key.pem) failed: %v", err)
}
ca, err = os.ReadFile(testdata.Path("x509/server_ca_cert.pem"))
if err != nil {
t.Fatalf("os.ReadFile(x509/server_ca_cert.pem) failed: %v", err)
}
roots := x509.NewCertPool()
if !roots.AppendCertsFromPEM(ca) {
t.Fatal("failed to append servers' CA certificates")
}
creds := credentials.NewTLS(&tls.Config{
Certificates: []tls.Certificate{cert},
RootCAs: roots,
ServerName: "x.test.example.com",
})

// we use such a big timeout, but we expect to fail fast
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

addr := strings.TrimSpace(httpSrv.Listener.Addr().String())

client, err := DialContext(
ctx,
addr,
WithTransportCredentials(creds),
WithBlock(),
FailOnNonTempDialError(true),
)
if err == nil {
client.Close()
t.Fatalf("Unexpected success (err=nil) while dialing")
}

expectedErrReadingServerPrefaceMsg := "error reading server preface"
expectedBadTLSMsg := "tls: bad certificate"
if !strings.Contains(err.Error(), expectedErrReadingServerPrefaceMsg) || !strings.Contains(err.Error(), expectedBadTLSMsg) {
t.Fatalf("DialContext(_) = %v; want a message that includes both %q and %q", err, expectedErrReadingServerPrefaceMsg, expectedBadTLSMsg)
}

// we don't expect that we reach context deadline (timeout)
if strings.Contains(err.Error(), context.DeadlineExceeded.Error()) {
t.Fatalf("DialContext(_) = %v; want a message that doesn't include %q", err, context.DeadlineExceeded.Error())
}
}

// 1. Client connects to a server that doesn't send preface.
// 2. After minConnectTimeout(500 ms here), client disconnects and retries.
// 3. The new server sends its preface.
Expand Down
2 changes: 1 addition & 1 deletion internal/transport/http2_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1563,7 +1563,7 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
func (t *http2Client) readServerPreface() error {
frame, err := t.framer.fr.ReadFrame()
if err != nil {
return connectionErrorf(true, err, "error reading server preface: %v", err)
return connectionErrorf(isTemporary(err), err, "error reading server preface: %v", err)
}
sf, ok := frame.(*http2.SettingsFrame)
if !ok {
Expand Down
34 changes: 34 additions & 0 deletions testdata/x509/client_bad.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
-----BEGIN CERTIFICATE-----
MIIF1zCCA7+gAwIBAgICA+gwDQYJKoZIhvcNAQELBQAwUDELMAkGA1UEBhMCVVMx
CzAJBgNVBAgMAkNBMQwwCgYDVQQHDANTVkwxDTALBgNVBAoMBGdSUEMxFzAVBgNV
BAMMDnRlc3QtY2xpZW50X2NhMB4XDTIzMDkxNTA3MDYzN1oXDTMzMDkxMjA3MDYz
N1owUTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQwwCgYDVQQHDANTVkwxDTAL
BgNVBAoMBGdSUEMxGDAWBgNVBAMMD3Rlc3QtY2xpZW50X2JhZDCCAiIwDQYJKoZI
hvcNAQEBBQADggIPADCCAgoCggIBAJd6g5IEDYE99Je+ojxE+glkpEsGI5H/EIjF
//moh/PQcih00dk0vacxmtUSpw0LCq0RVI3HDdmz4elknsvDX+uubkrZw46lpfhN
4Vg2FsYkKEMkSh9Bs5y2aBOH773c4+1rKjE+/vCI/fPun36Co+o9biQRni7458pL
WsgQSMmnh3lj3Zn8g7pJx8i11ITmR4L8lobhSVV/DETq9VirEWUWDNZiWKyLBn0C
mntLGucYuSWxwhueynfJVAeajd56pa7VaKjCpJCpBh4q1pmJhXY8Vcw0ms3mKeby
bkf8IpSaKqzqjAbds+TZY7cVXjO6b4EQGRuDYHJaZQ+cgwhpRichh4xHn/ssGdyq
bu5uxrfDG2JSDpblQm5wAQjB9iwNBRj82rbZ/zH2eBO6/2B3bAO+iyu+0zzXCwdX
yESwePlukes5QDBsMi0Zmga0a6MYJml2fPX9F311B5xB7q2PIUFupDANN1K5XmJS
sCjjFxUxw7JdFKONNUwSug/5IhBBf6DL+MSpBGeFxk0SsdkblNXbz+E76c+XhowR
o19RVVgYYPc4Zkoms+loriynokXfQbLiygnW+Ve9QCi3MCwDJICnB//ogZhWVYOm
OZX1kwgnGuQBBoclswCp7JJp9i11LjBbdEuJKm3/b3MUTynXGGfuCvzvUwDgtfpc
/fu9a+rFAgMBAAGjgbkwgbYwEQYJYIZIAYb4QgEBBAQDAgUgMCsGCWCGSAGG+EIB
DQQeFhxMb2NhbCBUZXN0IEVtYWlsIENlcnRpZmljYXRlMAwGA1UdEwEB/wQCMAAw
HQYDVR0OBBYEFNwUrb6qtoqww4ZH3G0Kr5ABW7XOMA4GA1UdDwEB/wQEAwIF4DAW
BgNVHSUBAf8EDDAKBggrBgEFBQcDBDAfBgNVHSMEGDAWgBTq92tDG5TfVvTqbu1b
A593K6aAwjANBgkqhkiG9w0BAQsFAAOCAgEABeTp3FoVp/z8o+Vgq8QxyRpqPoWc
72JnAKDkvG29KbXnVlAQr5HcoTLCsXBlS2/GaXJ0HiKqgWs4BfUaGuC4QMsulZkN
vU8ufYhKcxtq52cenVkyMDnL4BBhFeag6MhupeogqRp1dOgqZQAJZ4uS7A6OjjQ6
O/lSHsBBZcjXpmpMYuzsGAgivXEw/OVL+Id0+hyWhb2VyQQSF1ObtUbfJ64BpOry
G5IENTo+6i8zxz6wx3YU5503LRC8jvDVDwchQQHHRQVdQRD9FXPSsJ9boDKccuUy
0Nks/LIST0nK72PpRpTknayp9+o9oiSXzY0yjrC4fqXLcGqyUSqBIK6JrBTQKPjC
iwsWSoc++b1o37346DlQIJSR7GFKIAcWQj18CbEhZuG0ENp8qRh8uuFBTntOB2oJ
YXsM/w7MX1VAYWk7r2i29a+mgT7zb1WuoKxmtafOuKw/Z7h+i+U3Ykd+JgRU2C+6
sdZcFt6Kvvc64lkqtL6CHbVevSCsTXLyx6w//pITexpLOM4E2STb42V/VsogmnjE
BQBGHHwg6yvpnuftBzSZJdgSKAmWTqNqPlS+sEj0Wo3FFYknCWz+lXNcxxGWdf3G
KBp/yAD82GkuxHLQdnD4eMMEwASic0dKhfSH2hFd07VsifqWzC1oVTMbv4rC50qC
gU8f3kHw4tAfzwY=
-----END CERTIFICATE-----
52 changes: 52 additions & 0 deletions testdata/x509/client_bad_key.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
-----BEGIN PRIVATE KEY-----
MIIJRAIBADANBgkqhkiG9w0BAQEFAASCCS4wggkqAgEAAoICAQCXeoOSBA2BPfSX
vqI8RPoJZKRLBiOR/xCIxf/5qIfz0HIodNHZNL2nMZrVEqcNCwqtEVSNxw3Zs+Hp
ZJ7Lw1/rrm5K2cOOpaX4TeFYNhbGJChDJEofQbOctmgTh++93OPtayoxPv7wiP3z
7p9+gqPqPW4kEZ4u+OfKS1rIEEjJp4d5Y92Z/IO6ScfItdSE5keC/JaG4UlVfwxE
6vVYqxFlFgzWYlisiwZ9App7SxrnGLklscIbnsp3yVQHmo3eeqWu1WiowqSQqQYe
KtaZiYV2PFXMNJrN5inm8m5H/CKUmiqs6owG3bPk2WO3FV4zum+BEBkbg2ByWmUP
nIMIaUYnIYeMR5/7LBncqm7ubsa3wxtiUg6W5UJucAEIwfYsDQUY/Nq22f8x9ngT
uv9gd2wDvosrvtM81wsHV8hEsHj5bpHrOUAwbDItGZoGtGujGCZpdnz1/Rd9dQec
Qe6tjyFBbqQwDTdSuV5iUrAo4xcVMcOyXRSjjTVMEroP+SIQQX+gy/jEqQRnhcZN
ErHZG5TV28/hO+nPl4aMEaNfUVVYGGD3OGZKJrPpaK4sp6JF30Gy4soJ1vlXvUAo
tzAsAySApwf/6IGYVlWDpjmV9ZMIJxrkAQaHJbMAqeySafYtdS4wW3RLiSpt/29z
FE8p1xhn7gr871MA4LX6XP37vWvqxQIDAQABAoICAAMRMP7Q5DEcvwofiwtrXiC4
j/cQ7cF0B7KtR6yXdAFE1DrreqBKSBjNkVQFJ6rE8XdlpW+leytQUB+4sNpxX0zX
YFZVqHcH9Z9cfFab6cQjUUliv7l4TBm9O8hBdWJDyb2x8M3Ed8cJDxNUXphiLs18
ZvGGBczvr0nSUslAQykKIl0b343Rph0sh0YSIHZ8bzBAyw96GlEH3Ii8tTERba29
T487uI3t1rjLR7N9ZtMNFhqkScjjYzgftHre70Z090vd+FuKbWicc4pC1cuQFEq7
mAUlf4jxwyFxATil61FU+c48hY9SB+V3XWSEoz6q5L0ZRVjgRSCroeY6BhEyiIAU
XKYnkiQuzZwn+SgZLjsr5ShBO/D9wP0awM4wwO23Q5VgrOPmHLYm5R/s+Hncthhy
tzOONT2f8XCyTJHTLjLBsBVb6+bE7nZSrCJRK3fzm4er8+xl2FCqAYj+FT//1+Pt
oa8dzLZfJGMBTHhMHlTjpjGLFFWH05I9IpeEinmE03je6UA2cgvNoCTbOijVEhJ6
gOfGu88X8JnOzcKEFP5fIUrYB2iVA8zZzVbTjo9MOGPUDumZLAbMRv00Ix7scjDV
kGUc2lrM6R8Jq3BcmPSwiBHmRnCWpdcq7zld8LUMJz6OgQbVWOwE2M5PsODxYlYV
RM7HpwboUUbRmtTNjR8VAoIBAQDAcN2L1vcvMKQLYBkO9xeuwitbog8hqWnVBRIB
ctgCCb4YY0LR2pVVeAvKgAorYhOUtCyuNxzFAIJuqFONSrZ6wuJFjG4g9TyoYXtH
lDPHkL4bRTRPIBWpyOoOpilFxxSYnO38r2w16SjaUNV64PFS6i5cT/Z+z3Skt9KU
1DOkVs9iIjOFOQWjT6q+/1bHvnjwwjugSvA/YDz2yQlysiOFFIRzVa17CcLAqKKi
SAJ37t8yUnFGjVw89CSmy71VFxGmGsHWYmn8cahOsD1muxXIRtG5C2ocBW6noMC7
vqGKSd4UGRCpDFfHipOaJ4AUOd2g4d7yvi4OJidTlyPdTPCDAoIBAQDJgjrw+Rlp
kBoHLCSZCysl1yj/Qp++lfVqMZsCRkaxy/d9lyx1mgwEfdjZdn7F+FocZz7XWl+B
Jfuav9h+CjLE9f65+E3N/Z0TkXspXLdzaNvgy0UZq0xWffkTBPrpfnzLr4edMy1X
hQPmb5vUglcwoVcANpJnHqSOc4x2PzBIFccda3l4Pe07yzs87hvYMkczUqgveXri
JNB5Rbpo3DiaTvYVdlqcix4NRB2fObBAOMYZRU+gHtJZ8ppSTp7+FzfgePnsabax
GnZLSnL7AiPbHOkfmI86Nc4Mo1hN7/oZhXZnEzUEDBMtH9/1xxUqN/aaiofPFyCA
WAuVhsnpZUUXAoIBAQCVxcHHmjCbW6Hw8IPQL+MQsjIIiSJ5sl6Z2e5fjkArpeV7
GZXhudtLv7h9jBVeGmDe9TNpC5+qe3EIKp2Mc79937s3Icp8gCELc4L8/I0oHrC2
jK/ffTse/y2a5RtuITkYZdqbNPRlKOgQsaTUk8l+HKCkW9+eZluD70VHa7gdiSqy
V6f9YZnPtqtPJ28k2ktKJgE+CdTdnksUTva5e4dFhehn0yDHh5s5UoXxA1ZYfWLc
yWk73b0R+3mjDyJ13RRxUJXYApQ2U0cIpwXCtIyJaQTEneuhj2DFuckG1aDn/gGH
ZtKenuqadHHYIjH+i+K/2cso5Xb2sB8fYZwaGk19AoIBAQCcTQmO9IjDmuS1f4X7
t/AF7/h61qC14fd0IXoMr5oSOxWDMwlnRs/fhAX517HJuTOcKHFT9WNy95VLmVdB
nIN4xx6H/ZPMHdYC4atYj/Qz7rfui3zTxkz2Icxa9lfxWQ1PqJ1WT+Xxptk5moSS
t28N53dmUO4KUnhtji77YVP/9fR2W3Di1ZX39VHDlyW98nHL22ddO6sXUlrqVi2b
PIYjGvBCAYWTO01aGOWpJcQSbHcsDtkOaBndbxJImXc1u6i50tx4hh8RDGeJSvwY
urY5NRmWSm1+R1HF98V83pFtzlni5cEPnJYbQEgligBeuP3tLVreRSAOrTx/BQgE
o2qNAoIBAQCxeCNySZmYZ+PJO6b0PpojXfP8j46lh4QpuVGTOEYDBjRHBxS49H0T
uLj1gBTosUbPFiiJFGX1xu760k64G4HYnV97A+z8x7ZjaIouH1dqfRY32qe2oRmW
I0jUcoMosOIq+AIxCGIQsoMxI6XHPbeRT1Lf6NOGIJP//qzzkQcs7TYRROlmciqX
QnZtx5pd1yIwY9iWE28Ujn3NfBMrFq0iGfoNeYT9f1i9e1nqtZtIERWV2CbhBiPa
NEA7lR+bj14fl3bQv2sJ/05998k4dVWmLS32eIjFg0bTVPng9N3X9A23iOaiPHeH
Ey8ITAkWVbTMSgxgCyEyiSYSSb6ZWaVS
-----END PRIVATE KEY-----
25 changes: 25 additions & 0 deletions testdata/x509/create.sh
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,31 @@ openssl x509 -req \
-sha256
openssl verify -verbose -CAfile client_ca_cert.pem client2_cert.pem

# create a client certificate that is valid only for the email protection
# and can't be used for client authentication.
openssl genrsa -out client_bad_key.pem 4096
openssl req -new \
-key client_bad_key.pem \
-out client_bad.csr \
-subj /C=US/ST=CA/L=SVL/O=gRPC/CN=test-client_bad/ \
-config ./openssl.cnf \
-reqexts test_email_client

openssl x509 -req \
-in client_bad.csr \
-CAkey client_ca_key.pem \
-CA client_ca_cert.pem \
-days 3650 \
-set_serial 1000 \
-out client_bad.crt \
-extfile ./openssl.cnf \
-extensions test_email_client \
-sha256

openssl verify -verbose -CAfile client_ca_cert.pem client_bad.crt
# cleanup the CSRs
rm client_bad.csr

# Generate a cert with SPIFFE ID.
openssl req -x509 \
-newkey rsa:4096 \
Expand Down
8 changes: 8 additions & 0 deletions testdata/x509/openssl.cnf
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,11 @@ basicConstraints = critical,CA:FALSE
subjectKeyIdentifier = hash
keyUsage = critical,nonRepudiation,digitalSignature,keyEncipherment
extendedKeyUsage = critical,clientAuth

[test_email_client]
nsCertType = email
nsComment = "Local Test Email Certificate"
basicConstraints = critical,CA:FALSE
subjectKeyIdentifier = hash
keyUsage = critical,nonRepudiation,digitalSignature,keyEncipherment
extendedKeyUsage = critical,emailProtection