JakeHillion/storj
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[V3-1147] Ensure certificate validation happens properly (#1403)

Browse Source 
* add regression test & update transport tests

* separate client and server verificiation functions

* goimports
This commit is contained in:
Bryan White 2019-03-06 15:42:34 +01:00 committed by Dennis Coyle
parent ead4a0ae95
commit c607abf27c
4 changed files with 127 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
pkg/peertls/tlsopts/options.go
View File

@ -26,15 +26,23 @@ type Options struct {
Config Config
Ident *identity.FullIdentity
RevDB *identity.RevocationDB
VerificationFuncs []peertls.PeerCertVerificationFunc
VerificationFuncs *VerificationFuncs
Cert *tls.Certificate
}
// VerificationFuncs keeps track of of client and server peer certificate verification
// functions for use in tls handshakes.
type VerificationFuncs struct {
client []peertls.PeerCertVerificationFunc
server []peertls.PeerCertVerificationFunc
}
// NewOptions is a constructor for `tls options` given an identity and config
func NewOptions(i *identity.FullIdentity, c Config) (*Options, error) {
opts := &Options{
Config: c,
Ident: i,
Config: c,
Ident: i,
VerificationFuncs: new(VerificationFuncs),
}
err := opts.configure(c)
@ -48,7 +56,6 @@ func NewOptions(i *identity.FullIdentity, c Config) (*Options, error) {
// configure adds peer certificate verification functions and revocation
// database to the config.
func (opts *Options) configure(c Config) (err error) {
var pcvs []peertls.PeerCertVerificationFunc
parseOpts := peertls.ParseExtOptions{}
if c.UsePeerCAWhitelist {
@ -64,7 +71,7 @@ func (opts *Options) configure(c Config) (err error) {
return Error.Wrap(err)
}
parseOpts.CAWhitelist = parsed
pcvs = append(pcvs, peertls.VerifyCAWhitelist(parsed))
opts.VerificationFuncs.ClientAdd(peertls.VerifyCAWhitelist(parsed))
}
if c.Extensions.Revocation {
@ -72,22 +79,50 @@ func (opts *Options) configure(c Config) (err error) {
if err != nil {
return err
}
pcvs = append(pcvs, identity.VerifyUnrevokedChainFunc(opts.RevDB))
opts.VerificationFuncs.Add(identity.VerifyUnrevokedChainFunc(opts.RevDB))
}
exts := peertls.ParseExtensions(c.Extensions, parseOpts)
pcvs = append(pcvs, exts.VerifyFunc())
// NB: remove nil elements
for i, f := range pcvs {
if f == nil {
copy(pcvs[i:], pcvs[i+1:])
pcvs = pcvs[:len(pcvs)-1]
}
}
opts.VerificationFuncs = pcvs
opts.VerificationFuncs.Add(exts.VerifyFunc())
opts.Cert, err = peertls.TLSCert(opts.Ident.RawChain(), opts.Ident.Leaf, opts.Ident.Key)
return err
}
// Client returns the client verification functions.
func (vf *VerificationFuncs) Client() []peertls.PeerCertVerificationFunc {
return vf.client
}
// Server returns the server verification functions.
func (vf *VerificationFuncs) Server() []peertls.PeerCertVerificationFunc {
return vf.server
}
// Add adds verification functions so the client and server lists.
func (vf *VerificationFuncs) Add(verificationFuncs ...peertls.PeerCertVerificationFunc) {
vf.ClientAdd(verificationFuncs...)
vf.ServerAdd(verificationFuncs...)
}
// ClientAdd adds verification functions so the client list.
func (vf *VerificationFuncs) ClientAdd(verificationFuncs ...peertls.PeerCertVerificationFunc) {
verificationFuncs = removeNils(verificationFuncs)
vf.client = append(vf.client, verificationFuncs...)
}
// ServerAdd adds verification functions so the server list.
func (vf *VerificationFuncs) ServerAdd(verificationFuncs ...peertls.PeerCertVerificationFunc) {
verificationFuncs = removeNils(verificationFuncs)
vf.server = append(vf.server, verificationFuncs...)
}
func removeNils(verificationFuncs []peertls.PeerCertVerificationFunc) []peertls.PeerCertVerificationFunc {
for i, f := range verificationFuncs {
if f == nil {
copy(verificationFuncs[i:], verificationFuncs[i+1:])
verificationFuncs = verificationFuncs[:len(verificationFuncs)-1]
}
}
return verificationFuncs
}

64
pkg/peertls/tlsopts/options_test.go
View File

@ -13,9 +13,11 @@ import (
"storj.io/storj/internal/testcontext"
"storj.io/storj/internal/testplanet"
"storj.io/storj/pkg/identity"
"storj.io/storj/pkg/peertls"
"storj.io/storj/pkg/peertls/tlsopts"
"storj.io/storj/pkg/storj"
"storj.io/storj/pkg/transport"
)
func TestNewOptions(t *testing.T) {
@ -34,14 +36,15 @@ func TestNewOptions(t *testing.T) {
assert.NoError(t, err)
cases := []struct {
testID string
config tlsopts.Config
pcvFuncsLen int
testID string
config tlsopts.Config
clientVerificationFuncsLen int
serverVerificationFuncsLen int
}{
{
"default",
tlsopts.Config{},
0,
0, 0,
}, {
"revocation processing",
tlsopts.Config{
@ -50,14 +53,14 @@ func TestNewOptions(t *testing.T) {
Revocation: true,
},
},
2,
2, 2,
}, {
"ca whitelist verification",
tlsopts.Config{
PeerCAWhitelistPath: whitelistPath,
UsePeerCAWhitelist: true,
},
1,
1, 0,
}, {
"ca whitelist verification and whitelist signed leaf verification",
tlsopts.Config{
@ -68,7 +71,7 @@ func TestNewOptions(t *testing.T) {
WhitelistSignedLeaf: true,
},
},
2,
2, 1,
}, {
"revocation processing and whitelist verification",
tlsopts.Config{
@ -80,7 +83,7 @@ func TestNewOptions(t *testing.T) {
Revocation: true,
},
},
3,
3, 2,
}, {
"revocation processing, whitelist, and signed leaf verification",
tlsopts.Config{
@ -93,7 +96,7 @@ func TestNewOptions(t *testing.T) {
WhitelistSignedLeaf: true,
},
},
3,
3, 2,
},
}
@ -103,7 +106,48 @@ func TestNewOptions(t *testing.T) {
assert.NoError(t, err)
assert.True(t, reflect.DeepEqual(fi, opts.Ident))
assert.Equal(t, c.config, opts.Config)
assert.Len(t, opts.VerificationFuncs, c.pcvFuncsLen)
assert.Len(t, opts.VerificationFuncs.Client(), c.clientVerificationFuncsLen)
assert.Len(t, opts.VerificationFuncs.Server(), c.serverVerificationFuncsLen)
}
}
type identFunc func(int) (*identity.FullIdentity, error)
func TestOptions_ServerOption_Peer_CA_Whitelist(t *testing.T) {
ctx := testcontext.New(t)
planet, err := testplanet.New(t, 0, 2, 0)
require.NoError(t, err)
planet.Start(ctx)
defer ctx.Check(planet.Shutdown)
target := planet.StorageNodes[1].Local()
testCases := []struct {
name string
identF identFunc
}{
{"unsigned client identity", testplanet.PregeneratedIdentity},
{"signed client identity", testplanet.PregeneratedSignedIdentity},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
ident, err := testCase.identF(0)
require.NoError(t, err)
opts, err := tlsopts.NewOptions(ident, tlsopts.Config{})
require.NoError(t, err)
dialOption, err := opts.DialOption(target.Id)
require.NoError(t, err)
transportClient := transport.NewClient(opts)
conn, err := transportClient.DialNode(ctx, &target, dialOption)
assert.NotNil(t, conn)
assert.NoError(t, err)
})
}
}

17
pkg/peertls/tlsopts/tls.go
View File

@ -55,10 +55,19 @@ func (opts *Options) tlsConfig(isServer bool, verificationFuncs ...peertls.PeerC
},
verificationFuncs...,
)
verificationFuncs = append(
verificationFuncs,
opts.VerificationFuncs...,
)
switch isServer {
case true:
verificationFuncs = append(
verificationFuncs,
opts.VerificationFuncs.server...,
)
case false:
verificationFuncs = append(
verificationFuncs,
opts.VerificationFuncs.client...,
)
}
config := &tls.Config{
Certificates: []tls.Certificate{*opts.Cert},

17
pkg/transport/transport_test.go
View File

@ -138,7 +138,7 @@ func TestDialNode(t *testing.T) {
assert.NoError(t, conn.Close())
})
t.Run("DialNode with bad client certificate", func(t *testing.T) {
t.Run("DialNode with unsigned identity", func(t *testing.T) {
target := &pb.Node{
Id: planet.StorageNodes[1].ID(),
Address: &pb.NodeAddress{
@ -157,13 +157,12 @@ func TestDialNode(t *testing.T) {
)
cancel()
tag := fmt.Sprintf("%+v", target)
assert.Nil(t, conn, tag)
require.Error(t, err)
assert.Contains(t, err.Error(), "bad certificate")
assert.NotNil(t, conn)
require.NoError(t, err)
assert.NoError(t, conn.Close())
})
t.Run("DialAddress with bad client certificate", func(t *testing.T) {
t.Run("DialAddress with unsigned identity", func(t *testing.T) {
timedCtx, cancel := context.WithTimeout(ctx, time.Second)
dialOption := unsignedClientOpts.DialUnverifiedIDOption()
conn, err := client.DialAddress(
@ -171,9 +170,9 @@ func TestDialNode(t *testing.T) {
)
cancel()
assert.Nil(t, conn)
require.Error(t, err)
assert.Contains(t, err.Error(), "bad certificate")
assert.NotNil(t, conn)
require.NoError(t, err)
assert.NoError(t, conn.Close())
})
t.Run("DialAddress with valid address", func(t *testing.T) {