From 0dd3a081693478ea8ef7900f2616d293a631ae20 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 11 Aug 2020 08:29:50 -0700 Subject: [PATCH] move the certPool loader function into pkg/certs (#10239) --- cmd/config/certs.go | 34 ----------------- cmd/config/certs_test.go | 55 --------------------------- cmd/gateway-main.go | 3 +- cmd/server-main.go | 2 +- pkg/certs/ca-certs.go | 57 ++++++++++++++++++++++++++++ pkg/certs/ca-certs_test.go | 78 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 92 deletions(-) create mode 100644 pkg/certs/ca-certs.go create mode 100644 pkg/certs/ca-certs_test.go diff --git a/cmd/config/certs.go b/cmd/config/certs.go index 119395b66..2809be5a6 100644 --- a/cmd/config/certs.go +++ b/cmd/config/certs.go @@ -24,8 +24,6 @@ import ( "crypto/x509" "encoding/pem" "io/ioutil" - "os" - "path" "github.com/minio/minio/pkg/env" ) @@ -69,38 +67,6 @@ func ParsePublicCertFile(certFile string) (x509Certs []*x509.Certificate, err er return x509Certs, nil } -// GetRootCAs - returns all the root CAs into certPool -// at the input certsCADir -func GetRootCAs(certsCAsDir string) (*x509.CertPool, error) { - rootCAs, _ := x509.SystemCertPool() - if rootCAs == nil { - // In some systems (like Windows) system cert pool is - // not supported or no certificates are present on the - // system - so we create a new cert pool. - rootCAs = x509.NewCertPool() - } - - fis, err := ioutil.ReadDir(certsCAsDir) - if err != nil { - if os.IsNotExist(err) || os.IsPermission(err) { - // Return success if CA's directory is missing or permission denied. - err = nil - } - return rootCAs, err - } - - // Load all custom CA files. - for _, fi := range fis { - caCert, err := ioutil.ReadFile(path.Join(certsCAsDir, fi.Name())) - if err != nil { - // ignore files which are not readable. - continue - } - rootCAs.AppendCertsFromPEM(caCert) - } - return rootCAs, nil -} - // LoadX509KeyPair - load an X509 key pair (private key , certificate) // from the provided paths. The private key may be encrypted and is // decrypted using the ENV_VAR: MINIO_CERT_PASSWD. diff --git a/cmd/config/certs_test.go b/cmd/config/certs_test.go index a4c008119..a65d7dd19 100644 --- a/cmd/config/certs_test.go +++ b/cmd/config/certs_test.go @@ -20,7 +20,6 @@ import ( "fmt" "io/ioutil" "os" - "path/filepath" "runtime" "testing" ) @@ -194,60 +193,6 @@ M9ofSEt/bdRD } } -func TestGetRootCAs(t *testing.T) { - emptydir, err := ioutil.TempDir("", "test-get-root-cas") - if err != nil { - t.Fatalf("Unable create temp directory. %v", emptydir) - } - defer os.RemoveAll(emptydir) - - dir1, err := ioutil.TempDir("", "test-get-root-cas") - if err != nil { - t.Fatalf("Unable create temp directory. %v", dir1) - } - defer os.RemoveAll(dir1) - if err = os.Mkdir(filepath.Join(dir1, "empty-dir"), 0755); err != nil { - t.Fatalf("Unable create empty dir. %v", err) - } - - dir2, err := ioutil.TempDir("", "test-get-root-cas") - if err != nil { - t.Fatalf("Unable create temp directory. %v", dir2) - } - defer os.RemoveAll(dir2) - if err = ioutil.WriteFile(filepath.Join(dir2, "empty-file"), []byte{}, 0644); err != nil { - t.Fatalf("Unable create test file. %v", err) - } - - testCases := []struct { - certCAsDir string - expectedErr error - }{ - // ignores non-existent directories. - {"nonexistent-dir", nil}, - // Ignores directories. - {dir1, nil}, - // Ignore empty directory. - {emptydir, nil}, - // Loads the cert properly. - {dir2, nil}, - } - - for _, testCase := range testCases { - _, err := GetRootCAs(testCase.certCAsDir) - - if testCase.expectedErr == nil { - if err != nil { - t.Fatalf("error: expected = , got = %v", err) - } - } else if err == nil { - t.Fatalf("error: expected = %v, got = ", testCase.expectedErr) - } else if testCase.expectedErr.Error() != err.Error() { - t.Fatalf("error: expected = %v, got = %v", testCase.expectedErr, err) - } - } -} - func TestLoadX509KeyPair(t *testing.T) { for i, testCase := range loadX509KeyPairTests { privateKey, err := createTempFile("private.key", testCase.privateKey) diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index ce4c05753..6874296eb 100644 --- a/cmd/gateway-main.go +++ b/cmd/gateway-main.go @@ -28,7 +28,6 @@ import ( "github.com/gorilla/mux" "github.com/minio/cli" - "github.com/minio/minio/cmd/config" xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/certs" @@ -178,7 +177,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) { logger.FatalIf(err, "Invalid TLS certificate file") // Check and load Root CAs. - globalRootCAs, err = config.GetRootCAs(globalCertsCADir.Get()) + globalRootCAs, err = certs.GetRootCAs(globalCertsCADir.Get()) logger.FatalIf(err, "Failed to read root CAs (%v)", err) // Register root CAs for remote ENVs diff --git a/cmd/server-main.go b/cmd/server-main.go index 8ea8b8a4c..12d4b487f 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -119,7 +119,7 @@ func serverHandleCmdArgs(ctx *cli.Context) { logger.FatalIf(err, "Unable to load the TLS configuration") // Check and load Root CAs. - globalRootCAs, err = config.GetRootCAs(globalCertsCADir.Get()) + globalRootCAs, err = certs.GetRootCAs(globalCertsCADir.Get()) logger.FatalIf(err, "Failed to read root CAs (%v)", err) // Register root CAs for remote ENVs diff --git a/pkg/certs/ca-certs.go b/pkg/certs/ca-certs.go new file mode 100644 index 000000000..52f1f158b --- /dev/null +++ b/pkg/certs/ca-certs.go @@ -0,0 +1,57 @@ +/* + * MinIO Cloud Storage, (C) 2020 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package certs + +import ( + "crypto/x509" + "io/ioutil" + "os" + "path" +) + +// GetRootCAs - returns all the root CAs into certPool +// at the input certsCADir +func GetRootCAs(certsCAsDir string) (*x509.CertPool, error) { + rootCAs, _ := x509.SystemCertPool() + if rootCAs == nil { + // In some systems (like Windows) system cert pool is + // not supported or no certificates are present on the + // system - so we create a new cert pool. + rootCAs = x509.NewCertPool() + } + + fis, err := ioutil.ReadDir(certsCAsDir) + if err != nil { + if os.IsNotExist(err) || os.IsPermission(err) { + // Return success if CA's directory is missing or permission denied. + err = nil + } + return rootCAs, err + } + + // Load all custom CA files. + for _, fi := range fis { + caCert, err := ioutil.ReadFile(path.Join(certsCAsDir, fi.Name())) + if err != nil { + // ignore files which are not readable. + continue + } + rootCAs.AppendCertsFromPEM(caCert) + } + + return rootCAs, nil +} diff --git a/pkg/certs/ca-certs_test.go b/pkg/certs/ca-certs_test.go new file mode 100644 index 000000000..05e567296 --- /dev/null +++ b/pkg/certs/ca-certs_test.go @@ -0,0 +1,78 @@ +/* + * MinIO Cloud Storage, (C) 2020 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package certs + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +func TestGetRootCAs(t *testing.T) { + emptydir, err := ioutil.TempDir("", "test-get-root-cas") + if err != nil { + t.Fatalf("Unable create temp directory. %v", emptydir) + } + defer os.RemoveAll(emptydir) + + dir1, err := ioutil.TempDir("", "test-get-root-cas") + if err != nil { + t.Fatalf("Unable create temp directory. %v", dir1) + } + defer os.RemoveAll(dir1) + if err = os.Mkdir(filepath.Join(dir1, "empty-dir"), 0755); err != nil { + t.Fatalf("Unable create empty dir. %v", err) + } + + dir2, err := ioutil.TempDir("", "test-get-root-cas") + if err != nil { + t.Fatalf("Unable create temp directory. %v", dir2) + } + defer os.RemoveAll(dir2) + if err = ioutil.WriteFile(filepath.Join(dir2, "empty-file"), []byte{}, 0644); err != nil { + t.Fatalf("Unable create test file. %v", err) + } + + testCases := []struct { + certCAsDir string + expectedErr error + }{ + // ignores non-existent directories. + {"nonexistent-dir", nil}, + // Ignores directories. + {dir1, nil}, + // Ignore empty directory. + {emptydir, nil}, + // Loads the cert properly. + {dir2, nil}, + } + + for _, testCase := range testCases { + _, err := GetRootCAs(testCase.certCAsDir) + + if testCase.expectedErr == nil { + if err != nil { + t.Fatalf("error: expected = , got = %v", err) + } + } else if err == nil { + t.Fatalf("error: expected = %v, got = ", testCase.expectedErr) + } else if testCase.expectedErr.Error() != err.Error() { + t.Fatalf("error: expected = %v, got = %v", testCase.expectedErr, err) + } + } +}