From af62ffcc2afa29854f485184da4d8ff23861f769 Mon Sep 17 00:00:00 2001 From: "Ben Hearsum (he/him)" Date: Fri, 18 Oct 2024 09:15:59 -0400 Subject: [PATCH] AUT-313: break up too-large-for-GCP random reads before sending them to libkmsp11 (#1032) This allows for the reading of random bytes of any size, which is needed by various operations that Autograph performs. --- signer/entropy_test.go | 2 +- signer/hsm.go | 37 +++++- signer/signer_test.go | 265 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 297 insertions(+), 7 deletions(-) diff --git a/signer/entropy_test.go b/signer/entropy_test.go index 2c0bd6ca1..f7fd5c3e3 100644 --- a/signer/entropy_test.go +++ b/signer/entropy_test.go @@ -23,7 +23,7 @@ func shannonEntropy(data []byte) (entropy float64) { } func TestRng(t *testing.T) { - for i, testcase := range PASSINGTESTCASES { + for i, testcase := range passingTestCases { rng := testcase.cfg.GetRand() randomData := make([]byte, 512) _, err := rng.Read(randomData) diff --git a/signer/hsm.go b/signer/hsm.go index cba680129..73057cd42 100644 --- a/signer/hsm.go +++ b/signer/hsm.go @@ -27,14 +27,14 @@ func (hsm *GenericHSM) GetPrivateKey(label []byte) (crypto.PrivateKey, error) { return crypto11.FindKeyPair(nil, label) } -func (hsm *GenericHSM) GetRand() io.Reader { - return new(crypto11.PKCS11RandReader) -} - type AWSHSM struct { GenericHSM } +func (hsm *AWSHSM) GetRand() io.Reader { + return new(crypto11.PKCS11RandReader) +} + func (hsm *AWSHSM) MakeKey(keyTpl interface{}, keyName string) (crypto.PrivateKey, crypto.PublicKey, error) { var slots []uint slots, err := hsm.ctx.GetSlotList(true) @@ -107,6 +107,35 @@ type GCPHSM struct { GenericHSM } +type unlimitedBytesRandReader struct { + rr crypto11.PKCS11RandReader +} + +func (ubrr *unlimitedBytesRandReader) Read(data []byte) (int, error) { + var n int + var err error + need := len(data) + for need > 0 { + var bytesRead int + next := min(need, 1024) + bytesRead, err = ubrr.rr.Read(data[n : n+next]) + n += bytesRead + if err != nil { + return n, err + } + need -= bytesRead + } + return n, err +} + +// GCPHSM.GetRand returns an io.Reader that can generate an unlimited +// number of bytes from a crypto11.PKCS11RandReader. This is needed because +// GCP KMS can generate a maximum of 1024 bytes per call. +// https://cloud.google.com/kms/docs/generate-random#known_limitations +func (hsm *GCPHSM) GetRand() io.Reader { + return new(unlimitedBytesRandReader) +} + func (hsm *GCPHSM) MakeKey(keyTpl interface{}, keyName string) (crypto.PrivateKey, crypto.PublicKey, error) { var slots []uint slots, err := hsm.ctx.GetSlotList(true) diff --git a/signer/signer_test.go b/signer/signer_test.go index 535118029..b2b943932 100644 --- a/signer/signer_test.go +++ b/signer/signer_test.go @@ -7,9 +7,11 @@ package signer import ( + "bytes" "crypto/ecdsa" "crypto/rsa" "encoding/asn1" + "errors" "fmt" "testing" @@ -218,7 +220,7 @@ func TestNoSuitableKeyFound(t *testing.T) { } func TestMakeKey(t *testing.T) { - for i, testcase := range PASSINGTESTCASES { + for i, testcase := range passingTestCases { _, keyTpl, _, err := testcase.cfg.GetKeys() if err != nil { t.Fatalf("testcase %d failed to load signer configuration: %v", i, err) @@ -235,7 +237,7 @@ func TestMakeKey(t *testing.T) { } } -var PASSINGTESTCASES = []struct { +var passingTestCases = []struct { cfg Configuration }{ {cfg: Configuration{ @@ -662,3 +664,262 @@ func TestMakeKeyGCPRSA(t *testing.T) { t.Fatalf("MakeKey failed: %v", err) } } + +var unlimitedBytesReaderPassingTestCases = []struct { + requestedBytes int + expectedSessions int + expectedPkcs11RandReaderBytes [][]byte +}{ + { + requestedBytes: 1, + expectedSessions: 1, + expectedPkcs11RandReaderBytes: [][]byte{{'A'}}, + }, + { + requestedBytes: 1024, + expectedSessions: 1, + expectedPkcs11RandReaderBytes: [][]byte{bytes.Repeat([]byte("A"), 1024)}, + }, + { + requestedBytes: 1536, + expectedSessions: 2, + expectedPkcs11RandReaderBytes: [][]byte{ + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte("B"), 512), + }, + }, + { + requestedBytes: 2047, + expectedSessions: 2, + expectedPkcs11RandReaderBytes: [][]byte{ + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte("B"), 1023), + }, + }, + { + requestedBytes: 2048, + expectedSessions: 2, + expectedPkcs11RandReaderBytes: [][]byte{ + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte("B"), 1024), + }, + }, + { + requestedBytes: 2049, + expectedSessions: 3, + expectedPkcs11RandReaderBytes: [][]byte{ + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte("B"), 1024), + {'C'}, + }, + }, + { + requestedBytes: 5000, + expectedSessions: 5, + expectedPkcs11RandReaderBytes: [][]byte{ + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte("B"), 1024), + bytes.Repeat([]byte("C"), 1024), + bytes.Repeat([]byte("D"), 1024), + bytes.Repeat([]byte("E"), 904), + }, + }, +} + +func TestUnlimitedBytesRandReader(t *testing.T) { + ubrr := new(unlimitedBytesRandReader) + + ctrl := gomock.NewController(t) + mockCtx := mockpkcs11.NewMockPKCS11Context(ctrl) + defer ctrl.Finish() + + slot := uint(0) + session := pkcs11.SessionHandle(0) + mockCtx.EXPECT().Initialize().Return(nil).Times(1) + mockCtx.EXPECT().GetSlotList(true).Return([]uint{slot}, nil).Times(2) + mockCtx.EXPECT().GetTokenInfo(slot).Return(pkcs11.TokenInfo{}, nil).Times(1) + + // somewhat surprisingly, a new session is used each time we call + // GenerateRandom. It's unclear whether this happens exclusively in tests + // or if it also happens in production code + expectedSessions := 0 + for _, testcase := range unlimitedBytesReaderPassingTestCases { + expectedSessions += testcase.expectedSessions + } + mockCtx.EXPECT().OpenSession(slot, uint(6)).Return(session, nil).Times(expectedSessions) + + mockFactory := mockedPKCS11ContextFactory(mockCtx) + crypto11.Configure(&crypto11.PKCS11Config{}, mockFactory) + defer crypto11.Close() + + for _, testcase := range unlimitedBytesReaderPassingTestCases { + var expectedBytes []byte + for _, bytes := range testcase.expectedPkcs11RandReaderBytes { + expectedBytes = append(expectedBytes, bytes...) + mockCtx.EXPECT().GenerateRandom(session, len(bytes)).Return(bytes, nil).Times(1) + } + result := make([]byte, testcase.requestedBytes) + n, err := ubrr.Read(result) + if err != nil { + t.Fatalf("unlimitedBytesRandReader.Read failed: %v", err) + } + if n != testcase.requestedBytes { + t.Fatalf("failed to read %d bytes, read %d instead", testcase.requestedBytes, n) + } + if !bytes.Equal(result, expectedBytes) { + t.Fatalf("result is not the expected bytes. got: %v, want: %v", result, expectedBytes) + } + } + // these ones are called as part of Close(), not as part of our actual testing + mockCtx.EXPECT().CloseSession(session).Return(nil).Times(expectedSessions) + mockCtx.EXPECT().CloseAllSessions(slot).Return(nil).Times(1) + mockCtx.EXPECT().Finalize().Return(nil).Times(1) + mockCtx.EXPECT().Destroy().Times(1) +} + +// inputs and outputs to/from the under the hood GenerateRandom calls +type grExpectation struct { + requestedBytes int + returnedBytes []byte +} + +var unlimitedBytesReaderFailingTestCases = []struct { + // number of random bytes to request + requestedBytes int + // the actual bytes we expect back. this will always be the same + // length as requestedBytes, and will be filled with the actual bytes + // returned for success calls to PKCS11RandReader.Read, followed by zeros + // this is true even when GenerateRandom returns some (but not all of the + // requested) random bytes, because PKCS11RandReader.Read throws the + // returned bytes from GenerateRandom on any error + expectedBytes []byte + generateRandomExpectations []grExpectation +}{ + { + requestedBytes: 5, + expectedBytes: bytes.Repeat([]byte{0}, 5), + generateRandomExpectations: []grExpectation{ + { + requestedBytes: 5, + returnedBytes: []byte{}, + }, + }, + }, + { + requestedBytes: 1024, + expectedBytes: bytes.Repeat([]byte{0}, 1024), + generateRandomExpectations: []grExpectation{ + { + requestedBytes: 1024, + returnedBytes: bytes.Repeat([]byte("A"), 5), + }, + }, + }, + { + requestedBytes: 2048, + expectedBytes: append( + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte{0}, 1024)..., + ), + generateRandomExpectations: []grExpectation{ + { + requestedBytes: 1024, + returnedBytes: bytes.Repeat([]byte("A"), 1024), + }, + { + requestedBytes: 1024, + returnedBytes: bytes.Repeat([]byte("B"), 72), + }, + }, + }, + { + requestedBytes: 5000, + expectedBytes: append( + append( + bytes.Repeat([]byte("A"), 1024), + bytes.Repeat([]byte("B"), 1024)..., + ), + bytes.Repeat([]byte{0}, 2952)..., + ), + generateRandomExpectations: []grExpectation{ + { + requestedBytes: 1024, + returnedBytes: bytes.Repeat([]byte("A"), 1024), + }, + { + requestedBytes: 1024, + returnedBytes: bytes.Repeat([]byte("B"), 1024), + }, + { + requestedBytes: 1024, + returnedBytes: bytes.Repeat([]byte("C"), 42), + }, + }, + }, +} + +func TestUnlimitedBytesRandReaderErr(t *testing.T) { + ubrr := new(unlimitedBytesRandReader) + + ctrl := gomock.NewController(t) + mockCtx := mockpkcs11.NewMockPKCS11Context(ctrl) + defer ctrl.Finish() + + slot := uint(0) + session := pkcs11.SessionHandle(0) + mockCtx.EXPECT().Initialize().Return(nil).Times(1) + mockCtx.EXPECT().GetSlotList(true).Return([]uint{slot}, nil).Times(2) + mockCtx.EXPECT().GetTokenInfo(slot).Return(pkcs11.TokenInfo{}, nil).Times(1) + + // somewhat surprisingly, a new session is used each time we call + // GenerateRandom. It's unclear whether this happens exclusively in tests + // or if it also happens in production code + expectedSessions := 0 + for _, testcase := range unlimitedBytesReaderFailingTestCases { + expectedSessions += len(testcase.generateRandomExpectations) + } + mockCtx.EXPECT().OpenSession(slot, uint(6)).Return(session, nil).Times(expectedSessions) + + mockFactory := mockedPKCS11ContextFactory(mockCtx) + crypto11.Configure(&crypto11.PKCS11Config{}, mockFactory) + defer crypto11.Close() + + entropyErr := errors.New("not enough entropy") + + for _, testcase := range unlimitedBytesReaderFailingTestCases { + expectedN := 0 + for _, expectation := range testcase.generateRandomExpectations { + var expectedErr error = nil + if len(expectation.returnedBytes) != expectation.requestedBytes { + // if returned bytes and requested bytes are not the same, + // we're simulating a failure in GenerateRandom. + // we do not increase expectedN, because GenerateRandom always + // returns n = 0 in an error case + expectedErr = entropyErr + } else { + // if they are the same, expectedN goes up by the number of bytes + // returned + expectedN += len(expectation.returnedBytes) + } + mockCtx.EXPECT().GenerateRandom(session, expectation.requestedBytes).Return(expectation.returnedBytes, expectedErr).Times(1) + } + + result := make([]byte, testcase.requestedBytes) + n, err := ubrr.Read(result) + if err != entropyErr { + t.Fatalf("expected error %v, got %v", entropyErr, err) + } + if n != expectedN { + t.Fatalf("expected Read to return %d bytes, got %d", expectedN, n) + } + if !bytes.Equal(result, testcase.expectedBytes) { + t.Fatalf("result is not the expected bytes. got: %v, wanted: %v", result, testcase.expectedBytes) + } + } + + // these ones are called as part of Close(), not as part of our actual testing + mockCtx.EXPECT().CloseSession(session).Return(nil).Times(expectedSessions) + mockCtx.EXPECT().CloseAllSessions(slot).Return(nil).Times(1) + mockCtx.EXPECT().Finalize().Return(nil).Times(1) + mockCtx.EXPECT().Destroy().Times(1) +}