address PR code review feedback

This commit is contained in:
Will Xia
2020-08-30 08:18:26 -04:00
parent 7ca9034c47
commit 3542fb8e85
4 changed files with 162 additions and 55 deletions
+8 -10
View File
@@ -46,8 +46,8 @@ type Config struct {
type TLSConfig struct {
SkipVerify bool
CA string
MtlsCliCrt string
MtlsCliKey string
Cert string
Key string
}
//Client represents a client instance
@@ -120,15 +120,13 @@ func NewClient(c *Config) (*Client, error) {
}
}
//Specify client cert and key pair
if c.TLS.MtlsCliCrt != "" && c.TLS.MtlsCliKey != "" {
tc.GetClientCertificate = func(info *tls.CertificateRequestInfo) (*tls.Certificate, error) {
c, err := tls.LoadX509KeyPair(c.TLS.MtlsCliCrt, c.TLS.MtlsCliKey)
if err != nil {
return nil, fmt.Errorf("Error loading client cert and key pair: %v", err)
}
return &c, nil
if c.TLS.Cert != "" && c.TLS.Key != "" {
c, err := tls.LoadX509KeyPair(c.TLS.Cert, c.TLS.Key)
if err != nil {
return nil, fmt.Errorf("Error loading client cert and key pair: %v", err)
}
} else if c.TLS.MtlsCliCrt != "" || c.TLS.MtlsCliKey != "" {
tc.Certificates = []tls.Certificate{c}
} else if c.TLS.Cert != "" || c.TLS.Key != "" {
return nil, fmt.Errorf("Please specify client cert and key pair BOTH")
}
client.tlsConfig = tc
+12 -11
View File
@@ -158,10 +158,10 @@ var serverHelp = `
or disable caching by setting this variable to "-". You can optionally
provide a certificate notification email by setting CHISEL_LE_EMAIL.
--tls-mtls-ca-dir, for mTLS support only. Specify the path to load
all self signed client certificates and trusted CAs in PEM-encoded
format. For trusted CAs, any client certificates signed by these CAs
consider to be allowed.
--tls-ca, a path to a PEM encoded CA certificate bundle or a directory
holding multiple PEM encode CA certificate bundle files, which is used to
validate client connections. The provided CA certificates will be used
instead of the system roots. This is commonly used to implement mutual-TLS.
` + commonHelp
func server(args []string) {
@@ -180,7 +180,7 @@ func server(args []string) {
flags.StringVar(&config.TLS.Key, "tls-key", "", "")
flags.StringVar(&config.TLS.Cert, "tls-cert", "", "")
flags.Var(multiFlag{&config.TLS.Domains}, "tls-domain", "")
flags.StringVar(&config.TLS.MtlsCaDir, "tls-mtls-ca-dir", "", "")
flags.StringVar(&config.TLS.CA, "tls-ca", "", "")
host := flags.String("host", "", "")
p := flags.String("p", "", "")
@@ -374,11 +374,12 @@ var clientHelp = `
may be still verified (see --fingerprint) after inner connection
is established.
--tls-mtls-clicrt, for mTLS support only. PEM-encoded client certificate
for mTLS authentication.
--tls-key, a path to a PEM encoded private key used for client
authentication (mutual-TLS).
--tls-mtls-clikey, for mTLS support only. PEM-encoded client key for
mTLS authentication.
--tls-cert, a path to a PEM encoded certificate matching the provided
private key. The certificate must have client authentication
enabled (mutual-TLS).
` + commonHelp
func client(args []string) {
@@ -392,8 +393,8 @@ func client(args []string) {
flags.StringVar(&config.Proxy, "proxy", "", "")
flags.StringVar(&config.TLS.CA, "tls-ca", "", "")
flags.BoolVar(&config.TLS.SkipVerify, "tls-skip-verify", false, "")
flags.StringVar(&config.TLS.MtlsCliCrt, "tls-mtls-clicrt", "", "")
flags.StringVar(&config.TLS.MtlsCliKey, "tls-mtls-clikey", "", "")
flags.StringVar(&config.TLS.Cert, "tls-cert", "", "")
flags.StringVar(&config.TLS.Key, "tls-key", "", "")
flags.Var(&headerFlags{config.Headers}, "header", "")
hostname := flags.String("hostname", "", "")
pid := flags.Bool("pid", false, "")
+41 -28
View File
@@ -4,21 +4,20 @@ import (
"crypto/tls"
"crypto/x509"
"errors"
"golang.org/x/crypto/acme/autocert"
"io/ioutil"
"net"
"os"
"os/user"
"path/filepath"
"golang.org/x/crypto/acme/autocert"
)
//TLSConfig enables configures TLS
type TLSConfig struct {
Key string
Cert string
Domains []string
MtlsCaDir string
Key string
Cert string
Domains []string
CA string
}
func (s *Server) listener(host, port string) (net.Listener, error) {
@@ -33,7 +32,7 @@ func (s *Server) listener(host, port string) (net.Listener, error) {
}
extra := ""
if hasKeyCert {
c, err := s.tlsKeyCert(s.config.TLS.Key, s.config.TLS.Cert, s.config.TLS.MtlsCaDir)
c, err := s.tlsKeyCert(s.config.TLS.Key, s.config.TLS.Cert, s.config.TLS.CA)
if err != nil {
return nil, err
}
@@ -90,36 +89,50 @@ func (s *Server) tlsKeyCert(key, cert string, clientCaPath string) (*tls.Config,
if err != nil {
return nil, err
}
//file based tls config using tls defaults
tlsCfg := &tls.Config{
Certificates: []tls.Certificate{c},
}
//mTLS requires Client CAs
if clientCaPath != "" {
files, err := ioutil.ReadDir(clientCaPath)
fileInfo, err := os.Stat(clientCaPath)
if err != nil {
return nil, err
}
s.Infof("Looking for client CA files from %s", clientCaPath)
clientCAPool := x509.NewCertPool()
//add all cert files from path
for _, file := range files {
f := file.Name()
content, err := ioutil.ReadFile(filepath.Join(clientCaPath, f))
if err == nil {
if clientCAPool.AppendCertsFromPEM(content) {
s.Infof("Add client CA file %s", f)
} else {
s.Errorf("Fail to add client CA file %s", f)
if fileInfo.IsDir() {
//this is a directory holding CA bundle files
files, err := ioutil.ReadDir(clientCaPath)
if err != nil {
return nil, err
}
s.Infof("Looking for client CA files from %s", clientCaPath)
//add all cert files from path
for _, file := range files {
f := file.Name()
if err := addCABundle(filepath.Join(clientCaPath, f), clientCAPool); err != nil {
return nil, err
}
}
} else {
//this is a CA bundle file
if err := addCABundle(clientCaPath, clientCAPool); err != nil {
return nil, err
}
}
//return file based tls config with client CAs and cert verification enabled
return &tls.Config{
Certificates: []tls.Certificate{c},
ClientCAs: clientCAPool,
ClientAuth: tls.RequireAndVerifyClientCert,
}, nil
//set client CAs and enable cert verification
tlsCfg.ClientCAs = clientCAPool
tlsCfg.ClientAuth = tls.RequireAndVerifyClientCert
}
return tlsCfg, nil
}
func addCABundle(clientCaPath string, clientCAPool *x509.CertPool) error {
if content, err := ioutil.ReadFile(clientCaPath); err != nil {
return nil
} else if !clientCAPool.AppendCertsFromPEM(content) {
return errors.New("Fail to load certificates from : " + clientCaPath)
} else {
//return file based tls config using tls defaults
return &tls.Config{
Certificates: []tls.Certificate{c},
}, nil
return err
}
}
+101 -6
View File
@@ -13,9 +13,9 @@ func TestTls(t *testing.T) {
teardown := simpleSetup(t,
&chserver.Config{
TLS: chserver.TLSConfig{
Cert: "tls/server-crt/server.crt",
Key: "tls/server-crt/server.key",
MtlsCaDir: "tls/server-ca",
Cert: "tls/server-crt/server.crt",
Key: "tls/server-crt/server.key",
CA: "tls/server-ca/client.crt",
},
},
&chclient.Config{
@@ -23,9 +23,9 @@ func TestTls(t *testing.T) {
TLS: chclient.TLSConfig{
SkipVerify: false,
//for self signed cert, it needs the server cert, for real cert, this need to be the trusted CA cert
CA: "tls/client-ca/server.crt",
MtlsCliCrt: "tls/client-crt/client.crt",
MtlsCliKey: "tls/client-crt/client.key",
CA: "tls/client-ca/server.crt",
Cert: "tls/client-crt/client.crt",
Key: "tls/client-crt/client.key",
},
Server: "https://localhost:" + tmpPort,
})
@@ -39,3 +39,98 @@ func TestTls(t *testing.T) {
t.Fatalf("expected exclamation mark added")
}
}
func TestTlsMitiCAs(t *testing.T) {
tmpPort := availablePort()
//setup server, client, fileserver
teardown := simpleSetup(t,
&chserver.Config{
TLS: chserver.TLSConfig{
Cert: "tls/server-crt/server.crt",
Key: "tls/server-crt/server.key",
CA: "tls/server-ca",
},
},
&chclient.Config{
Remotes: []string{tmpPort + ":$FILEPORT"},
TLS: chclient.TLSConfig{
SkipVerify: false,
//for self signed cert, it needs the server cert, for real cert, this need to be the trusted CA cert
CA: "tls/client-ca/server.crt",
Cert: "tls/client-crt/client.crt",
Key: "tls/client-crt/client.key",
},
Server: "https://localhost:" + tmpPort,
})
defer teardown()
//test remote
result, err := post("http://localhost:"+tmpPort, "foo")
if err != nil {
t.Fatal(err)
}
if result != "foo!" {
t.Fatalf("expected exclamation mark added")
}
}
func TestTlsMissingClientCert(t *testing.T) {
tmpPort := availablePort()
//setup server, client, fileserver
teardown := simpleSetup(t,
&chserver.Config{
TLS: chserver.TLSConfig{
Cert: "tls/server-crt/server.crt",
Key: "tls/server-crt/server.key",
CA: "tls/server-ca/client.crt",
},
},
&chclient.Config{
Remotes: []string{tmpPort + ":$FILEPORT"},
TLS: chclient.TLSConfig{
SkipVerify: false,
CA: "tls/client-ca/server.crt",
//provide no client cert, server should reject the client request
//Cert: "tls/client-crt/client.crt",
//Key: "tls/client-crt/client.key",
},
Server: "https://localhost:" + tmpPort,
})
defer teardown()
//test remote
_, err := post("http://localhost:"+tmpPort, "foo")
if err == nil {
t.Fatal(err)
}
}
func TestTlsMissingClientCA(t *testing.T) {
tmpPort := availablePort()
//setup server, client, fileserver
teardown := simpleSetup(t,
&chserver.Config{
TLS: chserver.TLSConfig{
Cert: "tls/server-crt/server.crt",
Key: "tls/server-crt/server.key",
//specify a CA which does not match the client cert
//server should reject the client request
CA: "tls/server-crt/server.crt",
},
},
&chclient.Config{
Remotes: []string{tmpPort + ":$FILEPORT"},
TLS: chclient.TLSConfig{
SkipVerify: false,
//for self signed cert, it needs the server cert, for real cert, this need to be the trusted CA cert
CA: "tls/client-ca/server.crt",
Cert: "tls/client-crt/client.crt",
Key: "tls/client-crt/client.key",
},
Server: "https://localhost:" + tmpPort,
})
defer teardown()
//test remote
_, err := post("http://localhost:"+tmpPort, "foo")
if err == nil {
t.Fatal(err)
}
}