x/vuln: false negative for GO-2024-3321

[kaimaera]

Go version

go1.23.5

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOOS='linux'
GOROOT='/usr/local/go'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.5'
GODEBUG=''
GOTELEMETRY='local'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='0'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build753288169=/tmp/go-build -gno-record-gcc-switches

What did you do?

Ran govulncheck on the sample code below (https://pkg.go.dev/golang.org/x/crypto/ssh#ServerConn example, with a fmt.Println added):

package main
 
import (
       "fmt"
       "log"
       "net"
       "os"
       "sync"
 
       "golang.org/x/crypto/ssh"
       "golang.org/x/crypto/ssh/terminal"
)
 
func main() {
       // Public key authentication is done by comparing
       // the public key of a received connection
       // with the entries in the authorized_keys file.
       authorizedKeysBytes, err := os.ReadFile("authorized_keys")
       if err != nil {
              log.Fatalf("Failed to load authorized_keys, err: %v", err)
       }
 
       authorizedKeysMap := map[string]bool{}
       for len(authorizedKeysBytes) > 0 {
              pubKey, _, _, rest, err := ssh.ParseAuthorizedKey(authorizedKeysBytes)
              if err != nil {
                     log.Fatal(err)
              }
 
              authorizedKeysMap[string(pubKey.Marshal())] = true
              authorizedKeysBytes = rest
       }
 
       // An SSH server is represented by a ServerConfig, which holds
       // certificate details and handles authentication of ServerConns.
       config := &ssh.ServerConfig{
              // Remove to disable public key auth.
              PublicKeyCallback: func(c ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) {   // <<< Affected by the CVE and called
                     fmt.Printf("received key: %s\n", ssh.FingerprintSHA256(pubKey))
                     if authorizedKeysMap[string(pubKey.Marshal())] {
                           return &ssh.Permissions{
                                  // Record the public key used for authentication.
                                  Extensions: map[string]string{
                                         "pubkey-fp": ssh.FingerprintSHA256(pubKey),
                                  },
                           }, nil
                     }
                     return nil, fmt.Errorf("unknown public key for %q", c.User())
              },
       }
 
       privateBytes, err := os.ReadFile("hostkey")
       if err != nil {
              log.Fatal("Failed to load private key: ", err)
       }
 
       private, err := ssh.ParsePrivateKey(privateBytes)
       if err != nil {
              log.Fatal("Failed to parse private key: ", err)
       }
       config.AddHostKey(private)
 
       // Once a ServerConfig has been configured, connections can be
       // accepted.
       listener, err := net.Listen("tcp", "0.0.0.0:2022")
       if err != nil {
              log.Fatal("failed to listen for connection: ", err)
       }
       nConn, err := listener.Accept()
       if err != nil {
              log.Fatal("failed to accept incoming connection: ", err)
       }
 
       // Before use, a handshake must be performed on the incoming
       // net.Conn.
       conn, chans, reqs, err := ssh.NewServerConn(nConn, config)
       if err != nil {
              log.Fatal("failed to handshake: ", err)
       }
       log.Printf("logged in with key %s", conn.Permissions.Extensions["pubkey-fp"])
}

What did you see happen?

Program output (when connected to using the Linux SSH client):

$ go run .
received key: SHA256:bh9pEcEmRfexTHJqZk263fTJCGqwiZarIV7B8fWwif8
received key: SHA256:QxTUMyd5Rwm9eoBoasz9yyTj4rm7K5dUOJ0oQvp6Y4w
2025/01/25 09:05:56 logged in with key SHA256:QxTUMyd5Rwm9eoBoasz9yyTj4rm7K5dUOJ0oQvp6Y4w

govulncheck output:

Fetching vulnerabilities from the database...
 
Checking the code against the vulnerabilities...
 
The package pattern matched the following root package:
  cve-2024-45337.test/F001/bin
Govulncheck scanned the following 4 modules and the go1.23.5 standard library:
  cve-2024-45337.test/F001/bin
  golang.org/x/[email protected]
  golang.org/x/[email protected]
  golang.org/x/[email protected]
 
=== Symbol Results ===
 
No vulnerabilities found.
 
=== Package Results ===
 
Vulnerability #1: GO-2024-3321
    Misuse of ServerConfig.PublicKeyCallback may cause authorization bypass in
    golang.org/x/crypto
  More info: https://pkg.go.dev/vuln/GO-2024-3321
  Module: golang.org/x/crypto
    Found in: golang.org/x/[email protected]
    Fixed in: golang.org/x/[email protected]
 
=== Module Results ===
 
No other vulnerabilities found.
 
Your code is affected by 0 vulnerabilities.
This scan also found 1 vulnerability in packages you import and 0
vulnerabilities in modules you require, but your code doesn't appear to call
these vulnerabilities.

What did you expect to see?

The sample code is vulnerable to GO-2024-3321, as far as I can tell:

• uses golang.org/x/crypto v0.30.0
• serves SSH connections via ssh.NewServerConn and specifies a PublickeyCallback

Whilst the sample program does not misuse the callback, it potentially could. Based on the OSV record, the govulncheck seeks to determine whether ServerConfig.PublickeyCallback is executed (or reachable from main via call graph analysis). It clearly is but the report states otherwise.

[gabyhelp]

Related Issues

• x/crypto/ssh: SSH handshake fails over some net.Conn implementations (ex. net.Pipe) #32990
• x/crypto/ssh: TestValidTerminalMode fails with some shells #38037 (closed)
• x/exp/cmd/govulncheck: exclude (fixed) vulnerability info from the own project #48079 (closed)
• x/crypto/ssh: occasional handshake failures #32705 (closed)
• x/crypto/ssh: can't establish ssh connection using signed key #54027 (closed)
• x/crypto/ssh: Corrupting a byte of a clients private key results in panic, rather than authorization failure #42014 (closed)
• x/crypto/ssh: fix documention for ssh.NewServerConn to avoid potential DoS attack #43521
• x/crypto/ssh: unable to authenticate if the first private key fails. #60587 (closed)
• import/path: "SSH connection command ssh -v -oHostKeyAlgorithms=+ssh-rsa user@host -p 22 cannot connect." #68954 (closed)

Related Discussions

• [security] Vulnerability in golang.org/x/crypto

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[cagedmantis]

@golang/vulndb

[zpavlinovic]

If I understand correctly from what you are saying, ServerConfig.PublickeyCallback is not actually called. If that is the case, then the output of govulncheck is as expected. The function is not reachable via the call graph since it is not called, e.g., executed.

[seankhliao]

I think it is called?
It's called by conn, chans, reqs, err := ssh.NewServerConn(nConn, config), which is supported by the output line log.Printf("logged in with key %s", conn.Permissions.Extensions["pubkey-fp"]) printing a value set in the callback 2025/01/25 09:05:56 logged in with key SHA256:QxTUMyd5Rwm9eoBoasz9yyTj4rm7K5dUOJ0oQvp6Y4w.

The code doesn't misuse the callback arguments, but govulncheck wouldn't know about that, it should just flag the use of PublicKeyCallback

[zpavlinovic]

Then I got confused with the opening message. Actually, is it possible to get a smaller reproducer? If there is an issue, I suspect the example can be shortened.

[zpavlinovic]

Pending above comment resolution, I think I know where the problem might be: ssh.ServerConfig.PublicKeyCallback is not a symbol (function/method) name. It is the name of a struct field.

I will try to confirm this, which seems to be a db issue.

[kaimaera]

Then I got confused with the opening message. Actually, is it possible to get a smaller reproducer? If there is an issue, I suspect the example can be shortened.

Shortened as requested

[kaimaera]

Pending above comment resolution, I think I know where the problem might be: ssh.ServerConfig.PublicKeyCallback is not a symbol (function/method) name. It is the name of a struct field.

I will try to confirm this, which seems to be a db issue.

If govulncheck can’t handle functions as fields, then NewServerConn has to be flagged instead, which in turn will lead to more false positives (e.g. running a SSH server that does not use pubkey auth).

Catch 22… that said, would rather a false positive than false negative.

[seankhliao]

I think a minimal example would look like:

package main

import (
	"fmt"
	"net"

	"golang.org/x/crypto/ssh"
)

func main() {
	config := &ssh.ServerConfig{
		PublicKeyCallback: func(c ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) {
			return nil, fmt.Errorf("unknown public key for %q", c.User())
		},
	}

	var conn net.Conn
	ssh.NewServerConn(conn, config)
}

The callback is in the called from NewServerConn here:

https://go.googlesource.com/crypto/+/refs/heads/master/ssh/server.go#659