Skip to content

Commit

Permalink
ssh: support rsa-sha2-256/512 on the server side
Browse files Browse the repository at this point in the history
This lets clients know we support rsa-sha2-256/512 signatures from
ssh-rsa public keys. OpenSSH prefers to break the connection rather than
attempting trial and error, apparently.

We don't enable support for the "ext-info-s" because we're not
interested in any client->server extensions.

This also replaces isAcceptableAlgo which was rejecting the
rsa-sha2-256/512-cert-v01@openssh.com public key algorithms.

Tested with OpenSSH 9.1 on macOS Ventura.

Fixes golang/go#49269
Updates golang/go#49952

Co-authored-by: Nicola Murino <nicola.murino@gmail.com>
Co-authored-by: Kristin Davidson <kdavidson@atlassian.com>
Change-Id: I4955c3b12bb45575e9977ac657bb5805b49d00c3
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/447757
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
  • Loading branch information
3 people committed Nov 12, 2022
1 parent 21d60a1 commit 6fad3df
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 15 deletions.
4 changes: 1 addition & 3 deletions ssh/client_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ func TestClientAuthPublicKey(t *testing.T) {
if err := tryAuth(t, config); err != nil {
t.Fatalf("unable to dial remote side: %s", err)
}
// Once the server implements the server-sig-algs extension, this will turn
// into KeyAlgoRSASHA256.
if len(signer.used) != 1 || signer.used[0] != KeyAlgoRSA {
if len(signer.used) != 1 || signer.used[0] != KeyAlgoRSASHA256 {
t.Errorf("unexpected Sign/SignWithAlgorithm calls: %q", signer.used)
}
}
Expand Down
15 changes: 15 additions & 0 deletions ssh/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"io"
"math"
"strings"
"sync"

_ "crypto/sha1"
Expand Down Expand Up @@ -118,6 +119,20 @@ func algorithmsForKeyFormat(keyFormat string) []string {
}
}

// supportedPubKeyAuthAlgos specifies the supported client public key
// authentication algorithms. Note that this doesn't include certificate types
// since those use the underlying algorithm. This list is sent to the client if
// it supports the server-sig-algs extension. Order is irrelevant.
var supportedPubKeyAuthAlgos = []string{
KeyAlgoED25519,
KeyAlgoSKED25519, KeyAlgoSKECDSA256,
KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521,
KeyAlgoRSASHA256, KeyAlgoRSASHA512, KeyAlgoRSA,
KeyAlgoDSA,
}

var supportedPubKeyAuthAlgosList = strings.Join(supportedPubKeyAuthAlgos, ",")

// unexpectedMessageError results when the SSH message that we received didn't
// match what we wanted.
func unexpectedMessageError(expected, got uint8) error {
Expand Down
21 changes: 20 additions & 1 deletion ssh/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,8 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {
return err
}

if t.sessionID == nil {
firstKeyExchange := t.sessionID == nil
if firstKeyExchange {
t.sessionID = result.H
}
result.SessionID = t.sessionID
Expand All @@ -626,6 +627,24 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {
if err = t.conn.writePacket([]byte{msgNewKeys}); err != nil {
return err
}

// On the server side, after the first SSH_MSG_NEWKEYS, send a SSH_MSG_EXT_INFO
// message with the server-sig-algs extension if the client supports it. See
// RFC 8308, Sections 2.4 and 3.1.
if !isClient && firstKeyExchange && contains(clientInit.KexAlgos, "ext-info-c") {
extInfo := &extInfoMsg{
NumExtensions: 1,
Payload: make([]byte, 0, 4+15+4+len(supportedPubKeyAuthAlgosList)),
}
extInfo.Payload = appendInt(extInfo.Payload, len("server-sig-algs"))
extInfo.Payload = append(extInfo.Payload, "server-sig-algs"...)
extInfo.Payload = appendInt(extInfo.Payload, len(supportedPubKeyAuthAlgosList))
extInfo.Payload = append(extInfo.Payload, supportedPubKeyAuthAlgosList...)
if err := t.conn.writePacket(Marshal(extInfo)); err != nil {
return err
}
}

if packet, err := t.conn.readPacket(); err != nil {
return err
} else if packet[0] != msgNewKeys {
Expand Down
13 changes: 2 additions & 11 deletions ssh/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,6 @@ func (s *connection) serverHandshake(config *ServerConfig) (*Permissions, error)
return perms, err
}

func isAcceptableAlgo(algo string) bool {
switch algo {
case KeyAlgoRSA, KeyAlgoRSASHA256, KeyAlgoRSASHA512, KeyAlgoDSA, KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521, KeyAlgoSKECDSA256, KeyAlgoED25519, KeyAlgoSKED25519,
CertAlgoRSAv01, CertAlgoDSAv01, CertAlgoECDSA256v01, CertAlgoECDSA384v01, CertAlgoECDSA521v01, CertAlgoSKECDSA256v01, CertAlgoED25519v01, CertAlgoSKED25519v01:
return true
}
return false
}

func checkSourceAddress(addr net.Addr, sourceAddrs string) error {
if addr == nil {
return errors.New("ssh: no address known for client, but source-address match required")
Expand Down Expand Up @@ -514,7 +505,7 @@ userAuthLoop:
return nil, parseError(msgUserAuthRequest)
}
algo := string(algoBytes)
if !isAcceptableAlgo(algo) {
if !contains(supportedPubKeyAuthAlgos, underlyingAlgo(algo)) {
authErr = fmt.Errorf("ssh: algorithm %q not accepted", algo)
break
}
Expand Down Expand Up @@ -572,7 +563,7 @@ userAuthLoop:
// algorithm name that corresponds to algo with
// sig.Format. This is usually the same, but
// for certs, the names differ.
if !isAcceptableAlgo(sig.Format) {
if !contains(supportedPubKeyAuthAlgos, sig.Format) {
authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format)
break
}
Expand Down

0 comments on commit 6fad3df

Please sign in to comment.