From 8147adc1b0201513ac82b60b4630e432f4085be9 Mon Sep 17 00:00:00 2001 From: Waldemar Quevedo Date: Wed, 12 Jun 2019 11:29:55 -0700 Subject: [PATCH] Add support to extend leafnodes remote tls timeout Bump default TLS timeout for leafnode connections Add checks for when cert_file or key_file are missing in TLS config Signed-off-by: Waldemar Quevedo --- server/config_check_test.go | 23 ++++++- server/const.go | 3 + server/leafnode_test.go | 125 ++++++++++++++++++++++++++++++++++++ server/opts.go | 40 +++++++----- server/opts_test.go | 51 ++++++++++++++- 5 files changed, 222 insertions(+), 20 deletions(-) diff --git a/server/config_check_test.go b/server/config_check_test.go index 982bf71a..23124dce 100644 --- a/server/config_check_test.go +++ b/server/config_check_test.go @@ -1083,9 +1083,9 @@ func TestConfigCheck(t *testing.T) { } } `, - err: errors.New(`error parsing X509 certificate/key pair: open : no such file or directory`), - errorLine: 3, - errorPos: 5, + err: nil, + errorLine: 0, + errorPos: 0, }, { name: "invalid lame_duck_duration type", @@ -1096,6 +1096,23 @@ func TestConfigCheck(t *testing.T) { errorLine: 2, errorPos: 3, }, + { + name: "when only setting TLS timeout for a leafnode remote", + config: ` + leafnodes { + remotes = [ + { + url: "tls://connect.ngs.global:7422" + tls { + timeout: 0.01 + } + } + ] + }`, + err: nil, + errorLine: 0, + errorPos: 0, + }, } checkConfig := func(config string) error { diff --git a/server/const.go b/server/const.go index 33b10c3c..3b6e1886 100644 --- a/server/const.go +++ b/server/const.go @@ -115,6 +115,9 @@ const ( // DEFAULT_LEAF_NODE_RECONNECT LeafNode reconnect interval. DEFAULT_LEAF_NODE_RECONNECT = time.Second + // DEFAULT_LEAF_TLS_TIMEOUT TLS timeout for LeafNodes + DEFAULT_LEAF_TLS_TIMEOUT = 2 * time.Second + // PROTO_SNIPPET_SIZE is the default size of proto to print on parse errors. PROTO_SNIPPET_SIZE = 32 diff --git a/server/leafnode_test.go b/server/leafnode_test.go index 41600cec..4019e7c1 100644 --- a/server/leafnode_test.go +++ b/server/leafnode_test.go @@ -136,3 +136,128 @@ func TestLeafNodeTLSWithCerts(t *testing.T) { return nil }) } + +func TestLeafNodeTLSRemoteWithNoCerts(t *testing.T) { + conf1 := createConfFile(t, []byte(` + port: -1 + leaf { + listen: "127.0.0.1:-1" + tls { + ca_file: "../test/configs/certs/tlsauth/ca.pem" + cert_file: "../test/configs/certs/tlsauth/server.pem" + key_file: "../test/configs/certs/tlsauth/server-key.pem" + timeout: 2 + } + } + `)) + defer os.Remove(conf1) + s1, o1 := RunServerWithConfig(conf1) + defer s1.Shutdown() + + u, err := url.Parse(fmt.Sprintf("nats://localhost:%d", o1.LeafNode.Port)) + if err != nil { + t.Fatalf("Error parsing url: %v", err) + } + conf2 := createConfFile(t, []byte(fmt.Sprintf(` + port: -1 + leaf { + remotes [ + { + url: "%s" + tls { + ca_file: "../test/configs/certs/tlsauth/ca.pem" + timeout: 5 + } + } + ] + } + `, u.String()))) + defer os.Remove(conf2) + o2, err := ProcessConfigFile(conf2) + if err != nil { + t.Fatalf("Error processing config file: %v", err) + } + + if len(o2.LeafNode.Remotes) == 0 { + t.Fatal("Expected at least a single leaf remote") + } + + var ( + got float64 = o2.LeafNode.Remotes[0].TLSTimeout + expected float64 = 5 + ) + if got != expected { + t.Fatalf("Expected %v, got: %v", expected, got) + } + o2.NoLog, o2.NoSigs = true, true + o2.LeafNode.resolver = &testLoopbackResolver{} + s2 := RunServer(o2) + defer s2.Shutdown() + + checkFor(t, 3*time.Second, 10*time.Millisecond, func() error { + if nln := s1.NumLeafNodes(); nln != 1 { + return fmt.Errorf("Number of leaf nodes is %d", nln) + } + return nil + }) + + // Here we only process options without starting the server + // and without a root CA for the remote. + conf3 := createConfFile(t, []byte(fmt.Sprintf(` + port: -1 + leaf { + remotes [ + { + url: "%s" + tls { + timeout: 10 + } + } + ] + } + `, u.String()))) + defer os.Remove(conf3) + o3, err := ProcessConfigFile(conf3) + if err != nil { + t.Fatalf("Error processing config file: %v", err) + } + + if len(o3.LeafNode.Remotes) == 0 { + t.Fatal("Expected at least a single leaf remote") + } + got = o3.LeafNode.Remotes[0].TLSTimeout + expected = 10 + if got != expected { + t.Fatalf("Expected %v, got: %v", expected, got) + } + + // Here we only process options without starting the server + // and check the default for leafnode remotes. + conf4 := createConfFile(t, []byte(fmt.Sprintf(` + port: -1 + leaf { + remotes [ + { + url: "%s" + tls { + ca_file: "../test/configs/certs/tlsauth/ca.pem" + } + } + ] + } + `, u.String()))) + defer os.Remove(conf4) + o4, err := ProcessConfigFile(conf4) + if err != nil { + t.Fatalf("Error processing config file: %v", err) + } + + if len(o4.LeafNode.Remotes) == 0 { + t.Fatal("Expected at least a single leaf remote") + } + got = o4.LeafNode.Remotes[0].TLSTimeout + expected = float64(DEFAULT_LEAF_TLS_TIMEOUT) + if int(got) != int(expected) { + t.Fatalf("Expected %v, got: %v", expected, got) + } +} diff --git a/server/opts.go b/server/opts.go index b2dced5e..2934824a 100644 --- a/server/opts.go +++ b/server/opts.go @@ -1096,7 +1096,11 @@ func parseRemoteLeafNodes(v interface{}, errors *[]error, warnings *[]error) ([] // Set RootCAs since this tls.Config is used when soliciting // a connection (therefore behaves as a client). remote.TLSConfig.RootCAs = remote.TLSConfig.ClientCAs - remote.TLSTimeout = tc.Timeout + if tc.Timeout > 0 { + remote.TLSTimeout = tc.Timeout + } else { + remote.TLSTimeout = float64(DEFAULT_LEAF_TLS_TIMEOUT) + } default: if !tk.IsUsedVariable() { err := &unknownConfigFieldErr{ @@ -2144,29 +2148,35 @@ func parseTLS(v interface{}) (*TLSConfigOpts, error) { // GenTLSConfig loads TLS related configuration parameters. func GenTLSConfig(tc *TLSConfigOpts) (*tls.Config, error) { - - // Now load in cert and private key - cert, err := tls.LoadX509KeyPair(tc.CertFile, tc.KeyFile) - if err != nil { - return nil, fmt.Errorf("error parsing X509 certificate/key pair: %v", err) - } - cert.Leaf, err = x509.ParseCertificate(cert.Certificate[0]) - if err != nil { - return nil, fmt.Errorf("error parsing certificate: %v", err) - } - - // Create the tls.Config from our options. - // We will determine the cipher suites that we prefer. + // Create the tls.Config from our options before including the certs. + // It will determine the cipher suites that we prefer. // FIXME(dlc) change if ARM based. config := tls.Config{ MinVersion: tls.VersionTLS12, CipherSuites: tc.Ciphers, PreferServerCipherSuites: true, CurvePreferences: tc.CurvePreferences, - Certificates: []tls.Certificate{cert}, InsecureSkipVerify: tc.Insecure, } + switch { + case tc.CertFile != "" && tc.KeyFile == "": + return nil, fmt.Errorf("missing 'key_file' in TLS configuration") + case tc.CertFile == "" && tc.KeyFile != "": + return nil, fmt.Errorf("missing 'cert_file' in TLS configuration") + case tc.CertFile != "" && tc.KeyFile != "": + // Now load in cert and private key + cert, err := tls.LoadX509KeyPair(tc.CertFile, tc.KeyFile) + if err != nil { + return nil, fmt.Errorf("error parsing X509 certificate/key pair: %v", err) + } + cert.Leaf, err = x509.ParseCertificate(cert.Certificate[0]) + if err != nil { + return nil, fmt.Errorf("error parsing certificate: %v", err) + } + config.Certificates = []tls.Certificate{cert} + } + // Require client certificates as needed if tc.Verify { config.ClientAuth = tls.RequireAndVerifyClientCert diff --git a/server/opts_test.go b/server/opts_test.go index 5986e889..5698e961 100644 --- a/server/opts_test.go +++ b/server/opts_test.go @@ -1614,12 +1614,59 @@ func TestParsingGatewaysErrors(t *testing.T) { "to be filename", }, { - "tls_gen_error", + "tls_gen_error_cert_file_not_found", + `gateway { + name: "A" + port: -1 + tls { + cert_file: "./configs/certs/missing.pem" + key_file: "./configs/certs/server-key.pem" + } + }`, + "certificate/key pair", + }, + { + "tls_gen_error_key_file_not_found", `gateway { name: "A" port: -1 tls { cert_file: "./configs/certs/server.pem" + key_file: "./configs/certs/missing.pem" + } + }`, + "certificate/key pair", + }, + { + "tls_gen_error_key_file_missing", + `gateway { + name: "A" + port: -1 + tls { + cert_file: "./configs/certs/server.pem" + } + }`, + `missing 'key_file' in TLS configuration`, + }, + { + "tls_gen_error_cert_file_missing", + `gateway { + name: "A" + port: -1 + tls { + key_file: "./configs/certs/server-key.pem" + } + }`, + `missing 'cert_file' in TLS configuration`, + }, + { + "tls_gen_error_key_file_not_found", + `gateway { + name: "A" + port: -1 + tls { + cert_file: "./configs/certs/server.pem" + key_file: "./configs/certs/missing.pem" } }`, "certificate/key pair", @@ -1687,7 +1734,7 @@ func TestParsingGatewaysErrors(t *testing.T) { "to be filename", }, { - "gateway_unknon_field", + "gateway_unknown_field", `gateway { name: "A" port: -1