ssh-tpm-add: add -c flag for per-use confirmation

agent/agent.go

@@ -18,6 +18,7 @@ import (
 	"time"
 
 	keyfile "github.com/foxboron/go-tpm-keyfiles"
+	"github.com/foxboron/ssh-tpm-agent/askpass"
 	"github.com/foxboron/ssh-tpm-agent/internal/keyring"
 	"github.com/foxboron/ssh-tpm-agent/key"
 	"github.com/foxboron/ssh-tpm-agent/utils"
@@ -32,6 +33,7 @@ import (
 var (
 	ErrOperationUnsupported = errors.New("operation unsupported")
 	ErrNoMatchPrivateKeys   = errors.New("no private keys match the requested public key")
+	ErrUserDeniedConfirm    = errors.New("agent: user declined key confirmation")
 )
 
 var SSH_TPM_AGENT_ADD = "tpm-add-key"
@@ -198,6 +200,9 @@ func (a *Agent) SignWithFlags(key ssh.PublicKey, data []byte, flags agent.Signat
 		if !bytes.Equal(s.PublicKey().Marshal(), wantKey) {
 			continue
 		}
+		if err := a.confirmKeyUse(wantKey); err != nil {
+			return nil, err
+		}
 		return s.(ssh.AlgorithmSigner).SignWithAlgorithm(rand.Reader, data, alg)
 	}
 
@@ -224,6 +229,42 @@ func (a *Agent) Sign(key ssh.PublicKey, data []byte) (*ssh.Signature, error) {
 	return a.SignWithFlags(key, data, 0)
 }
 
+// confirmKeyUse prompts the user via SSH_ASKPASS when a key was added with
+// the confirm constraint (ssh-tpm-add -c). Mirrors ssh-agent(1) behaviour.
+// wantKey must be the marshalled public key with any certificate wrapper
+// already stripped, matching the comparison done in SignWithFlags.
+func (a *Agent) confirmKeyUse(wantKey []byte) error {
+	for _, k := range a.keys {
+		// AgentKey() may return a certificate; unwrap it so that a key
+		// added both plain and as a cert is covered by either entry's
+		// confirm constraint.
+		blob := k.AgentKey().Marshal()
+		if pub, err := ssh.ParsePublicKey(blob); err == nil {
+			if cert, ok := pub.(*ssh.Certificate); ok {
+				blob = cert.Key.Marshal()
+			}
+		}
+		if !bytes.Equal(blob, wantKey) {
+			continue
+		}
+		if !k.GetConfirmBeforeUse() {
+			continue
+		}
+		prompt := fmt.Sprintf("Allow use of key %s?\nKey fingerprint %s.",
+			k.GetDescription(), k.Fingerprint())
+		ok, err := askpass.AskPermission(prompt)
+		if err != nil {
+			slog.Info("askpass confirmation failed", slog.String("error", err.Error()))
+			return ErrUserDeniedConfirm
+		}
+		if !ok {
+			return ErrUserDeniedConfirm
+		}
+		return nil
+	}
+	return nil
+}
+
 func (a *Agent) serveConn(c net.Conn) {
 	if err := agent.ServeAgent(a, c); err != io.EOF {
 		slog.Info("Agent client connection ended unsuccessfully", slog.String("error", err.Error()))

agent/client.go

@@ -80,6 +80,12 @@ func ParseTPMKeyMsg(req []byte) (*key.SSHTPMKey, error) {
 		}
 	}
 
+	if tpmkey != nil && len(k.Constraints) != 0 {
+		if err := setConstraints(tpmkey, k.Constraints); err != nil {
+			return nil, err
+		}
+	}
+
 	if len(k.CertBytes) != 0 {
 		pubKey, err := ssh.ParsePublicKey(k.CertBytes)
 		if err != nil {

agent/gocrypto.go

@@ -2,6 +2,13 @@ package agent
 
 // Code taken from crypto/x/ssh/agent
 
+import (
+	"encoding/binary"
+	"fmt"
+
+	"github.com/foxboron/ssh-tpm-agent/key"
+)
+
 const (
 	// 3.7 Key constraint identifiers
 	agentConstrainLifetime = 1
@@ -27,40 +34,32 @@ type constrainLifetimeAgentMsg struct {
 	LifetimeSecs uint32 `sshtype:"1"`
 }
 
-// func parseConstraints(constraints []byte) (lifetimeSecs uint32, confirmBeforeUse bool, extensions []sshagent.ConstraintExtension, err error) {
-// 	for len(constraints) != 0 {
-// 		switch constraints[0] {
-// 		case agentConstrainLifetime:
-// 			lifetimeSecs = binary.BigEndian.Uint32(constraints[1:5])
-// 			constraints = constraints[5:]
-// 		case agentConstrainConfirm:
-// 			confirmBeforeUse = true
-// 			constraints = constraints[1:]
-// 		// case agentConstrainExtension, agentConstrainExtensionV00:
-// 		// 	var msg constrainExtensionAgentMsg
-// 		// 	if err = ssh.Unmarshal(constraints, &msg); err != nil {
-// 		// 		return 0, false, nil, err
-// 		// 	}
-// 		// 	extensions = append(extensions, sshagent.ConstraintExtension{
-// 		// 		ExtensionName:    msg.ExtensionName,
-// 		// 		ExtensionDetails: msg.ExtensionDetails,
-// 		// 	})
-// 		// 	constraints = msg.Rest
-// 		default:
-// 			return 0, false, nil, fmt.Errorf("unknown constraint type: %d", constraints[0])
-// 		}
-// 	}
-// 	return
-// }
+func parseConstraints(constraints []byte) (lifetimeSecs uint32, confirmBeforeUse bool, err error) {
+	for len(constraints) != 0 {
+		switch constraints[0] {
+		case agentConstrainLifetime:
+			if len(constraints) < 5 {
+				return 0, false, fmt.Errorf("truncated lifetime constraint")
+			}
+			lifetimeSecs = binary.BigEndian.Uint32(constraints[1:5])
+			constraints = constraints[5:]
+		case agentConstrainConfirm:
+			confirmBeforeUse = true
+			constraints = constraints[1:]
+		default:
+			return 0, false, fmt.Errorf("unknown constraint type: %d", constraints[0])
+		}
+	}
+	return
+}
 
-// func setConstraints(key *key.SSHTPMKey, constraintBytes []byte) error {
-// 	lifetimeSecs, confirmBeforeUse, constraintExtensions, err := parseConstraints(constraintBytes)
-// 	if err != nil {
-// 		return err
-// 	}
+func setConstraints(k *key.SSHTPMKey, constraintBytes []byte) error {
+	_, confirmBeforeUse, err := parseConstraints(constraintBytes)
+	if err != nil {
+		return err
+	}
 
-// 	key.LifetimeSecs = lifetimeSecs
-// 	key.ConfirmBeforeUse = confirmBeforeUse
-// 	key.ConstraintExtensions = constraintExtensions
-// 	return nil
-// }
+	// LifetimeSecs is not yet supported by ssh-tpm-agent
+	k.ConfirmBeforeUse = confirmBeforeUse
+	return nil
+}

agent/gocrypto_test.go

@@ -0,0 +1,52 @@
+package agent
+
+import (
+	"testing"
+
+	"github.com/foxboron/ssh-tpm-agent/key"
+	"github.com/google/go-tpm/tpm2"
+	"github.com/google/go-tpm/tpm2/transport/simulator"
+	sshagent "golang.org/x/crypto/ssh/agent"
+)
+
+// Verify that ConfirmBeforeUse survives the Marshal/Parse round-trip used
+// between ssh-tpm-add -c and the agent's tpm-add-key extension. This is the
+// regression guard for the -c flag: if someone refactors ParseTPMKeyMsg and
+// drops the setConstraints call, this fails.
+func TestTPMKeyMsgConfirmRoundTrip(t *testing.T) {
+	tpm, err := simulator.OpenSimulator()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer tpm.Close()
+
+	k, err := key.NewSSHTPMKey(tpm, tpm2.TPMAlgECC, 256, []byte(""))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tc := range []struct {
+		name    string
+		confirm bool
+	}{
+		{"no constraint", false},
+		{"confirm constraint", true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			msg := MarshalTPMKeyMsg(&sshagent.AddedKey{
+				PrivateKey:       k,
+				Comment:          "test",
+				ConfirmBeforeUse: tc.confirm,
+			})
+
+			parsed, err := ParseTPMKeyMsg(msg)
+			if err != nil {
+				t.Fatalf("ParseTPMKeyMsg: %v", err)
+			}
+			if parsed.GetConfirmBeforeUse() != tc.confirm {
+				t.Fatalf("ConfirmBeforeUse: got %v want %v",
+					parsed.GetConfirmBeforeUse(), tc.confirm)
+			}
+		})
+	}
+}

askpass/askpass.go

@@ -144,10 +144,14 @@ func SshAskPass(prompt, hint string) ([]byte, error) {
 		}
 	}
 
+	cmd := exec.Command(askpass, prompt)
 	if hint != "" {
-		os.Setenv("SSH_ASKPASS_PROMPT", hint)
+		// Set SSH_ASKPASS_PROMPT only on the subprocess. os.Setenv() would
+		// leak into subsequent passphrase prompts from the same agent
+		// process, causing them to be treated as confirm dialogs.
+		cmd.Env = append(os.Environ(), "SSH_ASKPASS_PROMPT="+hint)
 	}
-	out, err := exec.Command(askpass, prompt).Output()
+	out, err := cmd.Output()
 	switch hint {
 	case "confirm":
 		// TODO: Ugly and needs a rework
@@ -167,10 +171,10 @@ func SshAskPass(prompt, hint string) ([]byte, error) {
 	return bytes.TrimSpace(out), nil
 }
 
-// AskPremission runs SSH_ASKPASS in with SSH_ASKPASS_PROMPT=confirm set as env
-// it will expect exit code 0 or !0 and return 'yes' and 'no' respectively.
-func AskPermission() (bool, error) {
-	a, err := ReadPassphrase("Confirm touch", RP_USE_ASKPASS|RP_ASK_PERMISSION)
+// AskPermission runs SSH_ASKPASS with SSH_ASKPASS_PROMPT=confirm set as env.
+// It will expect exit code 0 or !0 and return true and false respectively.
+func AskPermission(prompt string) (bool, error) {
+	a, err := ReadPassphrase(prompt, RP_USE_ASKPASS|RP_ASK_PERMISSION)
 	if err != nil {
 		return false, err
 	}

cmd/ssh-tpm-add/main.go

@@ -24,9 +24,13 @@ import (
 var Version string
 
 const usage = `Usage:
-    ssh-tpm-add [FILE ...]
+    ssh-tpm-add [-c] [FILE ...]
     ssh-tpm-add --ca [URL] --user [USER] --host [HOSTNAME]
 
+Options:
+    -c                     Require confirmation via SSH_ASKPASS before each
+                           use of the key for signing.
+
 Options for CA provisioning:
     --ca URL               URL to the CA authority for CA key provisioning.
     --user USER            Username of the ssh server user.
@@ -44,10 +48,12 @@ func main() {
 	}
 
 	var caURL, host, user string
+	var confirm bool
 
 	flag.StringVar(&caURL, "ca", "", "ca authority")
 	flag.StringVar(&host, "host", "", "ssh hot")
 	flag.StringVar(&user, "user", "", "remote ssh user")
+	flag.BoolVar(&confirm, "c", false, "require confirmation before each use")
 	flag.Parse()
 
 	socket := utils.EnvSocketPath("")
@@ -60,15 +66,15 @@ func main() {
 
 	var ignorefile bool
 	var paths []string
-	if len(os.Args) == 1 {
+	if flag.NArg() == 0 && caURL == "" {
 		sshdir := utils.SSHDir()
 		paths = []string{
 			fmt.Sprintf("%s/id_ecdsa.tpm", sshdir),
 			fmt.Sprintf("%s/id_rsa.tpm", sshdir),
 		}
 		ignorefile = true
-	} else if len(os.Args) != 1 {
-		paths = os.Args[1:]
+	} else {
+		paths = flag.Args()
 	}
 
 	lsm.RestrictAdditionalPaths(
@@ -101,9 +107,10 @@ func main() {
 
 		sshagentclient := sshagent.NewClient(conn)
 		addedkey := sshagent.AddedKey{
-			PrivateKey:  k,
-			Comment:     k.Description,
-			Certificate: cert,
+			PrivateKey:       k,
+			Comment:          k.Description,
+			Certificate:      cert,
+			ConfirmBeforeUse: confirm,
 		}
 
 		_, err = sshagentclient.Extension(agent.SSH_TPM_AGENT_ADD, agent.MarshalTPMKeyMsg(&addedkey))
@@ -132,13 +139,17 @@ func main() {
 
 		if _, err = client.Extension(agent.SSH_TPM_AGENT_ADD, agent.MarshalTPMKeyMsg(
 			&sshagent.AddedKey{
-				PrivateKey: k,
-				Comment:    k.Description,
+				PrivateKey:       k,
+				Comment:          k.Description,
+				ConfirmBeforeUse: confirm,
 			},
 		)); err != nil {
 			log.Fatal(err)
 		}
 		fmt.Printf("Identity added: %s (%s)\n", path, k.Description)
+		if confirm {
+			fmt.Printf("The user must confirm each use of the key\n")
+		}
 
 		certStr := fmt.Sprintf("%s-cert.pub", strings.TrimSuffix(path, filepath.Ext(path)))
 		if _, err := os.Stat(certStr); !errors.Is(err, os.ErrNotExist) {
@@ -157,9 +168,10 @@ func main() {
 			}
 			if _, err = client.Extension(agent.SSH_TPM_AGENT_ADD, agent.MarshalTPMKeyMsg(
 				&sshagent.AddedKey{
-					PrivateKey:  k,
-					Certificate: cert,
-					Comment:     k.Description,
+					PrivateKey:       k,
+					Certificate:      cert,
+					Comment:          k.Description,
+					ConfirmBeforeUse: confirm,
 				},
 			)); err != nil {
 				log.Fatal(err)

cmd/ssh-tpm-agent/testdata/script/agent_confirm.txt

@@ -0,0 +1,38 @@
+chmod 755 askpass-test
+env SSH_ASKPASS=./askpass-test
+env SSH_ASKPASS_REQUIRE=force
+
+exec ssh-tpm-agent -d --no-load &agent&
+exec ssh-tpm-keygen -N ''
+
+# Add with -c: every signature requires confirmation via SSH_ASKPASS
+exec ssh-tpm-add -c
+
+# Approve: askpass exits 0, signing succeeds, prompt+hint are recorded
+exec ssh-keygen -Y sign -n file -f .ssh/id_ecdsa.pub file_to_sign.txt
+grep '^confirm Allow use of key' askpass.log
+rm askpass.log file_to_sign.txt.sig
+
+# Deny: sentinel file makes askpass exit 1, signing fails
+cp empty askpass.deny
+! exec ssh-keygen -Y sign -n file -f .ssh/id_ecdsa.pub file_to_sign.txt
+! exists file_to_sign.txt.sig
+grep '^confirm Allow use of key' askpass.log
+rm askpass.log askpass.deny
+
+# Re-add without -c: askpass must not be consulted even if it would deny
+exec ssh-add -D
+exec ssh-tpm-add
+cp empty askpass.deny
+exec ssh-keygen -Y sign -n file -f .ssh/id_ecdsa.pub file_to_sign.txt
+! exists askpass.log
+
+-- file_to_sign.txt --
+Hello World
+-- empty --
+-- askpass-test --
+#!/bin/sh
+# Log "<SSH_ASKPASS_PROMPT> <prompt>" and deny if the sentinel exists.
+# Uses a file because the background agent does not see later env changes.
+printf '%s %s\n' "$SSH_ASKPASS_PROMPT" "$1" >> askpass.log
+[ ! -e askpass.deny ]