diff --git a/src/control/security/config.go b/src/control/security/config.go index 7539bd51e90..db58f09a666 100644 --- a/src/control/security/config.go +++ b/src/control/security/config.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2021 Intel Corporation. +// (C) Copyright 2019-2023 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -11,6 +11,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "io/fs" "github.com/pkg/errors" ) @@ -51,6 +52,7 @@ type CertificateConfig struct { PrivateKeyPath string `yaml:"key"` tlsKeypair *tls.Certificate `yaml:"-"` caPool *x509.CertPool `yaml:"-"` + maxKeyPerms fs.FileMode `yaml:"-"` } // DefaultAgentTransportConfig provides a default transport config disabling @@ -66,14 +68,14 @@ func DefaultAgentTransportConfig() *TransportConfig { PrivateKeyPath: defaultAgentKey, tlsKeypair: nil, caPool: nil, + maxKeyPerms: MaxUserOnlyKeyPerm, }, } } // DefaultClientTransportConfig provides a default transport config disabling // certificate usage and specifying certificates located under /etc/daos/certs. -// As this -// credential is meant to be used as a client credential it specifies a default +// As this credential is meant to be used as a client credential it specifies a default // ServerName as well. func DefaultClientTransportConfig() *TransportConfig { return &TransportConfig{ @@ -86,6 +88,7 @@ func DefaultClientTransportConfig() *TransportConfig { PrivateKeyPath: defaultAdminKey, tlsKeypair: nil, caPool: nil, + maxKeyPerms: MaxGroupKeyPerm, }, } } @@ -103,6 +106,7 @@ func DefaultServerTransportConfig() *TransportConfig { PrivateKeyPath: defaultServerKey, tlsKeypair: nil, caPool: nil, + maxKeyPerms: MaxUserOnlyKeyPerm, }, } } @@ -119,7 +123,7 @@ func (tc *TransportConfig) PreLoadCertData() error { // In order to reload data use ReloadCertDatA return nil } - certificate, certPool, err := loadCertWithCustomCA(tc.CARootPath, tc.CertificatePath, tc.PrivateKeyPath) + certificate, certPool, err := loadCertWithCustomCA(tc.CARootPath, tc.CertificatePath, tc.PrivateKeyPath, tc.maxKeyPerms) if err != nil { return err } diff --git a/src/control/security/config_test.go b/src/control/security/config_test.go index 459eacffc1b..508246b34a0 100644 --- a/src/control/security/config_test.go +++ b/src/control/security/config_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2021 Intel Corporation. +// (C) Copyright 2019-2023 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -12,6 +12,8 @@ import ( "os" "strings" "testing" + + "github.com/google/go-cmp/cmp" ) func InsecureTC() *TransportConfig { @@ -42,6 +44,7 @@ func ServerTC() *TransportConfig { PrivateKeyPath: "testdata/certs/server.key", tlsKeypair: nil, caPool: nil, + maxKeyPerms: MaxUserOnlyKeyPerm, }, } } @@ -55,6 +58,7 @@ func AgentTC() *TransportConfig { PrivateKeyPath: "testdata/certs/agent.key", tlsKeypair: nil, caPool: nil, + maxKeyPerms: MaxUserOnlyKeyPerm, }, } } @@ -66,7 +70,7 @@ func SetupTCFilePerms(t *testing.T, conf *TransportConfig) { if err := os.Chmod(conf.CertificatePath, MaxCertPerm); err != nil { t.Fatal(err) } - if err := os.Chmod(conf.PrivateKeyPath, MaxKeyPerm); err != nil { + if err := os.Chmod(conf.PrivateKeyPath, MaxUserOnlyKeyPerm); err != nil { t.Fatal(err) } } @@ -240,3 +244,59 @@ func TestPublicKey(t *testing.T) { }) } } + +func TestSecurity_DefaultTransportConfigs(t *testing.T) { + for name, tc := range map[string]struct { + genTransportConfig func() *TransportConfig + expResult *TransportConfig + }{ + "admin": { + genTransportConfig: DefaultClientTransportConfig, + expResult: &TransportConfig{ + CertificateConfig: CertificateConfig{ + ServerName: defaultServer, + CARootPath: defaultCACert, + CertificatePath: defaultAdminCert, + PrivateKeyPath: defaultAdminKey, + maxKeyPerms: MaxGroupKeyPerm, + }, + }, + }, + "agent": { + genTransportConfig: DefaultAgentTransportConfig, + expResult: &TransportConfig{ + CertificateConfig: CertificateConfig{ + ServerName: defaultServer, + CARootPath: defaultCACert, + CertificatePath: defaultAgentCert, + PrivateKeyPath: defaultAgentKey, + maxKeyPerms: MaxUserOnlyKeyPerm, + }, + }, + }, + "server": { + genTransportConfig: DefaultServerTransportConfig, + expResult: &TransportConfig{ + CertificateConfig: CertificateConfig{ + ServerName: defaultServer, + CARootPath: defaultCACert, + ClientCertDir: defaultClientCertDir, + CertificatePath: defaultServerCert, + PrivateKeyPath: defaultServerKey, + maxKeyPerms: MaxUserOnlyKeyPerm, + }, + }, + }, + } { + t.Run(name, func(t *testing.T) { + result := tc.genTransportConfig() + + if diff := cmp.Diff(tc.expResult, result, cmp.AllowUnexported( + TransportConfig{}, + CertificateConfig{}, + )); diff != "" { + t.Fatalf("(want-, got+)\n %s", diff) + } + }) + } +} diff --git a/src/control/security/pem.go b/src/control/security/pem.go index fada6c6a5a5..05d6d983837 100644 --- a/src/control/security/pem.go +++ b/src/control/security/pem.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2021 Intel Corporation. +// (C) Copyright 2019-2023 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -21,9 +21,10 @@ import ( ) const ( - MaxKeyPerm os.FileMode = 0700 - MaxCertPerm os.FileMode = 0664 - MaxDirPerm os.FileMode = 0700 + MaxUserOnlyKeyPerm os.FileMode = 0400 + MaxGroupKeyPerm os.FileMode = 0440 + MaxCertPerm os.FileMode = 0664 + MaxDirPerm os.FileMode = 0750 ) type badPermsError struct { @@ -47,7 +48,7 @@ func checkMaxPermissions(filePath string, mode os.FileMode, modeMax os.FileMode) return nil } -func loadCertWithCustomCA(caRootPath, certPath, keyPath string) (*tls.Certificate, *x509.CertPool, error) { +func loadCertWithCustomCA(caRootPath, certPath, keyPath string, maxKeyPerm os.FileMode) (*tls.Certificate, *x509.CertPool, error) { caPEM, err := LoadPEMData(caRootPath, MaxCertPerm) if err != nil { switch { @@ -72,7 +73,7 @@ func loadCertWithCustomCA(caRootPath, certPath, keyPath string) (*tls.Certificat } } - keyPEM, err := LoadPEMData(keyPath, MaxKeyPerm) + keyPEM, err := LoadPEMData(keyPath, maxKeyPerm) if err != nil { switch { case os.IsNotExist(err): @@ -149,7 +150,7 @@ func LoadCertificate(certPath string) (*x509.Certificate, error) { // LoadPrivateKey loads the private key specified at the given path into an // crypto.PrivateKey interface compliant object. func LoadPrivateKey(keyPath string) (crypto.PrivateKey, error) { - pemData, err := LoadPEMData(keyPath, MaxKeyPerm) + pemData, err := LoadPEMData(keyPath, MaxUserOnlyKeyPerm) if err != nil { return nil, err diff --git a/src/control/security/pem_test.go b/src/control/security/pem_test.go index b4cca6ab396..86e9b161937 100644 --- a/src/control/security/pem_test.go +++ b/src/control/security/pem_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2021 Intel Corporation. +// (C) Copyright 2019-2023 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -62,26 +62,26 @@ func TestLoadPrivateKey(t *testing.T) { malformed := "testdata/certs/bad.key" toomany := "testdata/certs/toomanypem.key" notkey := "testdata/certs/notkey.crt" - badKeyPermError := &badPermsError{badKeyPerm, MaxCertPerm, MaxKeyPerm} + badKeyPermError := &badPermsError{badKeyPerm, MaxCertPerm, MaxUserOnlyKeyPerm} malformedError := fmt.Sprintf("%s does not contain PEM data", malformed) toomanyError := "Only one key allowed per file" notkeyError := "PEM Block is not a Private Key" // Setup permissions for tests below. - if err := os.Chmod(goodKeyPath, MaxKeyPerm); err != nil { + if err := os.Chmod(goodKeyPath, MaxUserOnlyKeyPerm); err != nil { t.Fatal(err) } // Intentionallly safe cert perm as it should be incorrect if err := os.Chmod(badKeyPerm, MaxCertPerm); err != nil { t.Fatal(err) } - if err := os.Chmod(malformed, MaxKeyPerm); err != nil { + if err := os.Chmod(malformed, MaxUserOnlyKeyPerm); err != nil { t.Fatal(err) } - if err := os.Chmod(toomany, MaxKeyPerm); err != nil { + if err := os.Chmod(toomany, MaxUserOnlyKeyPerm); err != nil { t.Fatal(err) } - if err := os.Chmod(notkey, MaxKeyPerm); err != nil { + if err := os.Chmod(notkey, MaxUserOnlyKeyPerm); err != nil { t.Fatal(err) } @@ -115,7 +115,7 @@ func TestLoadCertificate(t *testing.T) { badPerm := "testdata/certs/badperms.crt" malformed := "testdata/certs/bad.crt" toomany := "testdata/certs/toomanypem.crt" - badError := &badPermsError{badPerm, MaxKeyPerm, MaxCertPerm} + badError := &badPermsError{badPerm, 0700, MaxCertPerm} malformedError := fmt.Sprintf("%s does not contain PEM data", malformed) toomanyError := "Only one cert allowed per file" @@ -124,7 +124,7 @@ func TestLoadCertificate(t *testing.T) { t.Fatal(err) } // Intentionally safe cert perm as it should be incorrect - if err := os.Chmod(badPerm, MaxKeyPerm); err != nil { + if err := os.Chmod(badPerm, 0700); err != nil { t.Fatal(err) } if err := os.Chmod(malformed, MaxCertPerm); err != nil { diff --git a/src/control/security/signature_test.go b/src/control/security/signature_test.go index e639eb0cd6b..ac1cfc1b871 100644 --- a/src/control/security/signature_test.go +++ b/src/control/security/signature_test.go @@ -1,5 +1,5 @@ // -// (C) Copyright 2019-2021 Intel Corporation. +// (C) Copyright 2019-2023 Intel Corporation. // // SPDX-License-Identifier: BSD-2-Clause-Patent // @@ -33,7 +33,7 @@ func SeededSigner() *TokenSigner { func SignTestSetup(t *testing.T) (rsaKey, ecdsaKey crypto.PrivateKey, source []byte) { keyPath := "testdata/certs/daosCA.key" - if err := os.Chmod(keyPath, MaxKeyPerm); err != nil { + if err := os.Chmod(keyPath, MaxUserOnlyKeyPerm); err != nil { t.Fatal(err) } rsaKey, err := LoadPrivateKey(keyPath) diff --git a/src/control/security/testdata/certs/agent.key b/src/control/security/testdata/certs/agent.key old mode 100755 new mode 100644 diff --git a/src/control/security/testdata/certs/bad.key b/src/control/security/testdata/certs/bad.key old mode 100755 new mode 100644 diff --git a/src/control/security/testdata/certs/daosCA.key b/src/control/security/testdata/certs/daosCA.key old mode 100755 new mode 100644 diff --git a/src/control/security/testdata/certs/notkey.crt b/src/control/security/testdata/certs/notkey.crt old mode 100755 new mode 100644 diff --git a/src/control/security/testdata/certs/server.key b/src/control/security/testdata/certs/server.key old mode 100755 new mode 100644 diff --git a/src/control/security/testdata/certs/toomanypem.key b/src/control/security/testdata/certs/toomanypem.key old mode 100755 new mode 100644