Skip to content

Commit 716f63b

Browse files
author
Andrew Hare
committed
fix: Misc TLS issues
1 parent 4623e93 commit 716f63b

File tree

6 files changed

+149
-6
lines changed

6 files changed

+149
-6
lines changed

cmd/provider-services/cmd/grpc.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@ package cmd
33
import (
44
"fmt"
55
"net"
6+
"net/url"
67
)
78

89
func grpcURI(hostURI string) (string, error) {
9-
host, _, err := net.SplitHostPort(hostURI)
10+
u, err := url.Parse(hostURI)
11+
if err != nil {
12+
return "", fmt.Errorf("url parse: %w", err)
13+
}
14+
15+
h, _, err := net.SplitHostPort(u.Host)
1016
if err != nil {
1117
return "", fmt.Errorf("split host port: %w", err)
1218
}
1319

14-
return net.JoinHostPort(host, "8442"), nil
20+
return net.JoinHostPort(h, "8444"), nil
1521
}

cmd/provider-services/cmd/manifest.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,12 @@ func doSendManifest(cmd *cobra.Command, sdlpath string) error {
103103
)
104104

105105
for i, lid := range leases {
106-
err := func() error {
106+
err = func() error {
107107
ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
108108
defer cancel()
109109

110110
provAddr, _ := sdk.AccAddressFromBech32(lid.Provider)
111-
prov, err := cl.Provider(context.Background(), &ptypes.QueryProviderRequest{Owner: provAddr.String()})
111+
prov, err := cl.Provider(ctx, &ptypes.QueryProviderRequest{Owner: provAddr.String()})
112112
if err != nil {
113113
return fmt.Errorf("query client provider: %w", err)
114114
}

gateway/grpc/client.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ func NewClient(ctx context.Context, addr string, cert tls.Certificate, cquery ct
3131
tlsConfig := tls.Config{
3232
InsecureSkipVerify: true,
3333
Certificates: []tls.Certificate{cert},
34+
MinVersion: tls.VersionTLS13,
3435
VerifyPeerCertificate: func(certificates [][]byte, _ [][]*x509.Certificate) error {
3536
if _, err := utils.VerifyOwnerCertBytes(ctx, certificates, "", x509.ExtKeyUsageClientAuth, cquery); err != nil {
3637
return err

gateway/grpc/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func mtlsInterceptor(cquery ctypes.QueryClient) grpc.UnaryServerInterceptor {
114114
return func(ctx context.Context, req any, _ *grpc.UnaryServerInfo, next grpc.UnaryHandler) (any, error) {
115115
if p, ok := peer.FromContext(ctx); ok {
116116
if mtls, ok := p.AuthInfo.(credentials.TLSInfo); ok {
117-
owner, err := utils.VerifyOwnerCert(ctx, mtls.State.PeerCertificates, "", x509.ExtKeyUsageServerAuth, cquery)
117+
owner, err := utils.VerifyOwnerCert(ctx, mtls.State.PeerCertificates, "", x509.ExtKeyUsageClientAuth, cquery)
118118
if err != nil {
119119
return nil, fmt.Errorf("verify cert chain: %w", err)
120120
}

gateway/grpc/server_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,18 @@ package grpc
22

33
import (
44
"context"
5+
"crypto/ecdsa"
6+
"crypto/elliptic"
7+
"crypto/rand"
58
"crypto/tls"
9+
"crypto/x509"
10+
"crypto/x509/pkix"
11+
"encoding/pem"
612
"errors"
13+
"math/big"
714
"net"
815
"testing"
16+
"time"
917

1018
"github.com/stretchr/testify/assert"
1119
"github.com/stretchr/testify/mock"
@@ -209,3 +217,131 @@ func TestRPCs(t *testing.T) {
209217
})
210218
}
211219
}
220+
221+
func TestMTLS(t *testing.T) {
222+
var (
223+
qclient = &qmock.QueryClient{}
224+
com = testutil.CertificateOptionMocks(qclient)
225+
cod = testutil.CertificateOptionDomains([]string{"localhost", "127.0.0.1"})
226+
)
227+
228+
crt := testutil.Certificate(t, testutil.AccAddress(t), com, cod)
229+
230+
qclient.EXPECT().Certificates(mock.Anything, mock.Anything).Return(&types.QueryCertificatesResponse{
231+
Certificates: types.CertificatesResponse{
232+
types.CertificateResponse{
233+
Certificate: types.Certificate{
234+
State: types.CertificateValid,
235+
Cert: crt.PEM.Cert,
236+
Pubkey: crt.PEM.Pub,
237+
},
238+
Serial: crt.Serial.String(),
239+
},
240+
},
241+
}, nil)
242+
243+
cases := []struct {
244+
desc string
245+
cert func(*testing.T) tls.Certificate
246+
errContains string
247+
}{
248+
{
249+
desc: "good cert",
250+
cert: func(*testing.T) tls.Certificate {
251+
return testutil.Certificate(t, testutil.AccAddress(t), com, cod).Cert[0]
252+
},
253+
},
254+
{
255+
desc: "empty chain",
256+
cert: func(*testing.T) tls.Certificate {
257+
return tls.Certificate{}
258+
},
259+
errContains: "empty chain",
260+
},
261+
{
262+
desc: "invalid subject",
263+
cert: func(t *testing.T) tls.Certificate {
264+
t.Helper()
265+
266+
priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
267+
require.NoError(t, err)
268+
269+
template := x509.Certificate{
270+
SerialNumber: new(big.Int).SetInt64(time.Now().UTC().UnixNano()),
271+
Subject: pkix.Name{
272+
CommonName: "badcert",
273+
},
274+
BasicConstraintsValid: true,
275+
}
276+
277+
certDer, err := x509.CreateCertificate(rand.Reader, &template, &template, priv.Public(), priv)
278+
require.NoError(t, err)
279+
280+
keyDer, err := x509.MarshalPKCS8PrivateKey(priv)
281+
require.NoError(t, err)
282+
283+
certBytes := pem.EncodeToMemory(&pem.Block{
284+
Type: types.PemBlkTypeCertificate,
285+
Bytes: certDer,
286+
})
287+
privBytes := pem.EncodeToMemory(&pem.Block{
288+
Type: types.PemBlkTypeECPrivateKey,
289+
Bytes: keyDer,
290+
})
291+
292+
cert, err := tls.X509KeyPair(certBytes, privBytes)
293+
require.NoError(t, err)
294+
295+
return cert
296+
},
297+
errContains: "invalid certificate's subject",
298+
},
299+
}
300+
301+
for _, c := range cases {
302+
c := c
303+
304+
t.Run(c.desc, func(t *testing.T) {
305+
ctx, cancel := context.WithCancel(context.Background())
306+
defer cancel()
307+
308+
ctx = ContextWithQueryClient(ctx, qclient)
309+
310+
var (
311+
m mocks.Client
312+
mc mmocks.Client
313+
)
314+
315+
mc.EXPECT().Submit(mock.Anything, mock.Anything, mock.Anything).Return(nil)
316+
m.EXPECT().Manifest().Return(&mc)
317+
318+
s := newServer(ctx, crt.Cert, &m)
319+
defer s.Stop()
320+
321+
l, err := net.Listen("tcp", ":0")
322+
require.NoError(t, err)
323+
324+
go func() {
325+
require.NoError(t, s.Serve(l))
326+
}()
327+
328+
tlsConfig := tls.Config{
329+
InsecureSkipVerify: true,
330+
Certificates: []tls.Certificate{c.cert(t)},
331+
}
332+
333+
conn, err := grpc.DialContext(ctx, l.Addr().String(),
334+
grpc.WithTransportCredentials(credentials.NewTLS(&tlsConfig)))
335+
require.NoError(t, err)
336+
337+
defer conn.Close()
338+
339+
_, err = leasev1.NewLeaseRPCClient(conn).SendManifest(ctx, &leasev1.SendManifestRequest{})
340+
if c.errContains != "" {
341+
assert.ErrorContains(t, err, c.errContains)
342+
} else {
343+
assert.NoError(t, err)
344+
}
345+
})
346+
}
347+
}

gateway/utils/utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func VerifyOwnerCertBytes(ctx context.Context, chain [][]byte, dnsName string, u
5252

5353
func VerifyOwnerCert(ctx context.Context, chain []*x509.Certificate, dnsName string, usage x509.ExtKeyUsage, cquery ctypes.QueryClient) (sdk.Address, error) {
5454
if len(chain) == 0 {
55-
return nil, nil
55+
return nil, errors.Errorf("tls: empty chain")
5656
}
5757

5858
if len(chain) > 1 {

0 commit comments

Comments
 (0)