Skip to content

Commit

Permalink
handle terminal tpm errors (#2110)
Browse files Browse the repository at this point in the history
  • Loading branch information
James-Pickett authored Feb 19, 2025
1 parent 4f23f65 commit 12f4c62
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 9 deletions.
24 changes: 21 additions & 3 deletions ee/tpmrunner/tpmrunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"fmt"
"io"
"log/slog"
"os"
"runtime"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -67,7 +69,23 @@ func New(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDele
}

// assume we have a tpm until we know otherwise
tpmRunner.machineHasTpm.Store(true)
hasTPM := true

// on linux the TPM is at /dev/tpm0
// if it doesn't exist, we don't have a TPM
if runtime.GOOS == "linux" {
_, err := os.Stat("/dev/tpm0")
hasTPM = err == nil

if !hasTPM {
slogger.Log(ctx, slog.LevelInfo,
"no tpm found",
"err", err,
)
}
}

tpmRunner.machineHasTpm.Store(hasTPM)

for _, opt := range opts {
opt(tpmRunner)
Expand Down Expand Up @@ -228,11 +246,11 @@ func (tr *tpmRunner) loadOrCreateKeys(ctx context.Context) error {
priData, pubData, err = tr.signerCreator.CreateKey()
if err != nil {

if isTPMNotFoundErr(err) {
if isTerminalTPMError(err) {
tr.machineHasTpm.Store(false)

tr.slogger.Log(ctx, slog.LevelInfo,
"tpm not found",
"terminal tpm error, not retrying",
"err", err,
)

Expand Down
24 changes: 21 additions & 3 deletions ee/tpmrunner/tpmrunner_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,26 @@

package tpmrunner

// isTPMNotFoundErr always return false on linux because we don't yet how to
// detect if a TPM is not found on linux.
func isTPMNotFoundErr(err error) bool {
import (
"errors"
"os"
)

var terminalErrors = []error{
// on linux, if the tpm device is not present, we get this error
// stat /dev/tpm0: no such file or directory
// we should check this when we create a new tpmrunner so this
// is maybe belt and suspenders
os.ErrNotExist,
}

// isTerminalTPMError returns true if we should stop trying to use the TPM.
func isTerminalTPMError(err error) bool {
for _, e := range terminalErrors {
if errors.Is(err, e) {
return true
}
}

return false
}
15 changes: 15 additions & 0 deletions ee/tpmrunner/tpmrunner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ func Test_tpmRunner(t *testing.T) {
tpmRunner, err := New(context.TODO(), multislogger.NewNopLogger(), inmemory.NewStore(), withTpmSignerCreator(tpmSignerCreatorMock))
require.NoError(t, err)

// force the runner to think the machine has a TPM
// not using usual detection methods since were
// mocking it
tpmRunner.machineHasTpm.Store(true)

tpmSignerCreatorMock.On("CreateKey").Return(nil, nil, errors.New("not available yet")).Once()
require.Nil(t, tpmRunner.Public())

Expand Down Expand Up @@ -66,6 +71,11 @@ func Test_tpmRunner(t *testing.T) {
tpmRunner, err := New(context.TODO(), multislogger.NewNopLogger(), store, withTpmSignerCreator(tpmSignerCreatorMock))
require.NoError(t, err)

// force the runner to think the machine has a TPM
// not using usual detection methods since were
// mocking it
tpmRunner.machineHasTpm.Store(true)

tpmSignerCreatorMock.On("New", fakePrivData, fakePubData).Return(privKey, nil).Once()

// the call to public should load the key from the store and signer creator should not be called any more after
Expand All @@ -88,6 +98,11 @@ func Test_tpmRunner(t *testing.T) {
tpmRunner, err := New(context.TODO(), multislogger.NewNopLogger(), inmemory.NewStore(), withTpmSignerCreator(tpmSignerCreatorMock))
require.NoError(t, err)

// force the runner to think the machine has a TPM
// not using usual detection methods since were
// mocking it
tpmRunner.machineHasTpm.Store(true)

tpmSignerCreatorMock.On("CreateKey").Return(nil, nil, errors.New("not available yet")).Once()
require.Nil(t, tpmRunner.Public())

Expand Down
22 changes: 20 additions & 2 deletions ee/tpmrunner/tpmrunner_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,27 @@ package tpmrunner
import (
"errors"

"github.com/google/go-tpm/tpm2"
"github.com/google/go-tpm/tpmutil/tbs"
)

func isTPMNotFoundErr(err error) bool {
return errors.Is(err, tbs.ErrTPMNotFound)
var terminalErrors = []error{
tbs.ErrTPMNotFound,

// this covers the error "integrity check failed" we dont
// believe a machine will recover from this
tpm2.Error{
Code: tpm2.RCIntegrity,
},
}

// isTerminalTPMError returns true if we should stop trying to use the TPM.
func isTerminalTPMError(err error) bool {
for _, e := range terminalErrors {
if errors.Is(err, e) {
return true
}
}

return false
}
36 changes: 35 additions & 1 deletion ee/tpmrunner/tpmrunner_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"
"time"

"github.com/google/go-tpm/tpm2"
"github.com/google/go-tpm/tpmutil/tbs"
"github.com/kolide/launcher/ee/agent/storage/inmemory"
"github.com/kolide/launcher/ee/tpmrunner/mocks"
Expand Down Expand Up @@ -41,7 +42,7 @@ func Test_tpmRunner_windows(t *testing.T) {
require.Nil(t, tpmRunner.Public())
})

t.Run("handles no tpm in Public() call", func(t *testing.T) {
t.Run("handles terminal errors Public() call", func(t *testing.T) {
t.Parallel()

tpmSignerCreatorMock := mocks.NewTpmSignerCreator(t)
Expand All @@ -63,5 +64,38 @@ func Test_tpmRunner_windows(t *testing.T) {
require.NoError(t, tpmRunner.Execute())
require.Nil(t, tpmRunner.Public())
})
}

func Test_isTerminalError(t *testing.T) {
t.Parallel()

tests := []struct {
name string
err error
expected bool
}{
{
name: "tpm not found err",
err: tbs.ErrTPMNotFound,
expected: true,
},
{
name: "integrity check failed",
err: tpm2.Error{Code: tpm2.RCIntegrity},
expected: true,
},
{
name: "is not terminal error",
err: errors.New("not terminal"),
expected: false,
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.expected, isTerminalTPMError(tt.err))
})
}
}

0 comments on commit 12f4c62

Please sign in to comment.