Skip to content

Commit b4de73f

Browse files
hansnielsenzx2c4
hansnielsen
authored and
zx2c4
committed
ssh: support RSA SHA-2 (RFC8332) signatures
This change adds support for RSA SHA-2 based signatures for host keys and certificates. It also switches the default certificate signature algorithm for RSA to use SHA-512. This is implemented by treating ssh.Signer specially when the key type is `ssh-rsa` by also allowing SHA-256 and SHA-512 signatures. Fixes golang/go#37278 Change-Id: I2ee1ac4ae4c9c1de441a2d6cf1e806357ef18910 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/220037 Trust: Jason A. Donenfeld <[email protected]> Run-TryBot: Jason A. Donenfeld <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Jason A. Donenfeld <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]>
1 parent ceb1ce7 commit b4de73f

File tree

12 files changed

+213
-69
lines changed

12 files changed

+213
-69
lines changed

‎ssh/certs.go‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"time"
1515
)
1616

17-
// These constants from [PROTOCOL.certkeys] represent the algorithm names
17+
// These constants from [PROTOCOL.certkeys] represent the key algorithm names
1818
// for certificate types supported by this package.
1919
const (
2020
CertAlgoRSAv01 = "[email protected]"
@@ -27,6 +27,14 @@ const (
2727
CertAlgoSKED25519v01 = "[email protected]"
2828
)
2929

30+
// These constants from [PROTOCOL.certkeys] represent additional signature
31+
// algorithm names for certificate types supported by this package.
32+
const (
33+
CertSigAlgoRSAv01 = "[email protected]"
34+
CertSigAlgoRSASHA2256v01 = "[email protected]"
35+
CertSigAlgoRSASHA2512v01 = "[email protected]"
36+
)
37+
3038
// Certificate types distinguish between host and user
3139
// certificates. The values can be set in the CertType field of
3240
// Certificate.
@@ -423,6 +431,12 @@ func (c *Certificate) SignCert(rand io.Reader, authority Signer) error {
423431
}
424432
c.SignatureKey = authority.PublicKey()
425433

434+
if v, ok := authority.(AlgorithmSigner); ok {
435+
if v.PublicKey().Type() == KeyAlgoRSA {
436+
authority = &rsaSigner{v, SigAlgoRSASHA2512}
437+
}
438+
}
439+
426440
sig, err := authority.Sign(rand, c.bytesForSigning())
427441
if err != nil {
428442
return err
@@ -431,8 +445,14 @@ func (c *Certificate) SignCert(rand io.Reader, authority Signer) error {
431445
return nil
432446
}
433447

448+
// certAlgoNames includes a mapping from signature algorithms to the
449+
// corresponding certificate signature algorithm. When a key type (such
450+
// as ED25516) is associated with only one algorithm, the KeyAlgo
451+
// constant is used instead of the SigAlgo.
434452
var certAlgoNames = map[string]string{
435-
KeyAlgoRSA: CertAlgoRSAv01,
453+
SigAlgoRSA: CertSigAlgoRSAv01,
454+
SigAlgoRSASHA2256: CertSigAlgoRSASHA2256v01,
455+
SigAlgoRSASHA2512: CertSigAlgoRSASHA2512v01,
436456
KeyAlgoDSA: CertAlgoDSAv01,
437457
KeyAlgoECDSA256: CertAlgoECDSA256v01,
438458
KeyAlgoECDSA384: CertAlgoECDSA384v01,

‎ssh/certs_test.go‎

Lines changed: 31 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import (
99
"crypto/ecdsa"
1010
"crypto/elliptic"
1111
"crypto/rand"
12+
"fmt"
13+
"io"
1214
"net"
1315
"reflect"
1416
"testing"
1517
"time"
16-
17-
"golang.org/x/crypto/ssh/testdata"
1818
)
1919

2020
// Cert generated by ssh-keygen 6.0p1 Debian-4.
@@ -226,53 +226,33 @@ func TestHostKeyCert(t *testing.T) {
226226
}
227227
}
228228

229+
type legacyRSASigner struct {
230+
Signer
231+
}
232+
233+
func (s *legacyRSASigner) Sign(rand io.Reader, data []byte) (*Signature, error) {
234+
v, ok := s.Signer.(AlgorithmSigner)
235+
if !ok {
236+
return nil, fmt.Errorf("invalid signer")
237+
}
238+
return v.SignWithAlgorithm(rand, data, SigAlgoRSA)
239+
}
240+
229241
func TestCertTypes(t *testing.T) {
230242
var testVars = []struct {
231-
name string
232-
keys func() Signer
243+
name string
244+
signer Signer
245+
algo string
233246
}{
234-
{
235-
name: CertAlgoECDSA256v01,
236-
keys: func() Signer {
237-
s, _ := ParsePrivateKey(testdata.PEMBytes["ecdsap256"])
238-
return s
239-
},
240-
},
241-
{
242-
name: CertAlgoECDSA384v01,
243-
keys: func() Signer {
244-
s, _ := ParsePrivateKey(testdata.PEMBytes["ecdsap384"])
245-
return s
246-
},
247-
},
248-
{
249-
name: CertAlgoECDSA521v01,
250-
keys: func() Signer {
251-
s, _ := ParsePrivateKey(testdata.PEMBytes["ecdsap521"])
252-
return s
253-
},
254-
},
255-
{
256-
name: CertAlgoED25519v01,
257-
keys: func() Signer {
258-
s, _ := ParsePrivateKey(testdata.PEMBytes["ed25519"])
259-
return s
260-
},
261-
},
262-
{
263-
name: CertAlgoRSAv01,
264-
keys: func() Signer {
265-
s, _ := ParsePrivateKey(testdata.PEMBytes["rsa"])
266-
return s
267-
},
268-
},
269-
{
270-
name: CertAlgoDSAv01,
271-
keys: func() Signer {
272-
s, _ := ParsePrivateKey(testdata.PEMBytes["dsa"])
273-
return s
274-
},
275-
},
247+
{CertAlgoECDSA256v01, testSigners["ecdsap256"], ""},
248+
{CertAlgoECDSA384v01, testSigners["ecdsap384"], ""},
249+
{CertAlgoECDSA521v01, testSigners["ecdsap521"], ""},
250+
{CertAlgoED25519v01, testSigners["ed25519"], ""},
251+
{CertAlgoRSAv01, testSigners["rsa"], SigAlgoRSASHA2512},
252+
{CertAlgoRSAv01, &legacyRSASigner{testSigners["rsa"]}, SigAlgoRSA},
253+
{CertAlgoRSAv01, testSigners["rsa-sha2-256"], SigAlgoRSASHA2512},
254+
{CertAlgoRSAv01, testSigners["rsa-sha2-512"], SigAlgoRSASHA2512},
255+
{CertAlgoDSAv01, testSigners["dsa"], ""},
276256
}
277257

278258
k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
@@ -304,7 +284,7 @@ func TestCertTypes(t *testing.T) {
304284

305285
go NewServerConn(c1, conf)
306286

307-
priv := m.keys()
287+
priv := m.signer
308288
if err != nil {
309289
t.Fatalf("error generating ssh pubkey: %v", err)
310290
}
@@ -320,6 +300,10 @@ func TestCertTypes(t *testing.T) {
320300
t.Fatalf("error generating cert signer: %v", err)
321301
}
322302

303+
if m.algo != "" && cert.Signature.Format != m.algo {
304+
t.Errorf("expected %q signature format, got %q", m.algo, cert.Signature.Format)
305+
}
306+
323307
config := &ClientConfig{
324308
User: "user",
325309
HostKeyCallback: func(h string, r net.Addr, k PublicKey) error { return nil },

‎ssh/client.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,25 @@ func (c *connection) clientHandshake(dialAddress string, config *ClientConfig) e
115115

116116
// verifyHostKeySignature verifies the host key obtained in the key
117117
// exchange.
118-
func verifyHostKeySignature(hostKey PublicKey, result *kexResult) error {
118+
func verifyHostKeySignature(hostKey PublicKey, algo string, result *kexResult) error {
119119
sig, rest, ok := parseSignatureBody(result.Signature)
120120
if len(rest) > 0 || !ok {
121121
return errors.New("ssh: signature parse error")
122122
}
123123

124+
// For keys, underlyingAlgo is exactly algo. For certificates,
125+
// we have to look up the underlying key algorithm that SSH
126+
// uses to evaluate signatures.
127+
underlyingAlgo := algo
128+
for sigAlgo, certAlgo := range certAlgoNames {
129+
if certAlgo == algo {
130+
underlyingAlgo = sigAlgo
131+
}
132+
}
133+
if sig.Format != underlyingAlgo {
134+
return fmt.Errorf("ssh: invalid signature algorithm %q, expected %q", sig.Format, underlyingAlgo)
135+
}
136+
124137
return hostKey.Verify(result.H, sig)
125138
}
126139

‎ssh/client_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package ssh
66

77
import (
8+
"bytes"
9+
"crypto/rand"
810
"strings"
911
"testing"
1012
)
@@ -116,6 +118,45 @@ func TestHostKeyCheck(t *testing.T) {
116118
}
117119
}
118120

121+
func TestVerifyHostKeySignature(t *testing.T) {
122+
for _, tt := range []struct {
123+
key string
124+
signAlgo string
125+
verifyAlgo string
126+
wantError string
127+
}{
128+
{"rsa", SigAlgoRSA, SigAlgoRSA, ""},
129+
{"rsa", SigAlgoRSASHA2256, SigAlgoRSASHA2256, ""},
130+
{"rsa", SigAlgoRSA, SigAlgoRSASHA2512, `ssh: invalid signature algorithm "ssh-rsa", expected "rsa-sha2-512"`},
131+
{"ed25519", KeyAlgoED25519, KeyAlgoED25519, ""},
132+
} {
133+
key := testSigners[tt.key].PublicKey()
134+
s, ok := testSigners[tt.key].(AlgorithmSigner)
135+
if !ok {
136+
t.Fatalf("needed an AlgorithmSigner")
137+
}
138+
sig, err := s.SignWithAlgorithm(rand.Reader, []byte("test"), tt.signAlgo)
139+
if err != nil {
140+
t.Fatalf("couldn't sign: %q", err)
141+
}
142+
143+
b := bytes.Buffer{}
144+
writeString(&b, []byte(sig.Format))
145+
writeString(&b, sig.Blob)
146+
147+
result := kexResult{Signature: b.Bytes(), H: []byte("test")}
148+
149+
err = verifyHostKeySignature(key, tt.verifyAlgo, &result)
150+
if err != nil {
151+
if tt.wantError == "" || !strings.Contains(err.Error(), tt.wantError) {
152+
t.Errorf("got error %q, expecting %q", err.Error(), tt.wantError)
153+
}
154+
} else if tt.wantError != "" {
155+
t.Errorf("succeeded, but want error string %q", tt.wantError)
156+
}
157+
}
158+
}
159+
119160
func TestBannerCallback(t *testing.T) {
120161
c1, c2, err := netPipe()
121162
if err != nil {

‎ssh/common.go‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,13 @@ var preferredKexAlgos = []string{
6969
// supportedHostKeyAlgos specifies the supported host-key algorithms (i.e. methods
7070
// of authenticating servers) in preference order.
7171
var supportedHostKeyAlgos = []string{
72-
CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01,
72+
CertSigAlgoRSASHA2512v01, CertSigAlgoRSASHA2256v01,
73+
CertSigAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01,
7374
CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoED25519v01,
7475

7576
KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521,
76-
KeyAlgoRSA, KeyAlgoDSA,
77+
SigAlgoRSASHA2512, SigAlgoRSASHA2256,
78+
SigAlgoRSA, KeyAlgoDSA,
7779

7880
KeyAlgoED25519,
7981
}
@@ -90,16 +92,20 @@ var supportedCompressions = []string{compressionNone}
9092
// hashFuncs keeps the mapping of supported algorithms to their respective
9193
// hashes needed for signature verification.
9294
var hashFuncs = map[string]crypto.Hash{
93-
KeyAlgoRSA: crypto.SHA1,
94-
KeyAlgoDSA: crypto.SHA1,
95-
KeyAlgoECDSA256: crypto.SHA256,
96-
KeyAlgoECDSA384: crypto.SHA384,
97-
KeyAlgoECDSA521: crypto.SHA512,
98-
CertAlgoRSAv01: crypto.SHA1,
99-
CertAlgoDSAv01: crypto.SHA1,
100-
CertAlgoECDSA256v01: crypto.SHA256,
101-
CertAlgoECDSA384v01: crypto.SHA384,
102-
CertAlgoECDSA521v01: crypto.SHA512,
95+
SigAlgoRSA: crypto.SHA1,
96+
SigAlgoRSASHA2256: crypto.SHA256,
97+
SigAlgoRSASHA2512: crypto.SHA512,
98+
KeyAlgoDSA: crypto.SHA1,
99+
KeyAlgoECDSA256: crypto.SHA256,
100+
KeyAlgoECDSA384: crypto.SHA384,
101+
KeyAlgoECDSA521: crypto.SHA512,
102+
CertSigAlgoRSAv01: crypto.SHA1,
103+
CertSigAlgoRSASHA2256v01: crypto.SHA256,
104+
CertSigAlgoRSASHA2512v01: crypto.SHA512,
105+
CertAlgoDSAv01: crypto.SHA1,
106+
CertAlgoECDSA256v01: crypto.SHA256,
107+
CertAlgoECDSA384v01: crypto.SHA384,
108+
CertAlgoECDSA521v01: crypto.SHA512,
103109
}
104110

105111
// unexpectedMessageError results when the SSH message that we received didn't

‎ssh/handshake.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,15 @@ func (t *handshakeTransport) sendKexInit() error {
457457

458458
if len(t.hostKeys) > 0 {
459459
for _, k := range t.hostKeys {
460-
msg.ServerHostKeyAlgos = append(
461-
msg.ServerHostKeyAlgos, k.PublicKey().Type())
460+
algo := k.PublicKey().Type()
461+
switch algo {
462+
case KeyAlgoRSA:
463+
msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, []string{SigAlgoRSASHA2512, SigAlgoRSASHA2256, SigAlgoRSA}...)
464+
case CertAlgoRSAv01:
465+
msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, []string{CertSigAlgoRSASHA2512v01, CertSigAlgoRSASHA2256v01, CertSigAlgoRSAv01}...)
466+
default:
467+
msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, algo)
468+
}
462469
}
463470
} else {
464471
msg.ServerHostKeyAlgos = t.hostKeyAlgorithms
@@ -614,8 +621,22 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {
614621
func (t *handshakeTransport) server(kex kexAlgorithm, algs *algorithms, magics *handshakeMagics) (*kexResult, error) {
615622
var hostKey Signer
616623
for _, k := range t.hostKeys {
617-
if algs.hostKey == k.PublicKey().Type() {
624+
kt := k.PublicKey().Type()
625+
if kt == algs.hostKey {
618626
hostKey = k
627+
} else if signer, ok := k.(AlgorithmSigner); ok {
628+
// Some signature algorithms don't show up as key types
629+
// so we have to manually check for a compatible host key.
630+
switch kt {
631+
case KeyAlgoRSA:
632+
if algs.hostKey == SigAlgoRSASHA2256 || algs.hostKey == SigAlgoRSASHA2512 {
633+
hostKey = &rsaSigner{signer, algs.hostKey}
634+
}
635+
case CertAlgoRSAv01:
636+
if algs.hostKey == CertSigAlgoRSASHA2256v01 || algs.hostKey == CertSigAlgoRSASHA2512v01 {
637+
hostKey = &rsaSigner{signer, certToPrivAlgo(algs.hostKey)}
638+
}
639+
}
619640
}
620641
}
621642

@@ -634,7 +655,7 @@ func (t *handshakeTransport) client(kex kexAlgorithm, algs *algorithms, magics *
634655
return nil, err
635656
}
636657

637-
if err := verifyHostKeySignature(hostKey, result); err != nil {
658+
if err := verifyHostKeySignature(hostKey, algs.hostKey, result); err != nil {
638659
return nil, err
639660
}
640661

‎ssh/keys.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -939,6 +939,15 @@ func newDSAPrivateKey(key *dsa.PrivateKey) (Signer, error) {
939939
return &dsaPrivateKey{key}, nil
940940
}
941941

942+
type rsaSigner struct {
943+
AlgorithmSigner
944+
defaultAlgorithm string
945+
}
946+
947+
func (s *rsaSigner) Sign(rand io.Reader, data []byte) (*Signature, error) {
948+
return s.AlgorithmSigner.SignWithAlgorithm(rand, data, s.defaultAlgorithm)
949+
}
950+
942951
type wrappedSigner struct {
943952
signer crypto.Signer
944953
pubKey PublicKey

‎ssh/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error)
284284

285285
func isAcceptableAlgo(algo string) bool {
286286
switch algo {
287-
case KeyAlgoRSA, KeyAlgoDSA, KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, KeyAlgoSKECDSA256, KeyAlgoED25519, KeyAlgoSKED25519,
287+
case SigAlgoRSA, SigAlgoRSASHA2256, SigAlgoRSASHA2512, KeyAlgoDSA, KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, KeyAlgoSKECDSA256, KeyAlgoED25519, KeyAlgoSKED25519,
288288
CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoSKECDSA256v01, CertAlgoED25519v01, CertAlgoSKED25519v01:
289289
return true
290290
}

‎ssh/session_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,13 @@ func TestHostKeyAlgorithms(t *testing.T) {
757757
connect(clientConf, KeyAlgoECDSA256)
758758

759759
// Client asks for RSA explicitly.
760-
clientConf.HostKeyAlgorithms = []string{KeyAlgoRSA}
760+
clientConf.HostKeyAlgorithms = []string{SigAlgoRSA}
761+
connect(clientConf, KeyAlgoRSA)
762+
763+
// Client asks for RSA-SHA2-512 explicitly.
764+
clientConf.HostKeyAlgorithms = []string{SigAlgoRSASHA2512}
765+
// We get back an "ssh-rsa" key but the verification happened
766+
// with an RSA-SHA2-512 signature.
761767
connect(clientConf, KeyAlgoRSA)
762768

763769
c1, c2, err := netPipe()

‎ssh/test/test_unix_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ Banner {{.Dir}}/banner
3535
HostKey {{.Dir}}/id_rsa
3636
HostKey {{.Dir}}/id_dsa
3737
HostKey {{.Dir}}/id_ecdsa
38-
HostCertificate {{.Dir}}/id_rsa-cert.pub
38+
HostCertificate {{.Dir}}/id_rsa-sha2-512-cert.pub
3939
Pidfile {{.Dir}}/sshd.pid
4040
#UsePrivilegeSeparation no
4141
KeyRegenerationInterval 3600

0 commit comments

Comments
 (0)