From 08b6cfb082bcb68001d77c28a2219b364117c074 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 11 Jan 2017 13:59:51 -0800 Subject: [PATCH] ssl: Set a global boolean to enable SSL across Minio (#3558) We have been using `isSSL()` everywhere we can set a global value once and re-use it again. --- cmd/admin-rpc-client.go | 2 +- cmd/browser-peer-rpc.go | 2 +- cmd/globals.go | 6 +++ cmd/lock-rpc-server.go | 2 +- cmd/main.go | 5 ++- cmd/namespace-lock.go | 2 +- cmd/prepare-storage-msg.go | 6 +-- cmd/s3-peer-client.go | 2 +- cmd/server-main.go | 78 ++++++++++++++++++++++--------------- cmd/server-main_test.go | 60 ++++++++++++++++++++++++---- cmd/server-startup-msg.go | 2 +- cmd/server-startup-utils.go | 15 ++++--- cmd/storage-rpc-client.go | 2 +- cmd/update-main.go | 6 +-- cmd/version-main.go | 5 +-- 15 files changed, 130 insertions(+), 65 deletions(-) diff --git a/cmd/admin-rpc-client.go b/cmd/admin-rpc-client.go index 3598b1151..b80ce882f 100644 --- a/cmd/admin-rpc-client.go +++ b/cmd/admin-rpc-client.go @@ -125,7 +125,7 @@ func makeAdminPeers(eps []*url.URL) adminPeers { accessKey: serverCred.AccessKey, secretKey: serverCred.SecretKey, serverAddr: ep.Host, - secureConn: isSSL(), + secureConn: globalIsSSL, serviceEndpoint: path.Join(reservedBucket, adminPath), serviceName: "Admin", } diff --git a/cmd/browser-peer-rpc.go b/cmd/browser-peer-rpc.go index 3af174d06..834a9c75d 100644 --- a/cmd/browser-peer-rpc.go +++ b/cmd/browser-peer-rpc.go @@ -106,7 +106,7 @@ func updateCredsOnPeers(creds credential) map[string]error { accessKey: serverCred.AccessKey, secretKey: serverCred.SecretKey, serverAddr: peers[ix], - secureConn: isSSL(), + secureConn: globalIsSSL, serviceEndpoint: path.Join(reservedBucket, browserPeerPath), serviceName: "Browser", }) diff --git a/cmd/globals.go b/cmd/globals.go index 9f31944eb..307a02198 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -90,6 +90,9 @@ var ( // CA root certificates, a nil value means system certs pool will be used globalRootCAs *x509.CertPool + // IsSSL indicates if the server is configured with SSL. + globalIsSSL bool + // List of admin peers. globalAdminPeers = adminPeers{} @@ -124,4 +127,7 @@ func setGlobalsFromContext(c *cli.Context) { } // Set global quiet flag. globalQuiet = c.Bool("quiet") || c.GlobalBool("quiet") + + // Is TLS configured?. + globalIsSSL = isSSL() } diff --git a/cmd/lock-rpc-server.go b/cmd/lock-rpc-server.go index af2ffdf55..c1761dbd7 100644 --- a/cmd/lock-rpc-server.go +++ b/cmd/lock-rpc-server.go @@ -289,7 +289,7 @@ func (l *lockServer) lockMaintenance(interval time.Duration) { secretKey: serverCred.SecretKey, serverAddr: nlrip.lri.node, serviceEndpoint: nlrip.lri.rpcPath, - secureConn: isSSL(), + secureConn: globalIsSSL, serviceName: "Dsync", }) diff --git a/cmd/main.go b/cmd/main.go index 073f3e922..02f672613 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -164,7 +164,10 @@ func checkUpdate() { } // Generic Minio initialization to create/load config, prepare loggers, etc.. -func minioInit() { +func minioInit(ctx *cli.Context) { + // Set global variables after parsing passed arguments + setGlobalsFromContext(ctx) + // Sets new config directory. setGlobalConfigPath(globalConfigDir) diff --git a/cmd/namespace-lock.go b/cmd/namespace-lock.go index cdb4f0f5c..316b3fba4 100644 --- a/cmd/namespace-lock.go +++ b/cmd/namespace-lock.go @@ -44,7 +44,7 @@ func initDsyncNodes(eps []*url.URL) error { secretKey: cred.SecretKey, serverAddr: ep.Host, serviceEndpoint: pathutil.Join(lockRPCPath, getPath(ep)), - secureConn: isSSL(), + secureConn: globalIsSSL, serviceName: "Dsync", }) if isLocalStorage(ep) && myNode == -1 { diff --git a/cmd/prepare-storage-msg.go b/cmd/prepare-storage-msg.go index d4d38700a..9cc699b9f 100644 --- a/cmd/prepare-storage-msg.go +++ b/cmd/prepare-storage-msg.go @@ -99,11 +99,7 @@ func getHealEndpoint(tls bool, firstEndpoint *url.URL) (cEndpoint *url.URL) { func getHealMsg(endpoints []*url.URL, storageDisks []StorageAPI) string { msg := fmt.Sprintln("\nData volume requires HEALING. Healing is not implemented yet stay tuned:") // FIXME: Enable this after we bring in healing. - // msg += "MINIO_ACCESS_KEY=%s " - // msg += "MINIO_SECRET_KEY=%s " - // msg += "minio control heal %s" - // creds := serverConfig.GetCredential() - // msg = fmt.Sprintf(msg, creds.AccessKey, creds.SecretKey, getHealEndpoint(isSSL(), endpoints[0])) + // msg := "mc admin heal myminio" disksInfo, _, _ := getDisksInfo(storageDisks) for i, info := range disksInfo { if storageDisks[i] == nil { diff --git a/cmd/s3-peer-client.go b/cmd/s3-peer-client.go index 0f0cba687..6358f5881 100644 --- a/cmd/s3-peer-client.go +++ b/cmd/s3-peer-client.go @@ -67,7 +67,7 @@ func makeS3Peers(eps []*url.URL) s3Peers { secretKey: serverCred.SecretKey, serverAddr: ep.Host, serviceEndpoint: path.Join(reservedBucket, s3Path), - secureConn: isSSL(), + secureConn: globalIsSSL, serviceName: "S3", } diff --git a/cmd/server-main.go b/cmd/server-main.go index 36436ead2..b2a6b6d61 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -17,12 +17,14 @@ package cmd import ( + "errors" "fmt" "net" "net/url" "os" "path" "sort" + "strconv" "strings" "runtime" @@ -296,11 +298,10 @@ func checkServerSyntax(c *cli.Context) { } } - tls := isSSL() for _, ep := range endpoints { - if ep.Scheme == "https" && !tls { + if ep.Scheme == "https" && !globalIsSSL { // Certificates should be provided for https configuration. - fatalIf(errInvalidArgument, "Certificates not provided for https") + fatalIf(errInvalidArgument, "Certificates not provided for secure configuration") } } } @@ -317,17 +318,47 @@ func isAnyEndpointLocal(eps []*url.URL) bool { return anyLocalEp } +// Returned when there are no ports. +var errEmptyPort = errors.New("Port cannot be empty or '0', please use `--address` to pick a specific port") + +// Convert an input address of form host:port into, host and port, returns if any. +func getHostPort(address string) (host, port string, err error) { + // Check if requested port is available. + host, port, err = net.SplitHostPort(address) + if err != nil { + return "", "", err + } + + // Empty ports. + if port == "0" || port == "" { + // Port zero or empty means use requested to choose any freely available + // port. Avoid this since it won't work with any configured clients, + // can lead to serious loss of availability. + return "", "", errEmptyPort + } + + // Parse port. + if _, err = strconv.Atoi(port); err != nil { + return "", "", err + } + + // Check if port is available. + if err = checkPortAvailability(port); err != nil { + return "", "", err + } + + // Success. + return host, port, nil +} + // serverMain handler called for 'minio server' command. func serverMain(c *cli.Context) { if !c.Args().Present() || c.Args().First() == "help" { cli.ShowCommandHelpAndExit(c, "server", 1) } - // Set global variables after parsing passed arguments - setGlobalsFromContext(c) - // Initialization routine, such as config loading, enable logging, .. - minioInit() + minioInit(c) // Check for minio updates from dl.minio.io checkUpdate() @@ -335,20 +366,9 @@ func serverMain(c *cli.Context) { // Server address. serverAddr := c.String("address") - // Check if requested port is available. - host, portStr, err := net.SplitHostPort(serverAddr) - fatalIf(err, "Unable to parse %s.", serverAddr) - if portStr == "0" || portStr == "" { - // Port zero or empty means use requested to choose any freely available - // port. Avoid this since it won't work with any configured clients, - // can lead to serious loss of availability. - fatalIf(errInvalidArgument, "Invalid port `%s`, please use `--address` to pick a specific port", portStr) - } - globalMinioHost = host - - // Check if requested port is available. - fatalIf(checkPortAvailability(portStr), "Port unavailable %s", portStr) - globalMinioPort = portStr + var err error + globalMinioHost, globalMinioPort, err = getHostPort(serverAddr) + fatalIf(err, "Unable to extract host and port %s", serverAddr) // Check server syntax and exit in case of errors. // Done after globalMinioHost and globalMinioPort is set as parseStorageEndpoints() @@ -365,9 +385,6 @@ func serverMain(c *cli.Context) { fatalIf(errInvalidArgument, "None of the disks passed as command line args are local to this server.") } - // Is TLS configured?. - tls := isSSL() - // Sort endpoints for consistent ordering across multiple // nodes in a distributed setup. This is to avoid format.json // corruption if the disks aren't supplied in the same order @@ -415,7 +432,8 @@ func serverMain(c *cli.Context) { globalMinioAddr = getLocalAddress(srvConfig) // Determine API endpoints where we are going to serve the S3 API from. - apiEndPoints := finalizeAPIEndpoints(tls, apiServer.Server) + apiEndPoints, err := finalizeAPIEndpoints(apiServer.Server) + fatalIf(err, "Unable to finalize API endpoints for %s", apiServer.Server.Addr) // Set the global API endpoints value. globalAPIEndpoints = apiEndPoints @@ -427,15 +445,13 @@ func serverMain(c *cli.Context) { initGlobalAdminPeers(endpoints) // Start server, automatically configures TLS if certs are available. - go func(tls bool) { - var lerr error + go func() { cert, key := "", "" - if tls { + if globalIsSSL { cert, key = mustGetCertFile(), mustGetKeyFile() } - lerr = apiServer.ListenAndServe(cert, key) - fatalIf(lerr, "Failed to start minio server.") - }(tls) + fatalIf(apiServer.ListenAndServe(cert, key), "Failed to start minio server.") + }() // Wait for formatting of disks. formattedDisks, err := waitForFormatDisks(firstDisk, endpoints, storageDisks) diff --git a/cmd/server-main_test.go b/cmd/server-main_test.go index 3c64d6880..0586c8113 100644 --- a/cmd/server-main_test.go +++ b/cmd/server-main_test.go @@ -63,20 +63,66 @@ func TestGetListenIPs(t *testing.T) { } } +// Tests get host port. +func TestGetHostPort(t *testing.T) { + testCases := []struct { + addr string + err error + }{ + // Test 1 - successful. + { + addr: ":" + getFreePort(), + err: nil, + }, + // Test 2 port empty. + { + addr: ":0", + err: errEmptyPort, + }, + // Test 3 port empty. + { + addr: ":", + err: errEmptyPort, + }, + // Test 4 invalid port. + { + addr: "linux:linux", + err: errors.New("strconv.ParseInt: parsing \"linux\": invalid syntax"), + }, + // Test 5 port not present. + { + addr: "hostname", + err: errors.New("missing port in address hostname"), + }, + } + + // Validate all tests. + for i, testCase := range testCases { + _, _, err := getHostPort(testCase.addr) + if err != nil { + if err.Error() != testCase.err.Error() { + t.Fatalf("Test %d: Error: %s", i+1, err) + } + } + } +} + +// Tests finalize api endpoints. func TestFinalizeAPIEndpoints(t *testing.T) { testCases := []struct { - tls bool addr string }{ - {false, ":80"}, - {true, ":80"}, - {false, "localhost:80"}, - {true, "localhost:80"}, + {":80"}, + {":80"}, + {"localhost:80"}, + {"localhost:80"}, } for i, test := range testCases { - endPoints := finalizeAPIEndpoints(test.tls, &http.Server{Addr: test.addr}) - if len(endPoints) <= 0 { + endPoints, err := finalizeAPIEndpoints(&http.Server{ + Addr: test.addr, + }) + if err != nil && len(endPoints) <= 0 { t.Errorf("Test case %d returned with no API end points for %s", i+1, test.addr) } diff --git a/cmd/server-startup-msg.go b/cmd/server-startup-msg.go index af2fbae25..0bc8b74f2 100644 --- a/cmd/server-startup-msg.go +++ b/cmd/server-startup-msg.go @@ -67,7 +67,7 @@ func printStartupMessage(apiEndPoints []string) { // SSL is configured reads certification chain, prints // authority and expiry. - if isSSL() { + if globalIsSSL { certs, err := readCertificateChain() fatalIf(err, "Unable to read certificate chain.") printCertificateMsg(certs) diff --git a/cmd/server-startup-utils.go b/cmd/server-startup-utils.go index fa884f934..1bb4fce70 100644 --- a/cmd/server-startup-utils.go +++ b/cmd/server-startup-utils.go @@ -40,22 +40,27 @@ func getListenIPs(serverAddr string) (hosts []string, port string, err error) { } return hosts, port, nil } // if host != "" { + // Proceed to append itself, since user requested a specific endpoint. hosts = append(hosts, host) + + // Success. return hosts, port, nil } // Finalizes the API endpoints based on the host list and port. -func finalizeAPIEndpoints(tls bool, apiServer *http.Server) (endPoints []string) { +func finalizeAPIEndpoints(apiServer *http.Server) (endPoints []string, err error) { // Verify current scheme. scheme := "http" - if tls { + if globalIsSSL { scheme = "https" } // Get list of listen ips and port. - hosts, port, err := getListenIPs(apiServer.Addr) - fatalIf(err, "Unable to get list of ips to listen on") + hosts, port, err1 := getListenIPs(apiServer.Addr) + if err1 != nil { + return nil, err1 + } // Construct proper endpoints. for _, host := range hosts { @@ -63,5 +68,5 @@ func finalizeAPIEndpoints(tls bool, apiServer *http.Server) (endPoints []string) } // Success. - return endPoints + return endPoints, nil } diff --git a/cmd/storage-rpc-client.go b/cmd/storage-rpc-client.go index 846b17096..acb2b1376 100644 --- a/cmd/storage-rpc-client.go +++ b/cmd/storage-rpc-client.go @@ -121,7 +121,7 @@ func newStorageRPC(ep *url.URL) (StorageAPI, error) { secretKey: secretKey, serverAddr: rpcAddr, serviceEndpoint: rpcPath, - secureConn: isSSL(), + secureConn: globalIsSSL, serviceName: "Storage", disableReconnect: true, }), diff --git a/cmd/update-main.go b/cmd/update-main.go index e56d9682d..1cc39dd6b 100644 --- a/cmd/update-main.go +++ b/cmd/update-main.go @@ -265,12 +265,8 @@ func getReleaseUpdate(updateURL string, duration time.Duration) (updateMsg updat // main entry point for update command. func mainUpdate(ctx *cli.Context) { - - // Set global variables after parsing passed arguments - setGlobalsFromContext(ctx) - // Initialization routine, such as config loading, enable logging, .. - minioInit() + minioInit(ctx) if globalQuiet { return diff --git a/cmd/version-main.go b/cmd/version-main.go index 71e34f343..9ed543842 100644 --- a/cmd/version-main.go +++ b/cmd/version-main.go @@ -44,11 +44,8 @@ func mainVersion(ctx *cli.Context) { cli.ShowCommandHelpAndExit(ctx, "version", 1) } - // Set global variables after parsing passed arguments - setGlobalsFromContext(ctx) - // Initialization routine, such as config loading, enable logging, .. - minioInit() + minioInit(ctx) if globalQuiet { return