Skip to content

Commit a3cde87

Browse files
FoxboronFoxboron
committed
Merge remote-tracking branch 'origin/pull/114'
* origin/pull/114: cmd: add end-to-end testscript for ssh-tpm-add -c ssh-tpm-add: add -c flag for per-use confirmation agent: plumb ConfirmBeforeUse constraint through TPMKeyMsg askpass: do not leak SSH_ASKPASS_PROMPT into agent process env
2 parents bb52b1c + 5ddd1ef commit a3cde87

9 files changed

Lines changed: 225 additions & 59 deletions

File tree

‎agent/agent.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"time"
1919

2020
keyfile "github.com/foxboron/go-tpm-keyfiles"
21+
"github.com/foxboron/ssh-tpm-agent/askpass"
2122
"github.com/foxboron/ssh-tpm-agent/internal/keyring"
2223
"github.com/foxboron/ssh-tpm-agent/key"
2324
"github.com/foxboron/ssh-tpm-agent/utils"
@@ -32,6 +33,7 @@ import (
3233
var (
3334
ErrOperationUnsupported = errors.New("operation unsupported")
3435
ErrNoMatchPrivateKeys = errors.New("no private keys match the requested public key")
36+
ErrUserDeniedConfirm = errors.New("agent: user declined key confirmation")
3537
)
3638

3739
var SSH_TPM_AGENT_ADD = "tpm-add-key"
@@ -198,6 +200,9 @@ func (a *Agent) SignWithFlags(key ssh.PublicKey, data []byte, flags agent.Signat
198200
if !bytes.Equal(s.PublicKey().Marshal(), wantKey) {
199201
continue
200202
}
203+
if err := a.confirmKeyUse(wantKey); err != nil {
204+
return nil, err
205+
}
201206
return s.(ssh.AlgorithmSigner).SignWithAlgorithm(rand.Reader, data, alg)
202207
}
203208

@@ -224,6 +229,42 @@ func (a *Agent) Sign(key ssh.PublicKey, data []byte) (*ssh.Signature, error) {
224229
return a.SignWithFlags(key, data, 0)
225230
}
226231

232+
// confirmKeyUse prompts the user via SSH_ASKPASS when a key was added with
233+
// the confirm constraint (ssh-tpm-add -c). Mirrors ssh-agent(1) behaviour.
234+
// wantKey must be the marshalled public key with any certificate wrapper
235+
// already stripped, matching the comparison done in SignWithFlags.
236+
func (a *Agent) confirmKeyUse(wantKey []byte) error {
237+
for _, k := range a.keys {
238+
// AgentKey() may return a certificate; unwrap it so that a key
239+
// added both plain and as a cert is covered by either entry's
240+
// confirm constraint.
241+
blob := k.AgentKey().Marshal()
242+
if pub, err := ssh.ParsePublicKey(blob); err == nil {
243+
if cert, ok := pub.(*ssh.Certificate); ok {
244+
blob = cert.Key.Marshal()
245+
}
246+
}
247+
if !bytes.Equal(blob, wantKey) {
248+
continue
249+
}
250+
if !k.GetConfirmBeforeUse() {
251+
continue
252+
}
253+
prompt := fmt.Sprintf("Allow use of key %s?\nKey fingerprint %s.",
254+
k.GetDescription(), k.Fingerprint())
255+
ok, err := askpass.AskPermission(prompt)
256+
if err != nil {
257+
slog.Info("askpass confirmation failed", slog.String("error", err.Error()))
258+
return ErrUserDeniedConfirm
259+
}
260+
if !ok {
261+
return ErrUserDeniedConfirm
262+
}
263+
return nil
264+
}
265+
return nil
266+
}
267+
227268
func (a *Agent) serveConn(c net.Conn) {
228269
if err := agent.ServeAgent(a, c); err != io.EOF {
229270
slog.Info("Agent client connection ended unsuccessfully", slog.String("error", err.Error()))

‎agent/client.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ func ParseTPMKeyMsg(req []byte) (*key.SSHTPMKey, error) {
8080
}
8181
}
8282

83+
if tpmkey != nil && len(k.Constraints) != 0 {
84+
if err := setConstraints(tpmkey, k.Constraints); err != nil {
85+
return nil, err
86+
}
87+
}
88+
8389
if len(k.CertBytes) != 0 {
8490
pubKey, err := ssh.ParsePublicKey(k.CertBytes)
8591
if err != nil {

‎agent/gocrypto.go‎

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@ package agent
22

33
// Code taken from crypto/x/ssh/agent
44

5+
import (
6+
"encoding/binary"
7+
"fmt"
8+
9+
"github.com/foxboron/ssh-tpm-agent/key"
10+
)
11+
512
const (
613
// 3.7 Key constraint identifiers
714
agentConstrainLifetime = 1
@@ -27,40 +34,32 @@ type constrainLifetimeAgentMsg struct {
2734
LifetimeSecs uint32 `sshtype:"1"`
2835
}
2936

30-
// func parseConstraints(constraints []byte) (lifetimeSecs uint32, confirmBeforeUse bool, extensions []sshagent.ConstraintExtension, err error) {
31-
// for len(constraints) != 0 {
32-
// switch constraints[0] {
33-
// case agentConstrainLifetime:
34-
// lifetimeSecs = binary.BigEndian.Uint32(constraints[1:5])
35-
// constraints = constraints[5:]
36-
// case agentConstrainConfirm:
37-
// confirmBeforeUse = true
38-
// constraints = constraints[1:]
39-
// // case agentConstrainExtension, agentConstrainExtensionV00:
40-
// // var msg constrainExtensionAgentMsg
41-
// // if err = ssh.Unmarshal(constraints, &msg); err != nil {
42-
// // return 0, false, nil, err
43-
// // }
44-
// // extensions = append(extensions, sshagent.ConstraintExtension{
45-
// // ExtensionName: msg.ExtensionName,
46-
// // ExtensionDetails: msg.ExtensionDetails,
47-
// // })
48-
// // constraints = msg.Rest
49-
// default:
50-
// return 0, false, nil, fmt.Errorf("unknown constraint type: %d", constraints[0])
51-
// }
52-
// }
53-
// return
54-
// }
37+
func parseConstraints(constraints []byte) (lifetimeSecs uint32, confirmBeforeUse bool, err error) {
38+
for len(constraints) != 0 {
39+
switch constraints[0] {
40+
case agentConstrainLifetime:
41+
if len(constraints) < 5 {
42+
return 0, false, fmt.Errorf("truncated lifetime constraint")
43+
}
44+
lifetimeSecs = binary.BigEndian.Uint32(constraints[1:5])
45+
constraints = constraints[5:]
46+
case agentConstrainConfirm:
47+
confirmBeforeUse = true
48+
constraints = constraints[1:]
49+
default:
50+
return 0, false, fmt.Errorf("unknown constraint type: %d", constraints[0])
51+
}
52+
}
53+
return
54+
}
5555

56-
// func setConstraints(key *key.SSHTPMKey, constraintBytes []byte) error {
57-
// lifetimeSecs, confirmBeforeUse, constraintExtensions, err := parseConstraints(constraintBytes)
58-
// if err != nil {
59-
// return err
60-
// }
56+
func setConstraints(k *key.SSHTPMKey, constraintBytes []byte) error {
57+
_, confirmBeforeUse, err := parseConstraints(constraintBytes)
58+
if err != nil {
59+
return err
60+
}
6161

62-
// key.LifetimeSecs = lifetimeSecs
63-
// key.ConfirmBeforeUse = confirmBeforeUse
64-
// key.ConstraintExtensions = constraintExtensions
65-
// return nil
66-
// }
62+
// LifetimeSecs is not yet supported by ssh-tpm-agent
63+
k.ConfirmBeforeUse = confirmBeforeUse
64+
return nil
65+
}

‎agent/gocrypto_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package agent
2+
3+
import (
4+
"testing"
5+
6+
"github.com/foxboron/ssh-tpm-agent/key"
7+
"github.com/google/go-tpm/tpm2"
8+
"github.com/google/go-tpm/tpm2/transport/simulator"
9+
sshagent "golang.org/x/crypto/ssh/agent"
10+
)
11+
12+
// Verify that ConfirmBeforeUse survives the Marshal/Parse round-trip used
13+
// between ssh-tpm-add -c and the agent's tpm-add-key extension. This is the
14+
// regression guard for the -c flag: if someone refactors ParseTPMKeyMsg and
15+
// drops the setConstraints call, this fails.
16+
func TestTPMKeyMsgConfirmRoundTrip(t *testing.T) {
17+
tpm, err := simulator.OpenSimulator()
18+
if err != nil {
19+
t.Fatal(err)
20+
}
21+
defer tpm.Close()
22+
23+
k, err := key.NewSSHTPMKey(tpm, tpm2.TPMAlgECC, 256, []byte(""))
24+
if err != nil {
25+
t.Fatal(err)
26+
}
27+
28+
for _, tc := range []struct {
29+
name string
30+
confirm bool
31+
}{
32+
{"no constraint", false},
33+
{"confirm constraint", true},
34+
} {
35+
t.Run(tc.name, func(t *testing.T) {
36+
msg := MarshalTPMKeyMsg(&sshagent.AddedKey{
37+
PrivateKey: k,
38+
Comment: "test",
39+
ConfirmBeforeUse: tc.confirm,
40+
})
41+
42+
parsed, err := ParseTPMKeyMsg(msg)
43+
if err != nil {
44+
t.Fatalf("ParseTPMKeyMsg: %v", err)
45+
}
46+
if parsed.GetConfirmBeforeUse() != tc.confirm {
47+
t.Fatalf("ConfirmBeforeUse: got %v want %v",
48+
parsed.GetConfirmBeforeUse(), tc.confirm)
49+
}
50+
})
51+
}
52+
}

‎askpass/askpass.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,14 @@ func SshAskPass(prompt, hint string) ([]byte, error) {
144144
}
145145
}
146146

147+
cmd := exec.Command(askpass, prompt)
147148
if hint != "" {
148-
os.Setenv("SSH_ASKPASS_PROMPT", hint)
149+
// Set SSH_ASKPASS_PROMPT only on the subprocess. os.Setenv() would
150+
// leak into subsequent passphrase prompts from the same agent
151+
// process, causing them to be treated as confirm dialogs.
152+
cmd.Env = append(os.Environ(), "SSH_ASKPASS_PROMPT="+hint)
149153
}
150-
out, err := exec.Command(askpass, prompt).Output()
154+
out, err := cmd.Output()
151155
switch hint {
152156
case "confirm":
153157
// TODO: Ugly and needs a rework
@@ -167,10 +171,10 @@ func SshAskPass(prompt, hint string) ([]byte, error) {
167171
return bytes.TrimSpace(out), nil
168172
}
169173

170-
// AskPremission runs SSH_ASKPASS in with SSH_ASKPASS_PROMPT=confirm set as env
171-
// it will expect exit code 0 or !0 and return 'yes' and 'no' respectively.
172-
func AskPermission() (bool, error) {
173-
a, err := ReadPassphrase("Confirm touch", RP_USE_ASKPASS|RP_ASK_PERMISSION)
174+
// AskPermission runs SSH_ASKPASS with SSH_ASKPASS_PROMPT=confirm set as env.
175+
// It will expect exit code 0 or !0 and return true and false respectively.
176+
func AskPermission(prompt string) (bool, error) {
177+
a, err := ReadPassphrase(prompt, RP_USE_ASKPASS|RP_ASK_PERMISSION)
174178
if err != nil {
175179
return false, err
176180
}

‎cmd/ssh-tpm-add/main.go‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,13 @@ import (
2424
var Version string
2525

2626
const usage = `Usage:
27-
ssh-tpm-add [FILE ...]
27+
ssh-tpm-add [-c] [FILE ...]
2828
ssh-tpm-add --ca [URL] --user [USER] --host [HOSTNAME]
2929
30+
Options:
31+
-c Require confirmation via SSH_ASKPASS before each
32+
use of the key for signing.
33+
3034
Options for CA provisioning:
3135
--ca URL URL to the CA authority for CA key provisioning.
3236
--user USER Username of the ssh server user.
@@ -44,10 +48,12 @@ func main() {
4448
}
4549

4650
var caURL, host, user string
51+
var confirm bool
4752

4853
flag.StringVar(&caURL, "ca", "", "ca authority")
4954
flag.StringVar(&host, "host", "", "ssh hot")
5055
flag.StringVar(&user, "user", "", "remote ssh user")
56+
flag.BoolVar(&confirm, "c", false, "require confirmation before each use")
5157
flag.Parse()
5258

5359
socket := utils.EnvSocketPath("")
@@ -60,15 +66,15 @@ func main() {
6066

6167
var ignorefile bool
6268
var paths []string
63-
if len(os.Args) == 1 {
69+
if flag.NArg() == 0 && caURL == "" {
6470
sshdir := utils.SSHDir()
6571
paths = []string{
6672
fmt.Sprintf("%s/id_ecdsa.tpm", sshdir),
6773
fmt.Sprintf("%s/id_rsa.tpm", sshdir),
6874
}
6975
ignorefile = true
70-
} else if len(os.Args) != 1 {
71-
paths = os.Args[1:]
76+
} else {
77+
paths = flag.Args()
7278
}
7379

7480
lsm.RestrictAdditionalPaths(
@@ -101,9 +107,10 @@ func main() {
101107

102108
sshagentclient := sshagent.NewClient(conn)
103109
addedkey := sshagent.AddedKey{
104-
PrivateKey: k,
105-
Comment: k.Description,
106-
Certificate: cert,
110+
PrivateKey: k,
111+
Comment: k.Description,
112+
Certificate: cert,
113+
ConfirmBeforeUse: confirm,
107114
}
108115

109116
_, err = sshagentclient.Extension(agent.SSH_TPM_AGENT_ADD, agent.MarshalTPMKeyMsg(&addedkey))
@@ -132,13 +139,17 @@ func main() {
132139

133140
if _, err = client.Extension(agent.SSH_TPM_AGENT_ADD, agent.MarshalTPMKeyMsg(
134141
&sshagent.AddedKey{
135-
PrivateKey: k,
136-
Comment: k.Description,
142+
PrivateKey: k,
143+
Comment: k.Description,
144+
ConfirmBeforeUse: confirm,
137145
},
138146
)); err != nil {
139147
log.Fatal(err)
140148
}
141149
fmt.Printf("Identity added: %s (%s)\n", path, k.Description)
150+
if confirm {
151+
fmt.Printf("The user must confirm each use of the key\n")
152+
}
142153

143154
certStr := fmt.Sprintf("%s-cert.pub", strings.TrimSuffix(path, filepath.Ext(path)))
144155
if _, err := os.Stat(certStr); !errors.Is(err, os.ErrNotExist) {
@@ -157,9 +168,10 @@ func main() {
157168
}
158169
if _, err = client.Extension(agent.SSH_TPM_AGENT_ADD, agent.MarshalTPMKeyMsg(
159170
&sshagent.AddedKey{
160-
PrivateKey: k,
161-
Certificate: cert,
162-
Comment: k.Description,
171+
PrivateKey: k,
172+
Certificate: cert,
173+
Comment: k.Description,
174+
ConfirmBeforeUse: confirm,
163175
},
164176
)); err != nil {
165177
log.Fatal(err)

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
chmod 755 askpass-test
2+
env SSH_ASKPASS=./askpass-test
3+
env SSH_ASKPASS_REQUIRE=force
4+
5+
exec ssh-tpm-agent -d --no-load &agent&
6+
exec ssh-tpm-keygen -N ''
7+
8+
# Add with -c: every signature requires confirmation via SSH_ASKPASS
9+
exec ssh-tpm-add -c
10+
11+
# Approve: askpass exits 0, signing succeeds, prompt+hint are recorded
12+
exec ssh-keygen -Y sign -n file -f .ssh/id_ecdsa.pub file_to_sign.txt
13+
grep '^confirm Allow use of key' askpass.log
14+
rm askpass.log file_to_sign.txt.sig
15+
16+
# Deny: sentinel file makes askpass exit 1, signing fails
17+
cp empty askpass.deny
18+
! exec ssh-keygen -Y sign -n file -f .ssh/id_ecdsa.pub file_to_sign.txt
19+
! exists file_to_sign.txt.sig
20+
grep '^confirm Allow use of key' askpass.log
21+
rm askpass.log askpass.deny
22+
23+
# Re-add without -c: askpass must not be consulted even if it would deny
24+
exec ssh-add -D
25+
exec ssh-tpm-add
26+
cp empty askpass.deny
27+
exec ssh-keygen -Y sign -n file -f .ssh/id_ecdsa.pub file_to_sign.txt
28+
! exists askpass.log
29+
30+
-- file_to_sign.txt --
31+
Hello World
32+
-- empty --
33+
-- askpass-test --
34+
#!/bin/sh
35+
# Log "<SSH_ASKPASS_PROMPT> <prompt>" and deny if the sentinel exists.
36+
# Uses a file because the background agent does not see later env changes.
37+
printf '%s %s\n' "$SSH_ASKPASS_PROMPT" "$1" >> askpass.log
38+
[ ! -e askpass.deny ]

0 commit comments

Comments
 (0)