Skip to content
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 secure enclave signatures to local server response #1658

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
144 changes: 143 additions & 1 deletion cmd/launcher/secure_enclave_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,45 @@
package main

import (
"crypto"
"crypto/ecdsa"
"crypto/rand"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"os"
"time"

"github.com/kolide/kit/ulid"
"github.com/kolide/krypto/pkg/challenge"
"github.com/kolide/krypto/pkg/echelper"
"github.com/kolide/krypto/pkg/secureenclave"
"github.com/kolide/launcher/ee/agent/certs"
"github.com/kolide/launcher/ee/secureenclavesigner"
)

const secureEnclaveTimestampValiditySeconds = 10

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 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")
return errors.New("not enough arguments, expect create_key | sign")
}

switch args[0] {
case secureenclavesigner.CreateKeyCmd:
return createSecureEnclaveKey()
case secureenclavesigner.SignCmd:
if len(args) < 2 {
return errors.New("not enough arguments for sign command, expect sign <data>")
}

return signWithSecureEnclave(args[1])
default:
return fmt.Errorf("unknown command %s", args[0])
}
Expand All @@ -46,3 +62,129 @@ func createSecureEnclaveKey() error {
os.Stdout.Write(secureEnclavePubDer)
return nil
}

func signWithSecureEnclave(signRequestB64 string) error {
b, err := base64.StdEncoding.DecodeString(signRequestB64)
if err != nil {
return fmt.Errorf("decoding b64 sign request: %w", err)
}

var signRequest secureenclavesigner.SignRequest
if err := json.Unmarshal(b, &signRequest); err != nil {
return fmt.Errorf("unmarshaling msgpack sign request: %w", err)
}

if err := verifySecureEnclaveChallenge(signRequest); err != nil {
return fmt.Errorf("verifying challenge: %w", err)
}

userPubKey, err := echelper.PublicB64DerToEcdsaKey(signRequest.UserPubkey)
if err != nil {
return fmt.Errorf("marshalling b64 der to public key: %w", err)
}

seSigner, err := secureenclave.New(userPubKey)
if err != nil {
return fmt.Errorf("creating secure enclave cmd signer: %w", err)
}

// tag the ends of the data to sign, this is intended to ensure that launcher wont
// sign arbitrary things, any party verifying the signature will need to
// handle these tags
dataToSign := []byte(fmt.Sprintf("kolide:%s:kolide", signRequest.Data))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe I'm missing something (I will revisit once you've got a diagram available) -- why do we add kolide: + :kolide on either side of the data at this point? Shouldn't whatever is calling this command have already put kolide on either side so that we can validate that here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a PR for a doc here https://github.com/kolide/monorepo/pull/166.

We have to do it at this point because there is no way to ensure that the process which is execing launcher in the user context adds the tags.

The inability for a process to confidently detect it's parent process is the primary driver for all this tag hoopla.


signResponseInner := secureenclavesigner.SignResponseInner{
Nonce: fmt.Sprintf("%s%s", signRequest.BaseNonce, ulid.New()),
Timestamp: time.Now().UTC().Unix(),
Data: dataToSign,
}

innerResponseBytes, err := json.Marshal(signResponseInner)
if err != nil {
return fmt.Errorf("marshalling inner response: %w", err)
}

digest, err := echelper.HashForSignature(innerResponseBytes)
if err != nil {
return fmt.Errorf("hashing data for signature: %w", err)
}

sig, err := seSigner.Sign(rand.Reader, digest, crypto.SHA256)
if err != nil {
return fmt.Errorf("signing request: %w", err)
}

outerResponseBytes, err := json.Marshal(secureenclavesigner.SignResponseOuter{
Msg: innerResponseBytes,
Sig: sig,
})

if err != nil {
return fmt.Errorf("marshalling outer response: %w", err)
}

os.Stdout.Write([]byte(base64.StdEncoding.EncodeToString(outerResponseBytes)))
return nil
}

func verifySecureEnclaveChallenge(signRequest secureenclavesigner.SignRequest) error {
challengeUnmarshalled, err := challenge.UnmarshalChallenge(signRequest.Challenge)
if err != nil {
return fmt.Errorf("unmarshaling challenge: %w", err)
}

serverPubKey, err := loadSecureEnclaveServerPubKey(string(signRequest.ServerPubKey))
if err != nil {
return fmt.Errorf("loading server public key: %w", err)
}

if err := challengeUnmarshalled.Verify(*serverPubKey); err != nil {
return fmt.Errorf("verifying challenge: %w", err)
}

// Check the timestamp, this prevents people from saving a challenge and then
// reusing it a bunch. However, it will fail if the clocks are too far out of sync.
timestampDelta := time.Now().Unix() - challengeUnmarshalled.Timestamp()
if timestampDelta > secureEnclaveTimestampValiditySeconds || timestampDelta < -secureEnclaveTimestampValiditySeconds {
return fmt.Errorf("timestamp delta %d is outside of validity range %d", timestampDelta, secureEnclaveTimestampValiditySeconds)
}

return nil
}

