Skip to content

ssh: close net.Conn on all NewServerConn errors #279

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ssh/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,15 @@ func NewServerConn(c net.Conn, config *ServerConfig) (*ServerConn, <-chan NewCha
} else {
for _, algo := range fullConf.PublicKeyAuthAlgorithms {
if !contains(supportedPubKeyAuthAlgos, algo) {
c.Close()
return nil, nil, nil, fmt.Errorf("ssh: unsupported public key authentication algorithm %s", algo)
}
}
}
// Check if the config contains any unsupported key exchanges
for _, kex := range fullConf.KeyExchanges {
if _, ok := serverForbiddenKexAlgos[kex]; ok {
c.Close()
return nil, nil, nil, fmt.Errorf("ssh: unsupported key exchange %s for server", kex)
}
}
Expand Down
73 changes: 64 additions & 9 deletions ssh/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
package ssh

import (
"io"
"net"
"sync/atomic"
"testing"
"time"
)

func TestClientAuthRestrictedPublicKeyAlgos(t *testing.T) {
Expand Down Expand Up @@ -59,27 +63,78 @@ func TestClientAuthRestrictedPublicKeyAlgos(t *testing.T) {
}

func TestNewServerConnValidationErrors(t *testing.T) {
c1, c2, err := netPipe()
if err != nil {
t.Fatalf("netPipe: %v", err)
}
defer c1.Close()
defer c2.Close()

serverConf := &ServerConfig{
PublicKeyAuthAlgorithms: []string{CertAlgoRSAv01},
}
_, _, _, err = NewServerConn(c1, serverConf)
c := &markerConn{}
_, _, _, err := NewServerConn(c, serverConf)
if err == nil {
t.Fatal("NewServerConn with invalid public key auth algorithms succeeded")
}
if !c.isClosed() {
t.Fatal("NewServerConn with invalid public key auth algorithms left connection open")
}
if c.isUsed() {
t.Fatal("NewServerConn with invalid public key auth algorithms used connection")
}

serverConf = &ServerConfig{
Config: Config{
KeyExchanges: []string{kexAlgoDHGEXSHA256},
},
}
_, _, _, err = NewServerConn(c1, serverConf)
c = &markerConn{}
_, _, _, err = NewServerConn(c, serverConf)
if err == nil {
t.Fatal("NewServerConn with unsupported key exchange succeeded")
}
if !c.isClosed() {
t.Fatal("NewServerConn with unsupported key exchange left connection open")
}
if c.isUsed() {
t.Fatal("NewServerConn with unsupported key exchange used connection")
}
}

type markerConn struct {
closed uint32
used uint32
}

func (c *markerConn) isClosed() bool {
return atomic.LoadUint32(&c.closed) != 0
}

func (c *markerConn) isUsed() bool {
return atomic.LoadUint32(&c.used) != 0
}

func (c *markerConn) Close() error {
atomic.StoreUint32(&c.closed, 1)
return nil
}

func (c *markerConn) Read(b []byte) (n int, err error) {
atomic.StoreUint32(&c.used, 1)
if atomic.LoadUint32(&c.closed) != 0 {
return 0, net.ErrClosed
} else {
return 0, io.EOF
}
}

func (c *markerConn) Write(b []byte) (n int, err error) {
atomic.StoreUint32(&c.used, 1)
if atomic.LoadUint32(&c.closed) != 0 {
return 0, net.ErrClosed
} else {
return 0, io.ErrClosedPipe
}
}

func (*markerConn) LocalAddr() net.Addr { return nil }
func (*markerConn) RemoteAddr() net.Addr { return nil }

func (*markerConn) SetDeadline(t time.Time) error { return nil }
func (*markerConn) SetReadDeadline(t time.Time) error { return nil }
func (*markerConn) SetWriteDeadline(t time.Time) error { return nil }