-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for collecitng multiple user keys on via secure enclave #1644
Changes from 13 commits
bb7805d
d41f914
b1cf533
5adb031
78a0cd4
4ee293d
f1ebb6f
6ba5fcb
f730fb2
2a6e787
9a0a3e9
bc81209
5f22b6f
721794d
022537e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
//go:build darwin | ||
// +build darwin | ||
|
||
package main | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"os" | ||
|
||
"github.com/kolide/krypto/pkg/echelper" | ||
"github.com/kolide/krypto/pkg/secureenclave" | ||
"github.com/kolide/launcher/ee/secureenclavesigner" | ||
) | ||
|
||
// runSecureEnclave performs either a create-key operation using the secure enclave. | ||
// It's available as a separate command because launcher runs as root by default and since it's | ||
// not in a user security context, it can't use the secure enclave directly. However, this command | ||
// can be run in the user context using launchctl. | ||
func runSecureEnclave(args []string) error { | ||
// currently we are just creating key, but plan to add sign command in future | ||
if len(args) < 1 { | ||
return errors.New("not enough arguments, expect create_key") | ||
} | ||
|
||
switch args[0] { | ||
case secureenclavesigner.CreateKeyCmd: | ||
return createSecureEnclaveKey() | ||
|
||
default: | ||
return fmt.Errorf("unknown command %s", args[0]) | ||
} | ||
} | ||
|
||
func createSecureEnclaveKey() error { | ||
secureEnclavePubKey, err := secureenclave.CreateKey() | ||
if err != nil { | ||
return fmt.Errorf("creating secure enclave key: %w", err) | ||
} | ||
|
||
secureEnclavePubDer, err := echelper.PublicEcdsaToB64Der(secureEnclavePubKey) | ||
if err != nil { | ||
return fmt.Errorf("marshalling public key to der: %w", err) | ||
} | ||
|
||
os.Stdout.Write(secureEnclavePubDer) | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
//go:build !darwin | ||
// +build !darwin | ||
|
||
package main | ||
|
||
import "errors" | ||
|
||
func runSecureEnclave(args []string) error { | ||
return errors.New("not implemented on non darwin platforms") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
//go:build darwin | ||
// +build darwin | ||
|
||
package main | ||
|
||
import ( | ||
"bytes" | ||
"context" | ||
"fmt" | ||
"os" | ||
"os/exec" | ||
"path/filepath" | ||
"testing" | ||
"time" | ||
|
||
"github.com/kolide/krypto/pkg/echelper" | ||
"github.com/kolide/launcher/ee/secureenclavesigner" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
const ( | ||
testWrappedEnvVarKey = "SECURE_ENCLAVE_TEST_WRAPPED" | ||
macOsAppResourceDir = "../../ee/secureenclavesigner/test_app_resources" | ||
) | ||
|
||
// TestSecureEnclaveTestRunner creates a MacOS app with the binary of this packages tests, then signs the app with entitlements and runs the tests. | ||
// This is done because in order to access secure enclave to run tests, we need MacOS entitlements. | ||
// #nosec G306 -- Need readable files | ||
func TestSecureEnclaveTestRunner(t *testing.T) { | ||
directionless marked this conversation as resolved.
Show resolved
Hide resolved
|
||
t.Parallel() | ||
|
||
if os.Getenv("CI") != "" { | ||
t.Skipf("\nskipping because %s env var was not empty, this is being run in a CI environment without access to secure enclave", testWrappedEnvVarKey) | ||
} | ||
|
||
if os.Getenv(testWrappedEnvVarKey) != "" { | ||
t.Skipf("\nskipping because %s env var was not empty, this is the execution of the codesigned app with entitlements", testWrappedEnvVarKey) | ||
} | ||
|
||
t.Log("\nexecuting wrapped tests with codesigned app and entitlements") | ||
|
||
// set up app bundle | ||
rootDir := t.TempDir() | ||
appRoot := filepath.Join(rootDir, "launcher_test.app") | ||
|
||
// make required dirs launcher_test.app/Contents/MacOS and add files | ||
require.NoError(t, os.MkdirAll(filepath.Join(appRoot, "Contents", "MacOS"), 0700)) | ||
copyFile(t, filepath.Join(macOsAppResourceDir, "Info.plist"), filepath.Join(appRoot, "Contents", "Info.plist")) | ||
copyFile(t, filepath.Join(macOsAppResourceDir, "embedded.provisionprofile"), filepath.Join(appRoot, "Contents", "embedded.provisionprofile")) | ||
|
||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
defer cancel() | ||
|
||
// build an executable containing the tests into the app bundle | ||
executablePath := filepath.Join(appRoot, "Contents", "MacOS", "launcher_test") | ||
out, err := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd | ||
ctx, | ||
"go", | ||
"test", | ||
"-c", | ||
"--cover", | ||
"--race", | ||
"./", | ||
"-o", | ||
executablePath, | ||
).CombinedOutput() | ||
|
||
require.NoError(t, ctx.Err()) | ||
require.NoError(t, err, string(out)) | ||
|
||
// sign app bundle | ||
signApp(t, appRoot) | ||
|
||
// run app bundle executable | ||
cmd := exec.CommandContext(ctx, executablePath, "-test.v") //nolint:forbidigo // Only used in test, don't want as standard allowedcmd | ||
cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", testWrappedEnvVarKey, "true")) | ||
out, err = cmd.CombinedOutput() | ||
require.NoError(t, ctx.Err()) | ||
require.NoError(t, err, string(out)) | ||
|
||
// ensure the test ran successfully | ||
require.Contains(t, string(out), "PASS: TestSecureEnclaveCmd") | ||
require.NotContains(t, string(out), "FAIL") | ||
} | ||
|
||
func TestSecureEnclaveCmd(t *testing.T) { //nolint:paralleltest | ||
if os.Getenv(testWrappedEnvVarKey) == "" { | ||
t.Skipf("\nskipping because %s env var was empty, test not being run from codesigned app with entitlements", testWrappedEnvVarKey) | ||
} | ||
|
||
t.Log("\nrunning wrapped tests with codesigned app and entitlements") | ||
|
||
oldStdout := os.Stdout | ||
defer func() { | ||
os.Stdout = oldStdout | ||
}() | ||
|
||
// create a pipe to capture stdout | ||
pipeReader, pipeWriter, err := os.Pipe() | ||
require.NoError(t, err) | ||
|
||
os.Stdout = pipeWriter | ||
|
||
require.NoError(t, runSecureEnclave([]string{secureenclavesigner.CreateKeyCmd})) | ||
require.NoError(t, pipeWriter.Close()) | ||
|
||
var buf bytes.Buffer | ||
_, err = buf.ReadFrom(pipeReader) | ||
require.NoError(t, err) | ||
|
||
// convert response to public key | ||
createKeyResponse := buf.Bytes() | ||
secureEnclavePubKey, err := echelper.PublicB64DerToEcdsaKey(createKeyResponse) | ||
require.NoError(t, err) | ||
require.NotNil(t, secureEnclavePubKey, "should be able to get public key") | ||
} | ||
|
||
// #nosec G306 -- Need readable files | ||
func copyFile(t *testing.T, source, destination string) { | ||
bytes, err := os.ReadFile(source) | ||
require.NoError(t, err) | ||
require.NoError(t, os.WriteFile(destination, bytes, 0700)) | ||
} | ||
|
||
// #nosec G204 -- This triggers due to using env var in cmd, making exception for test | ||
func signApp(t *testing.T, appRootDir string) { | ||
codeSignId := os.Getenv("MACOS_CODESIGN_IDENTITY") | ||
require.NotEmpty(t, codeSignId, "need MACOS_CODESIGN_IDENTITY env var to sign app, such as [Mac Developer: Jane Doe (ABCD123456)]") | ||
|
||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
defer cancel() | ||
|
||
cmd := exec.CommandContext( //nolint:forbidigo // Only used in test, don't want as standard allowedcmd | ||
ctx, | ||
"codesign", | ||
"--deep", | ||
"--force", | ||
"--options", "runtime", | ||
"--entitlements", filepath.Join(macOsAppResourceDir, "entitlements"), | ||
"--sign", codeSignId, | ||
"--timestamp", | ||
appRootDir, | ||
) | ||
|
||
out, err := cmd.CombinedOutput() | ||
require.NoError(t, ctx.Err()) | ||
require.NoError(t, err, string(out)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,13 @@ import ( | |
|
||
"github.com/kolide/krypto/pkg/tpm" | ||
"github.com/kolide/launcher/ee/agent/types" | ||
"github.com/kolide/launcher/pkg/traces" | ||
) | ||
|
||
// nolint:unused | ||
func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { | ||
func setupHardwareKeys(ctx context.Context, slogger *slog.Logger, store types.GetterSetterDeleter) (keyInt, error) { | ||
_, span := traces.StartSpan(ctx) | ||
defer span.End() | ||
|
||
priData, pubData, err := fetchKeyData(store) | ||
if err != nil { | ||
return nil, err | ||
|
@@ -28,17 +31,24 @@ func setupHardwareKeys(slogger *slog.Logger, store types.GetterSetterDeleter) (k | |
priData, pubData, err = tpm.CreateKey() | ||
if err != nil { | ||
clearKeyData(slogger, store) | ||
traces.SetError(span, fmt.Errorf("creating key: %w", err)) | ||
return nil, fmt.Errorf("creating key: %w", err) | ||
} | ||
|
||
span.AddEvent("new_key_created") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need this? It's somewhat implied by the function name. I guess we're still learning the tools There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like seeing the events when I look at a span, but I can live without them. |
||
|
||
if err := storeKeyData(store, priData, pubData); err != nil { | ||
clearKeyData(slogger, store) | ||
traces.SetError(span, fmt.Errorf("storing key: %w", err)) | ||
return nil, fmt.Errorf("storing key: %w", err) | ||
} | ||
|
||
span.AddEvent("new_key_stored") | ||
} | ||
|
||
k, err := tpm.New(priData, pubData) | ||
if err != nil { | ||
traces.SetError(span, fmt.Errorf("creating tpm signer: from new key: %w", err)) | ||
return nil, fmt.Errorf("creating tpm signer: from new key: %w", err) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot decide, but we could try to examine the parent process to see if launcher is in there. Though the parent is just going to be sudo, so this may not be feasible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Early on we considered trying to do this, but decided not to since it was easy to spoof ppid. If we're feeling different now and we do go this route, we could even exec codesign and verify that the calling binary is signed as we expect.