func loadSecureEnclaveServerPubKey(b64Key string) (*ecdsa.PublicKey, error) {
providedKey, err := echelper.PublicB64DerToEcdsaKey([]byte(b64Key))
if err != nil {
return nil, fmt.Errorf("parsing provided server public key: %w", err)
}

if secureenclavesigner.Undertest {
if secureenclavesigner.TestServerPubKey == "" {
return nil, errors.New("test server public key not set")
}

k, err := echelper.PublicB64DerToEcdsaKey([]byte(secureenclavesigner.TestServerPubKey))
if err != nil {
return nil, fmt.Errorf("parsing test server public key: %w", err)
}

if !providedKey.Equal(k) {
return nil, errors.New("provided server public key does not match test server public key")
}

return k, nil
}

for _, serverKey := range []string{certs.K2EccServerCert, certs.ReviewEccServerCert, certs.LocalhostEccServerCert} {
k, err := echelper.PublicPemToEcdsaKey([]byte(serverKey))
if err != nil {
continue
}

if providedKey.Equal(k) {
return k, nil
}
}

return nil, errors.New("provided server public key does not match any known server public key")
}
153 changes: 0 additions & 153 deletions cmd/launcher/secure_enclave_test.go

This file was deleted.

14 changes: 7 additions & 7 deletions ee/localserver/certs.go → ee/agent/certs/certs.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package localserver
package certs

// These are the hardcoded certificates
const (
k2RsaServerCert = `-----BEGIN PUBLIC KEY-----
K2RsaServerCert = `-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAkeNJgRkJOow7LovGmrlW
1UzHkifTKQV1/8kX+p2MPLptGgPKlqpLnhZsGOhpHpswlUalgSZPyhBfM9Btdmps
QZ2PkZkgEiy62PleVSBeBtpGcwHibHTGamzmKVrji9GudAvU+qapfPGnr//275/1
Expand All @@ -13,7 +13,7 @@ msGeD7hPhtdB/h0O8eBWIiOQ6fH7exl71UfGTR6pYQmJMK1ZZeT7FeWVSGkswxkV
-----END PUBLIC KEY-----
`

reviewRsaServerCert = `-----BEGIN PUBLIC KEY-----
ReviewRsaServerCert = `-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAr0VjwKya7JNRM8uiPllw
An+W3SBuDkxAToqHxdRX6k2eJSIK0K4oynVIqkrP1MC2ultlIo2ZZhKYQVhQfCej
9RIBFm2wl1/daMNCpmkwu8KbsXDAVrc70yXvpzeAnh6QCnvI1PbCI6icbpVo8Wh1
Expand All @@ -24,7 +24,7 @@ AwIDAQAB
-----END PUBLIC KEY-----
`

localhostRsaServerCert = `-----BEGIN PUBLIC KEY-----
LocalhostRsaServerCert = `-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwXIB33Wvu/rn7WjSQaat
9lafwrrnmbP8NtTlOiY4b4gv/bL6nnyr21s95uKlcm+8WRbJZsch/ahrNYsdDO2Q
QmfZTi7VR7/IhwyISkh/JaaBPmipO/4KfdnKOarah3F619fl4Udd973+5QK0ZQmy
Expand All @@ -35,17 +35,17 @@ Skx1Y1JUHgZL9IVGMAmkJWEKoa4TPopfnr74SwpNDcU7rP86rgSIO597wMeMbnAM
-----END PUBLIC KEY-----
`

k2EccServerCert = `-----BEGIN PUBLIC KEY-----
K2EccServerCert = `-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEmAO4tYINU14/i0LvONs1IXVwaFnF
dNsydDr38XrL29kiFl+vTkp4gVx6172oNSL3KRBQmjMXqWkLNoxXaWS3uQ==
-----END PUBLIC KEY-----`

reviewEccServerCert = `-----BEGIN PUBLIC KEY-----
ReviewEccServerCert = `-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIgYTWPi8N7b0H69tnN543HbjAoLc
GINysvEwYrNoGjASt+nqzlFesagt+2A/4W7JR16nE91mbCHn+HV6x+H8gw==
-----END PUBLIC KEY-----`

localhostEccServerCert = `-----BEGIN PUBLIC KEY-----
LocalhostEccServerCert = `-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEwowFsPUaOC61LAfDz1hLnsuSDfEx
SC4TSfHtbHHv3lx2/Bfu+H0szXYZ75GF/qZ5edobq3UkABN6OaFnnJId3w==
-----END PUBLIC KEY-----`
Expand Down
9 changes: 7 additions & 2 deletions ee/agent/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@ type keyInt interface {
Type() string
}

var hardwareKeys keyInt = keys.Noop
type KeyIntHardware interface {
keyInt
SignConsoleUser(ctx context.Context, challenge, data, serverPubkey []byte, baseNonce string) ([]byte, error)
}

var hardwareKeys KeyIntHardware = keys.Noop
var localDbKeys keyInt = keys.Noop

func HardwareKeys() keyInt {
func HardwareKeys() KeyIntHardware {
return hardwareKeys
}

Expand Down
Loading
Loading