From 734ac435e024de61171501d57d6d2b366582faa9 Mon Sep 17 00:00:00 2001 From: ti-srebot <66930949+ti-srebot@users.noreply.github.com> Date: Tue, 27 Oct 2020 09:45:03 +0800 Subject: [PATCH] cherry pick #1217 to release-2.0 (#1220) --- dm/master/server_test.go | 180 ++++++++++++++++++++++++++++++++- dm/master/tls_for_test/ca.pem | 8 ++ dm/master/tls_for_test/dm.key | 8 ++ dm/master/tls_for_test/dm.pem | 10 ++ pkg/utils/util.go | 7 +- pkg/utils/util_test.go | 16 ++- tests/tls/conf/dm-master1.toml | 2 +- tests/tls/conf/dm-master2.toml | 4 +- tests/tls/conf/dm-master3.toml | 4 +- 9 files changed, 218 insertions(+), 21 deletions(-) create mode 100644 dm/master/tls_for_test/ca.pem create mode 100644 dm/master/tls_for_test/dm.key create mode 100644 dm/master/tls_for_test/dm.pem diff --git a/dm/master/server_test.go b/dm/master/server_test.go index bef2b7926f..213d75e1a9 100644 --- a/dm/master/server_test.go +++ b/dm/master/server_test.go @@ -19,7 +19,6 @@ import ( "database/sql" "fmt" "io/ioutil" - "net/http" "os" "sort" "strings" @@ -34,6 +33,7 @@ import ( "github.com/pingcap/parser" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" + toolutils "github.com/pingcap/tidb-tools/pkg/utils" tiddl "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/sessionctx" tidbmock "github.com/pingcap/tidb/util/mock" @@ -967,8 +967,184 @@ func (t *testMaster) TestServer(c *check.C) { }), check.IsTrue) } +func (t *testMaster) TestMasterTLS(c *check.C) { + masterAddr := tempurl.Alloc()[len("http://"):] + peerAddr := tempurl.Alloc()[len("http://"):] + + // all with `https://` prefix + cfg := NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=https://%s", masterAddr), + fmt.Sprintf("--advertise-addr=https://%s", masterAddr), + fmt.Sprintf("--peer-urls=https://%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + t.testTLSPrefix(c, cfg) + c.Assert(cfg.MasterAddr, check.Equals, masterAddr) + c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr) + c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr) + + // no `https://` prefix for `--master-addr` + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=%s", masterAddr), + fmt.Sprintf("--advertise-addr=https://%s", masterAddr), + fmt.Sprintf("--peer-urls=https://%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + t.testTLSPrefix(c, cfg) + + // no `https://` prefix for `--master-addr` and `--advertise-addr` + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=%s", masterAddr), + fmt.Sprintf("--advertise-addr=%s", masterAddr), + fmt.Sprintf("--peer-urls=https://%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + t.testTLSPrefix(c, cfg) + + // no `https://` prefix for `--master-addr`, `--advertise-addr` and `--peer-urls` + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=%s", masterAddr), + fmt.Sprintf("--advertise-addr=%s", masterAddr), + fmt.Sprintf("--peer-urls=%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + t.testTLSPrefix(c, cfg) + + // no `https://` prefix for `--master-addr`, `--advertise-addr`, `--peer-urls` and `--advertise-peer-urls` + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=%s", masterAddr), + fmt.Sprintf("--advertise-addr=%s", masterAddr), + fmt.Sprintf("--peer-urls=%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=https://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + t.testTLSPrefix(c, cfg) + + // all without `https://`/`http://` prefix + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=%s", masterAddr), + fmt.Sprintf("--advertise-addr=%s", masterAddr), + fmt.Sprintf("--peer-urls=%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + t.testTLSPrefix(c, cfg) + c.Assert(cfg.MasterAddr, check.Equals, masterAddr) + c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr) + c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr) + + // all with `http://` prefix, but with TLS enabled. + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=http://%s", masterAddr), + fmt.Sprintf("--advertise-addr=http://%s", masterAddr), + fmt.Sprintf("--peer-urls=http://%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=http://%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=http://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + c.Assert(cfg.MasterAddr, check.Equals, masterAddr) + c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr) + c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr) + + // different prefix for `--peer-urls` and `--initial-cluster` + cfg = NewConfig() + c.Assert(cfg.Parse([]string{ + "--name=master-tls", + fmt.Sprintf("--data-dir=%s", c.MkDir()), + fmt.Sprintf("--master-addr=https://%s", masterAddr), + fmt.Sprintf("--advertise-addr=https://%s", masterAddr), + fmt.Sprintf("--peer-urls=https://%s", peerAddr), + fmt.Sprintf("--advertise-peer-urls=https://%s", peerAddr), + fmt.Sprintf("--initial-cluster=master-tls=http://%s", peerAddr), + "--ssl-ca=./tls_for_test/ca.pem", + "--ssl-cert=./tls_for_test/dm.pem", + "--ssl-key=./tls_for_test/dm.key", + }), check.IsNil) + c.Assert(cfg.MasterAddr, check.Equals, masterAddr) + c.Assert(cfg.AdvertiseAddr, check.Equals, masterAddr) + c.Assert(cfg.PeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.AdvertisePeerUrls, check.Equals, "https://"+peerAddr) + c.Assert(cfg.InitialCluster, check.Equals, "master-tls=https://"+peerAddr) + t.testTLSPrefix(c, cfg) +} + +func (t *testMaster) testTLSPrefix(c *check.C, cfg *Config) { + s := NewServer(cfg) + + ctx, cancel := context.WithCancel(context.Background()) + err1 := s.Start(ctx) + c.Assert(err1, check.IsNil) + + t.testHTTPInterface(c, fmt.Sprintf("https://%s/status", cfg.AdvertiseAddr), []byte(utils.GetRawInfo())) + t.testHTTPInterface(c, fmt.Sprintf("https://%s/debug/pprof/", cfg.AdvertiseAddr), []byte("Types of profiles available")) + + // close + cancel() + s.Close() + + c.Assert(utils.WaitSomething(30, 100*time.Millisecond, func() bool { + return s.closed.Get() + }), check.IsTrue) +} + func (t *testMaster) testHTTPInterface(c *check.C, url string, contain []byte) { - resp, err := http.Get(url) + // we use HTTPS in some test cases. + tls, err := toolutils.NewTLS("./tls_for_test/ca.pem", "./tls_for_test/dm.pem", "./tls_for_test/dm.key", url, []string{}) + c.Assert(err, check.IsNil) + cli := toolutils.ClientWithTLS(tls.TLSConfig()) + + resp, err := cli.Get(url) c.Assert(err, check.IsNil) defer resp.Body.Close() c.Assert(resp.StatusCode, check.Equals, 200) diff --git a/dm/master/tls_for_test/ca.pem b/dm/master/tls_for_test/ca.pem new file mode 100644 index 0000000000..9fc215fa83 --- /dev/null +++ b/dm/master/tls_for_test/ca.pem @@ -0,0 +1,8 @@ +-----BEGIN CERTIFICATE----- +MIIBGDCBwAIJAOjYXLFw5V1HMAoGCCqGSM49BAMCMBQxEjAQBgNVBAMMCWxvY2Fs +aG9zdDAgFw0yMDAzMTcxMjAwMzNaGA8yMjkzMTIzMTEyMDAzM1owFDESMBAGA1UE +AwwJbG9jYWxob3N0MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEglCIJD8uVBfD +kuM+UQP+VA7Srbz17WPLA0Sqc+sQ2p6fT6HYKCW60EXiZ/yEC0925iyVbXEEbX4J +xCc2Heow5TAKBggqhkjOPQQDAgNHADBEAiAILL3Zt/3NFeDW9c9UAcJ9lc92E0ZL +GNDuH6i19Fex3wIgT0ZMAKAFSirGGtcLu0emceuk+zVKjJzmYbsLdpj/JuQ= +-----END CERTIFICATE----- diff --git a/dm/master/tls_for_test/dm.key b/dm/master/tls_for_test/dm.key new file mode 100644 index 0000000000..dfdc077bc4 --- /dev/null +++ b/dm/master/tls_for_test/dm.key @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEICF/GDtVxhTPTP501nOu4jgwGSDY01xN+61xd9MfChw+oAoGCCqGSM49 +AwEHoUQDQgAEgQOv5bQO7xK16vZWhwJqlz2vl19+AXW2Ql7KQyGiBJVSvLbyDLOr +kIeFlHN04iqQ39SKSOSfeGSfRt6doU6IcA== +-----END EC PRIVATE KEY----- diff --git a/dm/master/tls_for_test/dm.pem b/dm/master/tls_for_test/dm.pem new file mode 100644 index 0000000000..d4f846e3a2 --- /dev/null +++ b/dm/master/tls_for_test/dm.pem @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBZDCCAQqgAwIBAgIJAIT/lgXUc1JqMAoGCCqGSM49BAMCMBQxEjAQBgNVBAMM +CWxvY2FsaG9zdDAgFw0yMDAzMTcxMjAwMzNaGA8yMjkzMTIzMTEyMDAzM1owDTEL +MAkGA1UEAwwCZG0wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAASBA6/ltA7vErXq +9laHAmqXPa+XX34BdbZCXspDIaIElVK8tvIMs6uQh4WUc3TiKpDf1IpI5J94ZJ9G +3p2hTohwo0owSDAaBgNVHREEEzARgglsb2NhbGhvc3SHBH8AAAEwCwYDVR0PBAQD +AgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATAKBggqhkjOPQQDAgNI +ADBFAiEAx6ljJ+tNa55ypWLGNqmXlB4UdMmKmE4RSKJ8mmEelfECIG2ZmCE59rv5 +wImM6KnK+vM2QnEiISH3PeYyyRzQzycu +-----END CERTIFICATE----- diff --git a/pkg/utils/util.go b/pkg/utils/util.go index 6ebccce05e..18b7807e93 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -203,17 +203,14 @@ func wrapScheme(s string, https bool) string { if s == "" { return s } - if strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://") { - return s - } + s = UnwrapScheme(s) if https { return "https://" + s } return "http://" + s } -// WrapSchemes adds http or https scheme to input if missing. input could be a comma-separated list -// if input has wrong scheme, don't correct it (maybe user deliberately?) +// WrapSchemes adds http or https scheme to input if missing. input could be a comma-separated list. func WrapSchemes(s string, https bool) string { items := strings.Split(s, ",") output := make([]string, 0, len(items)) diff --git a/pkg/utils/util_test.go b/pkg/utils/util_test.go index c7bfb751f9..825d6f0e87 100644 --- a/pkg/utils/util_test.go +++ b/pkg/utils/util_test.go @@ -246,10 +246,9 @@ func (t *testUtilsSuite) TestWrapSchemes(c *C) { "https://abc.com:123", }, { - // if input has wrong scheme, don't correct it (maybe user deliberately?) "abc.com:123,http://abc.com:123,0.0.0.0:123,https://0.0.0.0:123", - "http://abc.com:123,http://abc.com:123,http://0.0.0.0:123,https://0.0.0.0:123", - "https://abc.com:123,http://abc.com:123,https://0.0.0.0:123,https://0.0.0.0:123", + "http://abc.com:123,http://abc.com:123,http://0.0.0.0:123,http://0.0.0.0:123", + "https://abc.com:123,https://abc.com:123,https://0.0.0.0:123,https://0.0.0.0:123", }, { "", @@ -264,17 +263,16 @@ func (t *testUtilsSuite) TestWrapSchemes(c *C) { } func (t *testUtilsSuite) TestWrapSchemesForInitialCluster(c *C) { - // no change c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293", false), Equals, "master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293") c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293", true), Equals, - "master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293") + "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293") - // add `http` or `https` for some URLs - c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=http://127.0.0.1:8293", false), Equals, + // correct `http` or `https` for some URLs + c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=https://127.0.0.1:8293", false), Equals, "master1=http://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=http://127.0.0.1:8293") - c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=http://127.0.0.1:8293", true), Equals, - "master1=http://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=http://127.0.0.1:8293") + c.Assert(WrapSchemesForInitialCluster("master1=http://127.0.0.1:8291,master2=127.0.0.1:8292,master3=https://127.0.0.1:8293", true), Equals, + "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293") // add `http` or `https` for all URLs c.Assert(WrapSchemesForInitialCluster("master1=127.0.0.1:8291,master2=127.0.0.1:8292,master3=127.0.0.1:8293", false), Equals, diff --git a/tests/tls/conf/dm-master1.toml b/tests/tls/conf/dm-master1.toml index 73b2016072..fdae0252af 100644 --- a/tests/tls/conf/dm-master1.toml +++ b/tests/tls/conf/dm-master1.toml @@ -3,7 +3,7 @@ name = "master1" master-addr = ":8261" advertise-addr = "127.0.0.1:8261" peer-urls = "127.0.0.1:8291" -initial-cluster = "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293" +initial-cluster = "master1=127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=https://127.0.0.1:8293" ssl-ca = "dir-placeholer/ca.pem" ssl-cert = "dir-placeholer/dm.pem" diff --git a/tests/tls/conf/dm-master2.toml b/tests/tls/conf/dm-master2.toml index a3d7e9f2fc..d107e4a4cd 100644 --- a/tests/tls/conf/dm-master2.toml +++ b/tests/tls/conf/dm-master2.toml @@ -2,8 +2,8 @@ name = "master2" master-addr = ":8361" advertise-addr = "127.0.0.1:8361" -peer-urls = "127.0.0.1:8292" -initial-cluster = "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293" +peer-urls = "http://127.0.0.1:8292" +initial-cluster = "master1=https://127.0.0.1:8291,master2=http://127.0.0.1:8292,master3=127.0.0.1:8293" ssl-ca = "dir-placeholer/ca.pem" ssl-cert = "dir-placeholer/dm.pem" diff --git a/tests/tls/conf/dm-master3.toml b/tests/tls/conf/dm-master3.toml index 8bd1bf4eb7..5e85ac017f 100644 --- a/tests/tls/conf/dm-master3.toml +++ b/tests/tls/conf/dm-master3.toml @@ -2,8 +2,8 @@ name = "master3" master-addr = ":8461" advertise-addr = "127.0.0.1:8461" -peer-urls = "127.0.0.1:8293" -initial-cluster = "master1=https://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=https://127.0.0.1:8293" +peer-urls = "https://127.0.0.1:8293" +initial-cluster = "master1=http://127.0.0.1:8291,master2=https://127.0.0.1:8292,master3=127.0.0.1:8293" ssl-ca = "dir-placeholer/ca.pem" ssl-cert = "dir-placeholer/dm.pem"