From c532ec436a8b983d7ae9cd7d20006a375aa77c39 Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sun, 1 May 2022 12:36:17 +0200 Subject: [PATCH] Improve test coverage of package hsm --- pkg/hsm/context.go | 34 ++++++- pkg/hsm/context_test.go | 92 ++++++++++++++----- pkg/hsm/hsm.go | 143 +++++++++++++++++++---------- pkg/hsm/hsm_test.go | 196 ++++++++++++++++++++++++++++++++++++++++ pkg/hsm/setup.go | 2 +- pkg/hsm/setup_test.go | 89 ++++++++++++++++++ 6 files changed, 481 insertions(+), 75 deletions(-) create mode 100644 pkg/hsm/hsm_test.go create mode 100644 pkg/hsm/setup_test.go diff --git a/pkg/hsm/context.go b/pkg/hsm/context.go index 8bc0f2a..36c0024 100644 --- a/pkg/hsm/context.go +++ b/pkg/hsm/context.go @@ -20,6 +20,7 @@ package hsm import ( "context" "errors" + "fmt" "github.com/ThalesIgnite/crypto11" @@ -29,7 +30,8 @@ import ( type ctxKey int const ( - ctxP11Contexts ctxKey = iota + ctxCADirectory ctxKey = iota + ctxP11Contexts ctxSetupMode ctxSignerConfig ctxVerboseLogging @@ -37,6 +39,12 @@ const ( type ConfigOption func(ctx context.Context) context.Context +func CADirectoryOption(path string) func(ctx context.Context) context.Context { + return func(ctx context.Context) context.Context { + return context.WithValue(ctx, ctxCADirectory, path) + } +} + func CaConfigOption(signerConfig *config.SignerConfig) func(context.Context) context.Context { return func(ctx context.Context) context.Context { return context.WithValue(ctx, ctxSignerConfig, signerConfig) @@ -114,3 +122,27 @@ func GetP11Context(ctx context.Context, entry *config.CaCertificateEntry) (*cryp return p11Context, nil } + +func CloseP11Contexts(ctx context.Context) error { + contexts, ok := ctx.Value(ctxP11Contexts).(map[string]*crypto11.Context) + if !ok { + return errors.New("type assertion failed, use hsm.SetupContext first") + } + + seen := make(map[*crypto11.Context]struct{}, 0) + + for _, p11Context := range contexts { + if _, ok := seen[p11Context]; ok { + continue + } + + seen[p11Context] = struct{}{} + + err := p11Context.Close() + if err != nil { + return fmt.Errorf("could not close context: %w", err) + } + } + + return nil +} diff --git a/pkg/hsm/context_test.go b/pkg/hsm/context_test.go index 7435c71..51868e1 100644 --- a/pkg/hsm/context_test.go +++ b/pkg/hsm/context_test.go @@ -36,49 +36,49 @@ import ( func TestCaConfigOption(t *testing.T) { testSignerConfig := config.SignerConfig{} - theContext := hsm.SetupContext(hsm.CaConfigOption(&testSignerConfig)) + ctx := hsm.SetupContext(hsm.CaConfigOption(&testSignerConfig)) - assert.Equal(t, &testSignerConfig, hsm.GetSignerConfig(theContext)) + assert.Equal(t, &testSignerConfig, hsm.GetSignerConfig(ctx)) } func TestGetSignerConfig_empty(t *testing.T) { - theContext := hsm.SetupContext() + ctx := hsm.SetupContext() - assert.Nil(t, hsm.GetSignerConfig(theContext)) + assert.Nil(t, hsm.GetSignerConfig(ctx)) } func TestSetupModeOption(t *testing.T) { - theContext := hsm.SetupContext(hsm.SetupModeOption()) + ctx := hsm.SetupContext(hsm.SetupModeOption()) - assert.True(t, hsm.IsSetupMode(theContext)) + assert.True(t, hsm.IsSetupMode(ctx)) } func TestIsSetupMode_not_set(t *testing.T) { - theContext := hsm.SetupContext() + ctx := hsm.SetupContext() - assert.False(t, hsm.IsSetupMode(theContext)) + assert.False(t, hsm.IsSetupMode(ctx)) } func TestVerboseLoggingOption(t *testing.T) { - theContext := hsm.SetupContext(hsm.VerboseLoggingOption()) + ctx := hsm.SetupContext(hsm.VerboseLoggingOption()) - assert.True(t, hsm.IsVerbose(theContext)) + assert.True(t, hsm.IsVerbose(ctx)) } func TestIsVerbose_not_set(t *testing.T) { - theContext := hsm.SetupContext() + ctx := hsm.SetupContext() - assert.False(t, hsm.IsVerbose(theContext)) + assert.False(t, hsm.IsVerbose(ctx)) } func TestSetupContext(t *testing.T) { testConfig := setupSignerConfig(t) - theContext := hsm.SetupContext(hsm.SetupModeOption(), hsm.VerboseLoggingOption(), hsm.CaConfigOption(testConfig)) + ctx := hsm.SetupContext(hsm.SetupModeOption(), hsm.VerboseLoggingOption(), hsm.CaConfigOption(testConfig)) - assert.True(t, hsm.IsSetupMode(theContext)) - assert.True(t, hsm.IsVerbose(theContext)) - assert.Equal(t, hsm.GetSignerConfig(theContext), testConfig) + assert.True(t, hsm.IsSetupMode(ctx)) + assert.True(t, hsm.IsVerbose(ctx)) + assert.Equal(t, hsm.GetSignerConfig(ctx), testConfig) } func TestGetP11Context_missing_SetupContext(t *testing.T) { @@ -92,33 +92,72 @@ func TestGetP11Context_missing_SetupContext(t *testing.T) { func TestGetP11Context_unknown_storage(t *testing.T) { testConfig := setupSignerConfig(t) - theContext := hsm.SetupContext(hsm.SetupModeOption(), hsm.CaConfigOption(testConfig)) + ctx := hsm.SetupContext(hsm.SetupModeOption(), hsm.CaConfigOption(testConfig)) definition := &config.CaCertificateEntry{Storage: "undefined"} - p11Context, err := hsm.GetP11Context(theContext, definition) + p11Context, err := hsm.GetP11Context(ctx, definition) assert.Error(t, err) assert.ErrorContains(t, err, "key storage undefined not available") assert.Nil(t, p11Context) } -func TestGetP11Context(t *testing.T) { +func TestGetP11Context_wrong_pin(t *testing.T) { testConfig := setupSignerConfig(t) setupSoftHsm(t) - theContext := hsm.SetupContext(hsm.CaConfigOption(testConfig)) + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "wrongpin") + + ctx := hsm.SetupContext(hsm.CaConfigOption(testConfig)) definition, err := testConfig.GetCADefinition("root") require.NoError(t, err) - p11Context1, err := hsm.GetP11Context(theContext, definition) + _, err = hsm.GetP11Context(ctx, definition) + + assert.ErrorContains(t, err, "could not configure PKCS#11 library") +} + +func TestGetP11Context_no_pin(t *testing.T) { + testConfig := setupSignerConfig(t) + setupSoftHsm(t) + + ctx := hsm.SetupContext(hsm.CaConfigOption(testConfig)) + + definition, err := testConfig.GetCADefinition("root") + + require.NoError(t, err) + + _, err = hsm.GetP11Context(ctx, definition) + + assert.ErrorContains(t, err, "stdin is not a terminal") +} + +func TestGetP11Context(t *testing.T) { + testConfig := setupSignerConfig(t) + setupSoftHsm(t) + + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") + + ctx := hsm.SetupContext(hsm.CaConfigOption(testConfig)) + + definition, err := testConfig.GetCADefinition("root") + + require.NoError(t, err) + + p11Context1, err := hsm.GetP11Context(ctx, definition) assert.NoError(t, err) assert.NotNil(t, p11Context1) - p11Context2, err := hsm.GetP11Context(theContext, definition) + p11Context2, err := hsm.GetP11Context(ctx, definition) + + t.Cleanup(func() { + err := hsm.CloseP11Contexts(ctx) + assert.NoError(t, err) + }) assert.NoError(t, err) assert.NotNil(t, p11Context1) @@ -201,6 +240,11 @@ func setupSoftHsm(t *testing.T) { ).Run() require.NoError(t, err) - - t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") +} + +func TestCloseP11Contexts_without_setup(t *testing.T) { + ctx := context.Background() + err := hsm.CloseP11Contexts(ctx) + + assert.ErrorContains(t, err, "type assertion failed, use hsm.SetupContext first") } diff --git a/pkg/hsm/hsm.go b/pkg/hsm/hsm.go index 3ea050e..12630bf 100644 --- a/pkg/hsm/hsm.go +++ b/pkg/hsm/hsm.go @@ -32,6 +32,7 @@ import ( "log" "math/big" "os" + "path" "syscall" "time" @@ -48,6 +49,83 @@ var ( errCertificateGenerationRefused = errors.New("not in setup mode, refusing certificate generation") ) +type caFile struct { + sc *config.SignerConfig + label string +} + +func (c *caFile) buildCertificatePath(ctx context.Context) (string, error) { + fileName := c.sc.CertificateFileName(c.label) + + caDir := ctx.Value(ctxCADirectory) + + if caDir != nil { + caPath, ok := caDir.(string) + if !ok { + return "", errors.New("context object CA directory is not a string") + } + + return path.Join(caPath, fileName), nil + } + + return fileName, nil +} + +func (c *caFile) loadCertificate(ctx context.Context) (*x509.Certificate, error) { + certFile, err := c.buildCertificatePath(ctx) + if err != nil { + return nil, err + } + + certFileInfo, err := os.Stat(certFile) + if err != nil { + if errors.Is(err, syscall.ENOENT) { + return nil, nil + } + + return nil, fmt.Errorf("could not get info for %s: %w", certFile, err) + } + + if !certFileInfo.Mode().IsRegular() { + return nil, fmt.Errorf("certificate file %s is not a regular file", certFile) + } + + certData, err := os.ReadFile(certFile) + if err != nil { + return nil, fmt.Errorf("could not read %s: %w", certFile, err) + } + + pemData, _ := pem.Decode(certData) + if pemData == nil { + return nil, fmt.Errorf("no PEM data in %s", certFile) + } + + if pemData.Type != "CERTIFICATE" { + return nil, fmt.Errorf("no certificate found in %s", certFile) + } + + certificate, err := x509.ParseCertificate(pemData.Bytes) + if err != nil { + return nil, fmt.Errorf("could not parse certificate from %s: %w", certFile, err) + } + + return certificate, nil +} + +func (c *caFile) storeCertificate(ctx context.Context, certificate []byte) error { + certFile, err := c.buildCertificatePath(ctx) + if err != nil { + return err + } + + err = os.WriteFile(certFile, certificate, 0o600) + if err != nil { + return fmt.Errorf("could not write certificate file %s: %w", certFile, err) + } + + return nil +} + func GetRootCACertificate(ctx context.Context, label string) (*x509.Certificate, error) { var ( certificate *x509.Certificate @@ -65,9 +143,9 @@ func GetRootCACertificate(ctx context.Context, label string) (*x509.Certificate, return nil, fmt.Errorf("CA definition %s is not a root CA definition", label) } - certFile := sc.CertificateFileName(label) + caFile := &caFile{sc: sc, label: label} - certificate, err = loadCertificate(certFile) + certificate, err = caFile.loadCertificate(ctx) if err != nil { return nil, err } @@ -97,7 +175,8 @@ func GetRootCACertificate(ctx context.Context, label string) (*x509.Certificate, subject := sc.CalculateSubject(caCert) certificate, err = generateRootCACertificate( - certFile, + ctx, + caFile, keyPair, &x509.Certificate{ Subject: subject, @@ -155,9 +234,9 @@ func GetIntermediaryCACertificate(ctx context.Context, certLabel string) (*x509. return nil, err } - certFile := sc.CertificateFileName(certLabel) + certFile := &caFile{sc: sc, label: certLabel} - certificate, err = loadCertificate(certFile) + certificate, err = certFile.loadCertificate(ctx) if err != nil { return nil, err } @@ -176,6 +255,8 @@ func GetIntermediaryCACertificate(ctx context.Context, certLabel string) (*x509. subject := sc.CalculateSubject(caCert) certificate, err = generateIntermediaryCACertificate( + ctx, + certFile, sc, certLabel, keyPair.Public(), @@ -219,6 +300,8 @@ func GetIntermediaryCACertificate(ctx context.Context, certLabel string) (*x509. } func generateIntermediaryCACertificate( + ctx context.Context, + certFile *caFile, config *config.SignerConfig, certLabel string, publicKey crypto.PublicKey, @@ -257,11 +340,9 @@ func generateIntermediaryCACertificate( Bytes: certBytes, } - certFile := config.CertificateFileName(certLabel) - - err = os.WriteFile(certFile, pem.EncodeToMemory(certBlock), 0o600) + err = certFile.storeCertificate(ctx, pem.EncodeToMemory(certBlock)) if err != nil { - return nil, fmt.Errorf("could not write certificate to %s: %w", certFile, err) + return nil, err } certificate, err := x509.ParseCertificate(certBytes) @@ -400,7 +481,8 @@ func randomObjectID() ([]byte, error) { } func generateRootCACertificate( - certFile string, + ctx context.Context, + certFile *caFile, keyPair crypto.Signer, template *x509.Certificate, ) (*x509.Certificate, error) { @@ -432,9 +514,8 @@ func generateRootCACertificate( Bytes: certBytes, } - err = os.WriteFile(certFile, pem.EncodeToMemory(certBlock), 0o600) - if err != nil { - return nil, fmt.Errorf("could not write certificate to %s: %w", certFile, err) + if err = certFile.storeCertificate(ctx, pem.EncodeToMemory(certBlock)); err != nil { + return nil, err } certificate, err := x509.ParseCertificate(certBytes) @@ -484,42 +565,6 @@ func certificateMatches(certificate *x509.Certificate, key crypto.Signer) bool { return false } -func loadCertificate(certFile string) (*x509.Certificate, error) { - certFileInfo, err := os.Stat(certFile) - if err != nil { - if errors.Is(err, syscall.ENOENT) { - return nil, nil - } - - return nil, fmt.Errorf("could not get info for %s: %w", certFile, err) - } - - if !certFileInfo.Mode().IsRegular() { - return nil, fmt.Errorf("certificate file %s is not a regular file", certFile) - } - - certData, err := os.ReadFile(certFile) - if err != nil { - return nil, fmt.Errorf("could not read %s: %w", certFile, err) - } - - pemData, _ := pem.Decode(certData) - if pemData == nil { - return nil, fmt.Errorf("no PEM data in %s", certFile) - } - - if pemData.Type != "CERTIFICATE" { - return nil, fmt.Errorf("no certificate found in %s", certFile) - } - - certificate, err := x509.ParseCertificate(pemData.Bytes) - if err != nil { - return nil, fmt.Errorf("could not parse certificate from %s: %w", certFile, err) - } - - return certificate, nil -} - func randomSerialNumber() (*big.Int, error) { serialNumber, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) if err != nil { diff --git a/pkg/hsm/hsm_test.go b/pkg/hsm/hsm_test.go new file mode 100644 index 0000000..71da724 --- /dev/null +++ b/pkg/hsm/hsm_test.go @@ -0,0 +1,196 @@ +/* +Copyright 2022 CAcert Inc. +SPDX-License-Identifier: Apache-2.0 + +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 hsm_test + +import ( + "context" + "crypto/x509" + "strings" + "testing" + + "git.cacert.org/cacert-gosigner/pkg/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "git.cacert.org/cacert-gosigner/pkg/hsm" +) + +func TestEnsureCAKeysAndCertificates_not_in_setup_mode(t *testing.T) { + testConfig := setupSignerConfig(t) + setupSoftHsm(t) + + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") + + ctx := hsm.SetupContext( + hsm.CaConfigOption(testConfig), + hsm.CADirectoryOption(t.TempDir())) + + err := hsm.EnsureCAKeysAndCertificates(ctx) + + t.Cleanup(func() { + err := hsm.CloseP11Contexts(ctx) + assert.NoError(t, err) + }) + + assert.ErrorContains(t, err, "not in setup mode") +} + +func prepareSoftHSM(t *testing.T) context.Context { + t.Helper() + + testConfig := setupSignerConfig(t) + setupSoftHsm(t) + + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") + + ctx := hsm.SetupContext( + hsm.CaConfigOption(testConfig), + hsm.SetupModeOption(), + hsm.CADirectoryOption(t.TempDir())) + + err := hsm.EnsureCAKeysAndCertificates(ctx) + + t.Cleanup(func() { + err := hsm.CloseP11Contexts(ctx) + assert.NoError(t, err) + }) + + require.NoError(t, err) + + return ctx +} + +func TestGetRootCACertificate(t *testing.T) { + ctx := prepareSoftHSM(t) + + testData := map[string]struct { + label, errMsg string + }{ + "known root": { + label: "root", + }, + "unknown root": { + label: "unknown", + errMsg: "could not get CA definition for label unknown", + }, + "known intermediary": { + label: "sub1", + errMsg: "CA definition sub1 is not a root CA definition", + }, + } + + for name, item := range testData { + t.Run(name, func(t *testing.T) { + root, err := hsm.GetRootCACertificate(ctx, item.label) + + if item.errMsg != "" { + assert.ErrorContains(t, err, item.errMsg) + assert.Nil(t, root) + } else { + assert.NoError(t, err) + assert.NotNil(t, root) + } + }) + } +} + +func TestGetIntermediaryCACertificate(t *testing.T) { + ctx := prepareSoftHSM(t) + + testData := map[string]struct { + label, errMsg string + }{ + "known intermediary": { + label: "sub1", + }, + "unknown intermediary": { + label: "unknown", + errMsg: "could not get CA definition for label unknown", + }, + "known root": { + label: "root", + errMsg: "CA definition root is a root CA definition, intermediary expected", + }, + } + + for name, item := range testData { + t.Run(name, func(t *testing.T) { + root, err := hsm.GetIntermediaryCACertificate(ctx, item.label) + + if item.errMsg != "" { + assert.ErrorContains(t, err, item.errMsg) + assert.Nil(t, root) + } else { + assert.NoError(t, err) + assert.NotNil(t, root) + } + }) + } +} + +func TestRSAKeyGeneration(t *testing.T) { + const testRSASignerConfig = `--- +Settings: + organization: + organization: ["Acme CAs Ltd."] + validity-years: + root: 30 + intermediary: 10 + url-patterns: + ocsp: http://ocsp.example.org/ + crl: http://crl.example.org/%s.crl + issuer: http://%s.cas.example.org/ +CAs: + root: + common-name: "Acme CAs root" + key-info: + algorithm: RSA + rsa-bits: 2048 +KeyStorage: + default: + type: softhsm + label: acme-test-hsm +` + testConfig, err := config.LoadConfiguration(strings.NewReader(testRSASignerConfig)) + + require.NoError(t, err) + + setupSoftHsm(t) + + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") + + ctx := hsm.SetupContext( + hsm.CaConfigOption(testConfig), + hsm.SetupModeOption(), + hsm.CADirectoryOption(t.TempDir())) + + err = hsm.EnsureCAKeysAndCertificates(ctx) + + t.Cleanup(func() { + err := hsm.CloseP11Contexts(ctx) + assert.NoError(t, err) + }) + + require.NoError(t, err) + + root, err := hsm.GetRootCACertificate(ctx, "root") + + assert.NoError(t, err) + assert.NotNil(t, root) + assert.Equal(t, x509.RSA, root.PublicKeyAlgorithm) +} diff --git a/pkg/hsm/setup.go b/pkg/hsm/setup.go index 5d63e7e..b3d60bf 100644 --- a/pkg/hsm/setup.go +++ b/pkg/hsm/setup.go @@ -27,7 +27,7 @@ func EnsureCAKeysAndCertificates(ctx context.Context) error { conf := GetSignerConfig(ctx) - for _, label := range conf.RootCAs() { + for _, label = range conf.RootCAs() { crt, err := GetRootCACertificate(ctx, label) if err != nil { return err diff --git a/pkg/hsm/setup_test.go b/pkg/hsm/setup_test.go new file mode 100644 index 0000000..524d50b --- /dev/null +++ b/pkg/hsm/setup_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2022 CAcert Inc. +SPDX-License-Identifier: Apache-2.0 + +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 hsm_test + +import ( + "bytes" + "log" + "testing" + + "github.com/stretchr/testify/assert" + + "git.cacert.org/cacert-gosigner/pkg/hsm" +) + +func TestEnsureCAKeysAndCertificates(t *testing.T) { + testConfig := setupSignerConfig(t) + setupSoftHsm(t) + + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") + + buf := bytes.NewBuffer(nil) + + log.SetOutput(buf) + + ctx := hsm.SetupContext( + hsm.CaConfigOption(testConfig), + hsm.SetupModeOption(), + hsm.CADirectoryOption(t.TempDir())) + + err := hsm.EnsureCAKeysAndCertificates(ctx) + + t.Cleanup(func() { + err := hsm.CloseP11Contexts(ctx) + assert.NoError(t, err) + }) + + assert.NoError(t, err) + + output := buf.String() + + assert.NoError(t, err) + assert.Contains(t, output, "found root CA certificate root: Acme CAs root") + assert.Contains(t, output, "found intermediary CA certificate sub1: Acme CAs server sub CA") +} + +func TestEnsureCAKeysAndCertificates_verbose(t *testing.T) { + testConfig := setupSignerConfig(t) + setupSoftHsm(t) + + t.Setenv("TOKEN_PIN_ACME_TEST_HSM", "123456") + + buf := bytes.NewBuffer(nil) + + log.SetOutput(buf) + + ctx := hsm.SetupContext( + hsm.CaConfigOption(testConfig), + hsm.SetupModeOption(), + hsm.VerboseLoggingOption(), + hsm.CADirectoryOption(t.TempDir())) + + err := hsm.EnsureCAKeysAndCertificates(ctx) + + t.Cleanup(func() { + err := hsm.CloseP11Contexts(ctx) + assert.NoError(t, err) + }) + + output := buf.String() + + assert.NoError(t, err) + assert.Contains(t, output, "found root CA certificate root:\n Subject") + assert.Contains(t, output, "found intermediary CA certificate sub1:\n Subject") +}