From 0ae49b8145643036e0e6c266cf4edc0f543ea9e0 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 4 Jun 2025 11:54:11 +0200 Subject: [PATCH 1/8] ssh: reject certificate keys used as signature keys for SSH certs As specified in draft-miller-ssh-cert-01, Section 2.1.1: Implementations MUST NOT accept certificate keys as CA keys. Change-Id: I2e559a8a58b7bceccd0d8c6b80803abdbe281067 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/678715 Reviewed-by: Filippo Valsorda LUCI-TryBot-Result: Go LUCI Auto-Submit: Nicola Murino Reviewed-by: Dmitri Shuralyov Reviewed-by: David Chase --- ssh/certs.go | 9 ++++++++- ssh/certs_test.go | 29 +++++++++++++++++++++++++++++ ssh/doc.go | 1 + 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/ssh/certs.go b/ssh/certs.go index a3dc629c62..4f535ebeb8 100644 --- a/ssh/certs.go +++ b/ssh/certs.go @@ -447,12 +447,19 @@ func (c *CertChecker) CheckCert(principal string, cert *Certificate) error { // SignCert signs the certificate with an authority, setting the Nonce, // SignatureKey, and Signature fields. If the authority implements the // MultiAlgorithmSigner interface the first algorithm in the list is used. This -// is useful if you want to sign with a specific algorithm. +// is useful if you want to sign with a specific algorithm. As specified in +// [SSH-CERTS], Section 2.1.1, authority can't be a [Certificate]. func (c *Certificate) SignCert(rand io.Reader, authority Signer) error { c.Nonce = make([]byte, 32) if _, err := io.ReadFull(rand, c.Nonce); err != nil { return err } + // The Type() function is intended to return only certificate key types, but + // we use certKeyAlgoNames anyway for safety, to match [Certificate.Type]. + if _, ok := certKeyAlgoNames[authority.PublicKey().Type()]; ok { + return fmt.Errorf("ssh: certificates cannot be used as authority (public key type %q)", + authority.PublicKey().Type()) + } c.SignatureKey = authority.PublicKey() if v, ok := authority.(MultiAlgorithmSigner); ok { diff --git a/ssh/certs_test.go b/ssh/certs_test.go index 1a4b49990a..e2a6fedc8b 100644 --- a/ssh/certs_test.go +++ b/ssh/certs_test.go @@ -404,3 +404,32 @@ func TestCertSignWithMultiAlgorithmSigner(t *testing.T) { }) } } + +func TestCertSignWithCertificate(t *testing.T) { + cert := &Certificate{ + Key: testPublicKeys["rsa"], + ValidBefore: CertTimeInfinity, + CertType: UserCert, + } + if err := cert.SignCert(rand.Reader, testSigners["ecdsa"]); err != nil { + t.Fatalf("SignCert: %v", err) + } + signer, err := NewSignerWithAlgorithms(testSigners["rsa"].(AlgorithmSigner), []string{KeyAlgoRSASHA256}) + if err != nil { + t.Fatal(err) + } + certSigner, err := NewCertSigner(cert, signer) + if err != nil { + t.Fatalf("NewCertSigner: %v", err) + } + + cert1 := &Certificate{ + Key: testPublicKeys["ecdsa"], + ValidBefore: CertTimeInfinity, + CertType: UserCert, + } + + if err := cert1.SignCert(rand.Reader, certSigner); err == nil { + t.Fatal("successfully signed a certificate using another certificate, it is expected to fail") + } +} diff --git a/ssh/doc.go b/ssh/doc.go index f5d352fe3a..04ccce3461 100644 --- a/ssh/doc.go +++ b/ssh/doc.go @@ -16,6 +16,7 @@ References: [PROTOCOL]: https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL?rev=HEAD [PROTOCOL.certkeys]: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD [SSH-PARAMETERS]: http://www.iana.org/assignments/ssh-parameters/ssh-parameters.xml#ssh-parameters-1 + [SSH-CERTS]: https://datatracker.ietf.org/doc/html/draft-miller-ssh-cert-01 This package does not fall under the stability promise of the Go language itself, so its API may be changed when pressing needs arise. From c6fce028266aa1271946a7dfde94cd71cf077d5e Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 4 Jun 2025 12:39:12 +0200 Subject: [PATCH 2/8] ssh: refuse to parse certificates that use a certificate as signing key According to draft-miller-ssh-cert-01, Section 2.1.1, certificates with certificate keys as signature keys are invalid Change-Id: I474524ea444deb78f2fa7c2682e47c0fd057f0b8 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/678716 LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Auto-Submit: Nicola Murino Reviewed-by: Dmitri Shuralyov Reviewed-by: Filippo Valsorda --- ssh/certs.go | 17 +++++++++-------- ssh/keys.go | 2 +- ssh/keys_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/ssh/certs.go b/ssh/certs.go index 4f535ebeb8..139fa31e1b 100644 --- a/ssh/certs.go +++ b/ssh/certs.go @@ -233,7 +233,11 @@ func parseCert(in []byte, privAlgo string) (*Certificate, error) { if err != nil { return nil, err } - + // The Type() function is intended to return only certificate key types, but + // we use certKeyAlgoNames anyway for safety, to match [Certificate.Type]. + if _, ok := certKeyAlgoNames[k.Type()]; ok { + return nil, fmt.Errorf("ssh: the signature key type %q is invalid for certificates", k.Type()) + } c.SignatureKey = k c.Signature, rest, ok = parseSignatureBody(g.Signature) if !ok || len(rest) > 0 { @@ -301,16 +305,13 @@ type CertChecker struct { SupportedCriticalOptions []string // IsUserAuthority should return true if the key is recognized as an - // authority for the given user certificate. This allows for - // certificates to be signed by other certificates. This must be set - // if this CertChecker will be checking user certificates. + // authority for user certificate. This must be set if this CertChecker + // will be checking user certificates. IsUserAuthority func(auth PublicKey) bool // IsHostAuthority should report whether the key is recognized as - // an authority for this host. This allows for certificates to be - // signed by other keys, and for those other keys to only be valid - // signers for particular hostnames. This must be set if this - // CertChecker will be checking host certificates. + // an authority for this host. This must be set if this CertChecker + // will be checking host certificates. IsHostAuthority func(auth PublicKey, address string) bool // Clock is used for verifying time stamps. If nil, time.Now diff --git a/ssh/keys.go b/ssh/keys.go index 566e09d5a1..a28c0de503 100644 --- a/ssh/keys.go +++ b/ssh/keys.go @@ -273,7 +273,7 @@ func ParseAuthorizedKey(in []byte) (out PublicKey, comment string, options []str return nil, "", nil, nil, errors.New("ssh: no key found") } -// ParsePublicKey parses an SSH public key formatted for use in +// ParsePublicKey parses an SSH public key or certificate formatted for use in // the SSH wire protocol according to RFC 4253, section 6.6. func ParsePublicKey(in []byte) (out PublicKey, err error) { algo, in, ok := parseString(in) diff --git a/ssh/keys_test.go b/ssh/keys_test.go index 7d5b86ff0d..f3eb223a9c 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go @@ -810,3 +810,30 @@ func TestCryptoPublicKey(t *testing.T) { } } } + +func TestParseCertWithCertSignatureKey(t *testing.T) { + certBytes := []byte(`-----BEGIN SSH CERTIFICATE----- +AAAAIHNzaC1lZDI1NTE5LWNlcnQtdjAxQG9wZW5zc2guY29tAAAAIPSp27hvNSB0 +IotJnVhjC4zxNgNS8BHlUCxD0VJi4D/eAAAAIIJMi1e5qfx+IFuKD/p/Ssqcb3os +CpOw/4wBs1pQ53zwAAAAAAAAAAEAAAACAAAAAAAAABMAAAAPZm9vLmV4YW1wbGUu +Y29tAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAT0AAAAgc3NoLWVkMjU1 +MTktY2VydC12MDFAb3BlbnNzaC5jb20AAAAg+sNYhCO35mQT1UBMpmMk8ey+culd +IU8vBlPEl4B07swAAAAggiv+RLnboS4znGCVl/n1jDg2uD0h15tW4s/04eS2mLQA +AAAAAAAAAQAAAAIAAAAAAAAAEwAAAA9mb28uZXhhbXBsZS5jb20AAAAAAAAAAAAA +AAAAAAAAAAAAAAAAAAAAAAAAAAAAMwAAAAtzc2gtZWQyNTUxOQAAACCV2wETgLKL +Kt0bRl3YUnd/ZYSlq0xJMbn4Jj3cdPWykQAAAFMAAAALc3NoLWVkMjU1MTkAAABA +WOdbRGEzyRAhiIK227CLUQD5caXYMV8FvSIB7toEE2M/8HnWdG9H3Rsg/v3unruQ +JrQldnuPJNe7KOP2+zvUDgAAAFMAAAALc3NoLWVkMjU1MTkAAABAm3bIPp85ZpIe +D+izJcUqlcAOri7HO8bULFNHT6LVegvB06xQ5TLwMlrxWUF4cafl1tSe8JQck4a6 +cLYUOHfQDw== +-----END SSH CERTIFICATE----- + `) + block, _ := pem.Decode(certBytes) + if block == nil { + t.Fatal("invalid test certificate") + } + + if _, err := ParsePublicKey(block.Bytes); err == nil { + t.Fatal("parsing an SSH certificate using another certificate as signature key succeeded; expected failure") + } +} From 952517d181d424f6c77f7460bf728205cb048411 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Mon, 16 Jun 2025 16:01:14 +0000 Subject: [PATCH 3/8] x509roots/fallback: update bundle This is an automated CL which updates the NSS root bundle. [git-generate] go generate ./x509roots Change-Id: Icb71f9f7c509dc6f49ad4385aa287bd6a8966523 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/681915 Auto-Submit: Gopher Robot LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-by: Roland Shoemaker --- x509roots/fallback/bundle.go | 38 ------------------------------------ 1 file changed, 38 deletions(-) diff --git a/x509roots/fallback/bundle.go b/x509roots/fallback/bundle.go index 81ad6e8b5a..19a168c4bb 100644 --- a/x509roots/fallback/bundle.go +++ b/x509roots/fallback/bundle.go @@ -4148,44 +4148,6 @@ Jd1hJyMctTEHBDa0GpC9oHRxUIltvBTjD4au8as+x6AJzKNI0eDbZOeStc+vckNw i/nDhDwTqn6Sm1dTk/pwwpEOMfmbZ13pljheX7NzTogVZ96edhBiIL5VaZVDADlN 9u6wWk5JRFRYX0KD -----END CERTIFICATE----- -`, - }, - { - cn: "OU=ePKI Root Certification Authority,O=Chunghwa Telecom Co.\\, Ltd.,C=TW", - sha256Hash: "c0a6f4dc63a24bfdcf54ef2a6a082a0a72de35803e2ff5ff527ae5d87206dfd5", - pem: `-----BEGIN CERTIFICATE----- -MIIFsDCCA5igAwIBAgIQFci9ZUdcr7iXAF7kBtK8nTANBgkqhkiG9w0BAQUFADBe -MQswCQYDVQQGEwJUVzEjMCEGA1UECgwaQ2h1bmdod2EgVGVsZWNvbSBDby4sIEx0 -ZC4xKjAoBgNVBAsMIWVQS0kgUm9vdCBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eTAe -Fw0wNDEyMjAwMjMxMjdaFw0zNDEyMjAwMjMxMjdaMF4xCzAJBgNVBAYTAlRXMSMw -IQYDVQQKDBpDaHVuZ2h3YSBUZWxlY29tIENvLiwgTHRkLjEqMCgGA1UECwwhZVBL -SSBSb290IENlcnRpZmljYXRpb24gQXV0aG9yaXR5MIICIjANBgkqhkiG9w0BAQEF -AAOCAg8AMIICCgKCAgEA4SUP7o3biDN1Z82tH306Tm2d0y8U82N0ywEhajfqhFAH -SyZbCUNsIZ5qyNUD9WBpj8zwIuQf5/dqIjG3LBXy4P4AakP/h2XGtRrBp0xtInAh -ijHyl3SJCRImHJ7K2RKilTza6We/CKBk49ZCt0Xvl/T29de1ShUCWH2YWEtgvM3X -DZoTM1PRYfl61dd4s5oz9wCGzh1NlDivqOx4UXCKXBCDUSH3ET00hl7lSM2XgYI1 -TBnsZfZrxQWh7kcT1rMhJ5QQCtkkO7q+RBNGMD+XPNjX12ruOzjjK9SXDrkb5wdJ -fzcq+Xd4z1TtW0ado4AOkUPB1ltfFLqfpo0kR0BZv3I4sjZsN/+Z0V0OWQqraffA -sgRFelQArr5T9rXn4fg8ozHSqf4hUmTFpmfwdQcGlBSBVcYn5AGPF8Fqcde+S/uU -WH1+ETOxQvdibBjWzwloPn9s9h6PYq2lY9sJpx8iQkEeb5mKPtf5P0B6ebClAZLS -nT0IFaUQAS2zMnaolQ2zepr7BxB4EW/hj8e6DyUadCrlHJhBmd8hh+iVBmoKs2pH -dmX2Os+PYhcZewoozRrSgx4hxyy/vv9haLdnG7t4TY3OZ+XkwY63I2binZB1NJip -NiuKmpS5nezMirH4JYlcWrYvjB9teSSnUmjDhDXiZo1jDiVN1Rmy5nk3pyKdVDEC -AwEAAaNqMGgwHQYDVR0OBBYEFB4M97Zn8uGSJglFwFU5Lnc/QkqiMAwGA1UdEwQF -MAMBAf8wOQYEZyoHAAQxMC8wLQIBADAJBgUrDgMCGgUAMAcGBWcqAwAABBRFsMLH -ClZ87lt4DJX5GFPBphzYEDANBgkqhkiG9w0BAQUFAAOCAgEACbODU1kBPpVJufGB -uvl2ICO1J2B01GqZNF5sAFPZn/KmsSQHRGoqxqWOeBLoR9lYGxMqXnmbnwoqZ6Yl -PwZpVnPDimZI+ymBV3QGypzqKOg4ZyYr8dW1P2WT+DZdjo2NQCCHGervJ8A9tDkP -JXtoUHRVnAxZfVo9QZQlUgjgRywVMRnVvwdVxrsStZf0X4OFunHB2WyBEXYKCrC/ -gpf36j36+uwtqSiUO1bd0lEursC9CBWMd1I0ltabrNMdjmEPNXubrjlpC2JgQCA2 -j6/7Nu4tCEoduL+bXPjqpRugc6bY+G7gMwRfaKonh+3ZwZCc7b3jajWvY9+rGNm6 -5ulK6lCKD2GTHuItGeIwlDWSXQ62B68ZgI9HkFFLLk3dheLSClIKF5r8GrBQAuUB -o2M3IUxExJtRmREOc5wGj1QupyheRDmHVi03vYVElOEMSyycw5KFNGHLD7ibSkNS -/jQ6fbjpKdx2qcgw+BRxgMYeNkh0IkFch4LoGHGLQYlE535YW6i4jRPpp2zDR+2z -Gp1iro2C6pSe3VkQw63d4k3jMdXH7OjysP6SHhYKGvzZ8/gntsm+HbRsZJB/9OTE -W9c3rkIO3aQab3yIVMUWbuF6aC74Or8NpDyJO3inTmODBCEIZ43ygknQW/2xzQ+D -hNQ+IIX3Sj0rnP0qCglN6oH4EZw= ------END CERTIFICATE----- `, }, { From 97bf78725562ce22e18036873215f2203b3e0e1e Mon Sep 17 00:00:00 2001 From: qiulaidongfeng <2645477756@qq.com> Date: Sat, 25 Jan 2025 17:26:55 +0800 Subject: [PATCH 4/8] blake2b: implement hash.XOF Fixes golang/go#69518 Change-Id: Id9989ac9b28262df77017e97f985f67c1571c3ce Reviewed-on: https://go-review.googlesource.com/c/crypto/+/644255 Reviewed-by: Austin Clements Auto-Submit: Austin Clements Reviewed-by: Cherry Mui LUCI-TryBot-Result: Go LUCI --- blake2b/blake2x.go | 8 ++++++++ blake2b/go125.go | 11 +++++++++++ 2 files changed, 19 insertions(+) create mode 100644 blake2b/go125.go diff --git a/blake2b/blake2x.go b/blake2b/blake2x.go index 52c414db0e..7692bb3460 100644 --- a/blake2b/blake2x.go +++ b/blake2b/blake2x.go @@ -12,6 +12,8 @@ import ( // XOF defines the interface to hash functions that // support arbitrary-length output. +// +// New callers should prefer the standard library [hash.XOF]. type XOF interface { // Write absorbs more data into the hash's state. It panics if called // after Read. @@ -47,6 +49,8 @@ const maxOutputLength = (1 << 32) * 64 // // A non-nil key turns the hash into a MAC. The key must between // zero and 32 bytes long. +// +// The result can be safely interface-upgraded to [hash.XOF]. func NewXOF(size uint32, key []byte) (XOF, error) { if len(key) > Size { return nil, errKeySize @@ -93,6 +97,10 @@ func (x *xof) Clone() XOF { return &clone } +func (x *xof) BlockSize() int { + return x.d.BlockSize() +} + func (x *xof) Reset() { x.cfg[0] = byte(Size) binary.LittleEndian.PutUint32(x.cfg[4:], uint32(Size)) // leaf length diff --git a/blake2b/go125.go b/blake2b/go125.go new file mode 100644 index 0000000000..67e990b7e1 --- /dev/null +++ b/blake2b/go125.go @@ -0,0 +1,11 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build go1.25 + +package blake2b + +import "hash" + +var _ hash.XOF = (*xof)(nil) From 1dc4269656dd23b2c4e71c51b8af6bc2b63eecb7 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Thu, 29 May 2025 14:34:34 -0400 Subject: [PATCH 5/8] acme: add Pebble integration testing This commit adds integration test coverage for a complete TLS-ALPN-01 and HTTP-01 based issuance flow. For each tested challenge type we: * Spin up a pebble/pebble-challtestsrv environment * Spin up a small challenge response server * Create an ACME account * Create an order for multiple DNS type identifiers * Provision challenge responses based on the challenge type under test * Wait for the order to become ready for issuance * Finalize the order, issuing a certificate * Check the newly issued certificate chain validates with the Pebble trust anchor, and that the certificate is valid for each of the names from our initial order These tests are skipped in short mode (Pebble has variable delays for validation requests). The Pebble source is fetched through the Go module proxy (unless a local directory is specified to aid development), similar to how the stdlib crypto packages fetch BoGo tooling. More test coverage for various other parts of the protocol (key rollover, account/authz deactivation, revocation, etc) can be added as follow-up work now that the groundwork for integration testing is laid. Fixes golang/go#73914 Cq-Include-Trybots: luci.golang.try:x_crypto-gotip-linux-amd64-longtest Change-Id: I4e79f4858f31ef290a0c91d345e15fbdc510e9ab Reviewed-on: https://go-review.googlesource.com/c/crypto/+/677575 Reviewed-by: Roland Shoemaker Auto-Submit: Daniel McCarney Reviewed-by: Ian Stapleton Cordasco Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI --- acme/pebble_test.go | 793 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 793 insertions(+) create mode 100644 acme/pebble_test.go diff --git a/acme/pebble_test.go b/acme/pebble_test.go new file mode 100644 index 0000000000..625e20b65a --- /dev/null +++ b/acme/pebble_test.go @@ -0,0 +1,793 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package acme_test + +import ( + "bytes" + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "encoding/json" + "encoding/pem" + "errors" + "flag" + "fmt" + "io" + "log" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "golang.org/x/crypto/acme" +) + +const ( + // pebbleModVersion is the module version used for Pebble and Pebble's + // challenge test server. It is ignored if `-pebble-local-dir` is provided. + pebbleModVersion = "v2.7.0" + // startingPort is the first port number used for binding interface + // addresses. Each call to takeNextPort() will increment a port number + // starting at this value. + startingPort = 5555 +) + +var ( + pebbleLocalDir = flag.String( + "pebble-local-dir", + "", + "Local Pebble to use, instead of fetching from source", + ) + nextPort atomic.Uint32 +) + +func init() { + nextPort.Store(startingPort) +} + +func TestWithPebble(t *testing.T) { + // We want to use process groups w/ syscall.Kill, and the acme package + // is very platform-agnostic, so skip on non-Linux. + if runtime.GOOS != "linux" { + t.Skip("skipping pebble tests on non-linux OS") + } + + if testing.Short() { + t.Skip("skipping pebble tests in short mode") + } + + tests := []struct { + name string + challSrv func(*environment) (challengeServer, string) + }{ + { + name: "TLSALPN01-Issuance", + challSrv: func(env *environment) (challengeServer, string) { + bindAddr := fmt.Sprintf(":%d", env.config.TLSPort) + return newChallTLSServer(bindAddr), bindAddr + }, + }, + + { + name: "HTTP01-Issuance", + challSrv: func(env *environment) (challengeServer, string) { + bindAddr := fmt.Sprintf(":%d", env.config.HTTPPort) + return newChallHTTPServer(bindAddr), bindAddr + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + env := startPebbleEnvironment(t, nil) + challSrv, challSrvAddr := tt.challSrv(&env) + challSrv.Run() + + t.Cleanup(func() { + challSrv.Shutdown() + }) + + waitForServer(t, challSrvAddr) + testIssuance(t, &env, challSrv) + }) + } +} + +// challengeServer abstracts over the details of running a challenge response +// server for some supported acme.Challenge type. Responses are provisioned +// during the test issuance process to be presented to the ACME server's +// validation authority. +type challengeServer interface { + Run() + Shutdown() error + Supported(chal *acme.Challenge) bool + Provision(client *acme.Client, ident acme.AuthzID, chal *acme.Challenge) error +} + +// challTLSServer is a simple challenge response server that listens for TLS +// connections on a specific port and if they are TLS-ALPN-01 challenge +// requests, completes the handshake using the configured challenge response +// certificate for the SNI value provided. +type challTLSServer struct { + *http.Server + // mu protects challCerts. + mu sync.RWMutex + // challCerts is a map from SNI domain name to challenge response certificate. + challCerts map[string]*tls.Certificate +} + +// https://datatracker.ietf.org/doc/html/rfc8737#section-4 +const acmeTLSAlpnProtocol = "acme-tls/1" + +func newChallTLSServer(address string) *challTLSServer { + challServer := &challTLSServer{Server: &http.Server{ + Addr: address, + ReadTimeout: 5 * time.Second, + WriteTimeout: 5 * time.Second, + }, challCerts: make(map[string]*tls.Certificate)} + + // Configure the server to support the TLS-ALPN-01 challenge protocol + // and to use a callback for selecting the handshake certificate. + challServer.Server.TLSConfig = &tls.Config{ + NextProtos: []string{acmeTLSAlpnProtocol}, + GetCertificate: challServer.getCertificate, + } + + return challServer +} + +func (c *challTLSServer) Shutdown() error { + log.Printf("challTLSServer: shutting down") + ctx, cancel := context.WithTimeout(context.Background(), 10) + defer cancel() + return c.Server.Shutdown(ctx) +} + +func (c *challTLSServer) Run() { + go func() { + // Note: certFile and keyFile are empty because our config uses a + // GetCertificate callback. + if err := c.Server.ListenAndServeTLS("", ""); err != nil { + if !errors.Is(err, http.ErrServerClosed) { + log.Printf("challTLSServer error: %v", err) + } + } + }() +} + +func (c *challTLSServer) Supported(chal *acme.Challenge) bool { + return chal.Type == "tls-alpn-01" +} + +func (c *challTLSServer) Provision(client *acme.Client, ident acme.AuthzID, chal *acme.Challenge) error { + respCert, err := client.TLSALPN01ChallengeCert(chal.Token, ident.Value) + if err != nil { + return fmt.Errorf("challTLSServer: failed to generate challlenge response cert for %s: %w", + ident.Value, err) + } + + log.Printf("challTLSServer: setting challenge response certificate for %s", ident.Value) + c.mu.Lock() + defer c.mu.Unlock() + c.challCerts[ident.Value] = &respCert + + return nil +} + +func (c *challTLSServer) getCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { + // Verify the request looks like a TLS-ALPN-01 challenge request. + if len(clientHello.SupportedProtos) != 1 || clientHello.SupportedProtos[0] != acmeTLSAlpnProtocol { + return nil, fmt.Errorf( + "challTLSServer: non-TLS-ALPN-01 challenge request received with SupportedProtos: %s", + clientHello.SupportedProtos) + } + + serverName := clientHello.ServerName + + // TLS-ALPN-01 challenge requests for IP addresses are encoded in the SNI + // using the reverse-DNS notation. See RFC 8738 Section 6: + // https://www.rfc-editor.org/rfc/rfc8738.html#section-6 + if strings.HasSuffix(serverName, ".in-addr.arpa") { + serverName = strings.TrimSuffix(serverName, ".in-addr.arpa") + parts := strings.Split(serverName, ".") + for i, j := 0, len(parts)-1; i < j; i, j = i+1, j-1 { + parts[i], parts[j] = parts[j], parts[i] + } + serverName = strings.Join(parts, ".") + } + + log.Printf("challTLSServer: selecting certificate for request from %s for %s", + clientHello.Conn.RemoteAddr(), serverName) + + c.mu.RLock() + defer c.mu.RUnlock() + cert := c.challCerts[serverName] + if cert == nil { + return nil, fmt.Errorf("challTLSServer: no challenge response certificate configured for %s", serverName) + } + + return cert, nil +} + +// challHTTPServer is a simple challenge response server that listens for HTTP +// connections on a specific port and if they are HTTP-01 challenge requests, +// serves the challenge response key authorization. +type challHTTPServer struct { + *http.Server + // mu protects challMap + mu sync.RWMutex + // challMap is a mapping from request path to response body. + challMap map[string]string +} + +func newChallHTTPServer(address string) *challHTTPServer { + challServer := &challHTTPServer{ + Server: &http.Server{ + Addr: address, + ReadTimeout: 5 * time.Second, + WriteTimeout: 5 * time.Second, + }, + challMap: make(map[string]string), + } + + challServer.Server.Handler = challServer + + return challServer +} + +func (c *challHTTPServer) Supported(chal *acme.Challenge) bool { + return chal.Type == "http-01" +} + +func (c *challHTTPServer) Provision(client *acme.Client, ident acme.AuthzID, chall *acme.Challenge) error { + path := client.HTTP01ChallengePath(chall.Token) + body, err := client.HTTP01ChallengeResponse(chall.Token) + if err != nil { + return fmt.Errorf("failed to generate HTTP-01 challenge response for %v challenge %s token %s: %w", + ident, chall.URI, chall.Token, err) + } + + c.mu.Lock() + defer c.mu.Unlock() + log.Printf("challHTTPServer: setting challenge response for %s", path) + c.challMap[path] = body + + return nil +} + +func (c *challHTTPServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + log.Printf("challHTTPServer: handling %s to %s from %s", r.Method, r.URL.Path, r.RemoteAddr) + if r.Method != http.MethodGet { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + + c.mu.RLock() + defer c.mu.RUnlock() + response, exists := c.challMap[r.URL.Path] + + if !exists { + http.NotFound(w, r) + return + } + + w.Header().Set("Content-Type", "text/plain") + w.Write([]byte(response)) +} + +func (c *challHTTPServer) Shutdown() error { + log.Printf("challHTTPServer: shutting down") + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + return c.Server.Shutdown(ctx) +} + +func (c *challHTTPServer) Run() { + go func() { + if err := c.Server.ListenAndServe(); err != nil { + if !errors.Is(err, http.ErrServerClosed) { + log.Printf("challHTTPServer error: %v", err) + } + } + }() +} + +func testIssuance(t *testing.T, env *environment, challSrv challengeServer) { + t.Helper() + + // Bound the total issuance process by a timeout of 60 seconds. + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + // Create a new ACME account. + client := env.client + acct, err := client.Register(ctx, &acme.Account{}, acme.AcceptTOS) + if err != nil { + t.Fatalf("failed to register account: %v", err) + } + if acct.Status != acme.StatusValid { + t.Fatalf("expected new account status to be valid, got %v", acct.Status) + } + log.Printf("registered account: %s", acct.URI) + + // Create a new order for some example identifiers + identifiers := []acme.AuthzID{ + { + Type: "dns", + Value: "example.com", + }, + { + Type: "dns", + Value: "www.example.com", + }, + // TODO(@cpu): enable this identifier once IP addresses are handled correctly + // by acme.TLSALPN01ChallengeCert + /* + { + Type: "ip", + Value: "127.0.0.1", + }, + */ + } + order, err := client.AuthorizeOrder(ctx, identifiers) + if err != nil { + t.Fatalf("failed to create order for %v: %v", identifiers, err) + } + if order.Status != acme.StatusPending { + t.Fatalf("expected new order status to be pending, got %v", order.Status) + } + orderURL := order.URI + log.Printf("created order: %v", orderURL) + + // For each pending authz provision a supported challenge type's response + // with the test challenge server, and tell the ACME server to verify it. + for _, authzURL := range order.AuthzURLs { + authz, err := client.GetAuthorization(ctx, authzURL) + if err != nil { + t.Fatalf("failed to get order %s authorization %s: %v", + orderURL, authzURL, err) + } + + if authz.Status != acme.StatusPending { + continue + } + + for _, challenge := range authz.Challenges { + if challenge.Status != acme.StatusPending || !challSrv.Supported(challenge) { + continue + } + + if err := challSrv.Provision(client, authz.Identifier, challenge); err != nil { + t.Fatalf("failed to provision challenge %s: %v", challenge.URI, err) + } + + _, err = client.Accept(ctx, challenge) + if err != nil { + t.Fatalf("failed to accept order %s challenge %s: %v", + orderURL, challenge.URI, err) + } + } + } + + // Wait for the order to become ready for finalization. + order, err = client.WaitOrder(ctx, order.URI) + if err != nil { + t.Fatalf("failed to wait for order %s: %s", orderURL, err) + } + if order.Status != acme.StatusReady { + t.Fatalf("expected order %s status to be ready, got %v", + orderURL, order.Status) + } + + // Generate a certificate keypair and a CSR for the order identifiers. + certKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate certificate key: %v", err) + } + var dnsNames []string + var ipAddresses []net.IP + for _, ident := range identifiers { + switch ident.Type { + case "dns": + dnsNames = append(dnsNames, ident.Value) + case "ip": + ipAddresses = append(ipAddresses, net.ParseIP(ident.Value)) + default: + t.Fatalf("unsupported identifier type: %s", ident.Type) + } + } + csrDer, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{ + DNSNames: dnsNames, + IPAddresses: ipAddresses, + }, certKey) + if err != nil { + t.Fatalf("failed to create CSR: %v", err) + } + + // Finalize the order by creating a certificate with our CSR. + chain, _, err := client.CreateOrderCert(ctx, order.FinalizeURL, csrDer, true) + if err != nil { + t.Fatalf("failed to finalize order %s with finalize URL %s: %v", + orderURL, order.FinalizeURL, err) + } + + // Split the chain into the leaf and any intermediates. + leaf := chain[0] + intermediatesDER := chain[1:] + leafCert, err := x509.ParseCertificate(leaf) + if err != nil { + t.Fatalf("failed to parse order %s leaf certificate: %v", orderURL, err) + } + intermediates := x509.NewCertPool() + for i, intermediateDER := range intermediatesDER { + intermediate, err := x509.ParseCertificate(intermediateDER) + if err != nil { + t.Fatalf("failed to parse intermediate %d: %v", i, err) + } + intermediates.AddCert(intermediate) + } + + // Verify there is a valid path from the leaf certificate to Pebble's + // issuing root using the provided intermediate certificates. + roots, err := env.RootCert() + if err != nil { + t.Fatalf("failed to get Pebble issuer root certs: %v", err) + } + paths, err := leafCert.Verify(x509.VerifyOptions{ + Intermediates: intermediates, + Roots: roots, + }) + if err != nil { + t.Fatalf("failed to verify order %s leaf certificate: %v", orderURL, err) + } + log.Printf("verified %d path(s) from issued leaf certificate to Pebble root CA", len(paths)) + + // Also verify that the leaf cert is valid for each of the DNS names + // and IP addresses from our order's identifiers. + for _, name := range dnsNames { + if err := leafCert.VerifyHostname(name); err != nil { + t.Fatalf("failed to verify order %s leaf certificate for order DNS name %s: %v", + orderURL, name, err) + } + } + for _, ip := range ipAddresses { + if err := leafCert.VerifyHostname(ip.String()); err != nil { + t.Fatalf("failed to verify order %s leaf certificate for order IP address %s: %v", + orderURL, ip, err) + } + } +} + +type environment struct { + config *environmentConfig + client *acme.Client +} + +// RootCert returns the Pebble CA's primary issuing hierarchy root certificate. +// This is generated randomly at each startup and can be used to verify +// certificate chains issued by Pebble's ACME interface. Note that this +// is separate from the static root certificate used by the Pebble ACME +// HTTPS interface. +func (e *environment) RootCert() (*x509.CertPool, error) { + // NOTE: in the future we may want to consider the alternative chains + // returned as Link alternative headers. + rootURL := fmt.Sprintf("https://%s/roots/0", e.config.pebbleConfig.ManagementListenAddress) + resp, err := e.client.HTTPClient.Get(rootURL) + if err != nil || resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed to GET Pebble root CA from %s: %v", rootURL, err) + } + + roots := x509.NewCertPool() + rootPEM, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to parse Pebble root CA PEM: %v", err) + } + rootDERBlock, _ := pem.Decode(rootPEM) + rootCA, err := x509.ParseCertificate(rootDERBlock.Bytes) + if err != nil { + return nil, fmt.Errorf("failed to parse Pebble root CA DER: %v", err) + } + roots.AddCert(rootCA) + + return roots, nil +} + +// environmentConfig describes the Pebble configuration, and configuration +// shared between pebble and pebble-challtestsrv. +type environmentConfig struct { + pebbleConfig + dnsPort uint32 +} + +// defaultConfig returns an environmentConfig populated with default values. +// The provided pebbleDir is used to specify certificate/private key paths +// for the HTTPS ACME interface. +func defaultConfig(pebbleDir string) environmentConfig { + return environmentConfig{ + pebbleConfig: pebbleConfig{ + ListenAddress: fmt.Sprintf("127.0.0.1:%d", takeNextPort()), + ManagementListenAddress: fmt.Sprintf("127.0.0.1:%d", takeNextPort()), + HTTPPort: takeNextPort(), + TLSPort: takeNextPort(), + Certificate: fmt.Sprintf("%s/test/certs/localhost/cert.pem", pebbleDir), + PrivateKey: fmt.Sprintf("%s/test/certs/localhost/key.pem", pebbleDir), + OCSPResponderURL: "", + ExternalAccountBindingRequired: false, + ExternalAccountMACKeys: make(map[string]string), + DomainBlocklist: []string{"blocked-domain.example"}, + Profiles: map[string]struct { + Description string + ValidityPeriod uint64 + }{ + "default": { + Description: "default profile", + ValidityPeriod: 3600, + }, + }, + RetryAfter: struct { + Authz int + Order int + }{ + 3, + 5, + }, + }, + dnsPort: takeNextPort(), + } +} + +// pebbleConfig matches the JSON structure of the Pebble configuration file. +type pebbleConfig struct { + ListenAddress string + ManagementListenAddress string + HTTPPort uint32 + TLSPort uint32 + Certificate string + PrivateKey string + OCSPResponderURL string + ExternalAccountBindingRequired bool + ExternalAccountMACKeys map[string]string + DomainBlocklist []string + Profiles map[string]struct { + Description string + ValidityPeriod uint64 + } + RetryAfter struct { + Authz int + Order int + } +} + +func takeNextPort() uint32 { + return nextPort.Add(1) - 1 +} + +// startPebbleEnvironment is a test helper that spawns Pebble and Pebble +// challenge test server processes based on the provided environmentConfig. The +// processes will be torn down when the test ends. +func startPebbleEnvironment(t *testing.T, config *environmentConfig) environment { + t.Helper() + + var pebbleDir string + if *pebbleLocalDir != "" { + pebbleDir = *pebbleLocalDir + } else { + pebbleDir = fetchModule(t, "github.com/letsencrypt/pebble/v2", pebbleModVersion) + } + + binDir := prepareBinaries(t, pebbleDir) + + if config == nil { + cfg := defaultConfig(pebbleDir) + config = &cfg + } + + marshalConfig := struct { + Pebble pebbleConfig + }{ + Pebble: config.pebbleConfig, + } + + configData, err := json.Marshal(marshalConfig) + if err != nil { + t.Fatalf("failed to marshal config: %v", err) + } + + configFile, err := os.CreateTemp("", "pebble-config-*.json") + if err != nil { + t.Fatalf("failed to create temp config file: %v", err) + } + t.Cleanup(func() { os.Remove(configFile.Name()) }) + + if _, err := configFile.Write(configData); err != nil { + t.Fatalf("failed to write config file: %v", err) + } + configFile.Close() + + log.Printf("pebble dir: %s", pebbleDir) + log.Printf("config file: %s", configFile.Name()) + + // Spawn the Pebble CA server. It answers ACME requests and performs + // outbound validations. We configure it to use a mock DNS server that + // always answers 127.0.0.1 for all A queries so that validation + // requests for any domain name will resolve to our local challenge + // server instances. + spawnServerProcess(t, binDir, "pebble", "-config", configFile.Name(), + "-dnsserver", fmt.Sprintf("127.0.0.1:%d", config.dnsPort), + "-strict") + + // Spawn the Pebble challenge test server. We'll use it to mock DNS + // responses but disable all the other interfaces. We want to stand + // up our own challenge response servers for TLS-ALPN-01, + // etc. + // Note: we specify -defaultIPv6 "" so that no AAAA records are served. + // The LUCI CI runners have issues with IPv6 connectivity on localhost. + spawnServerProcess(t, binDir, "pebble-challtestsrv", + "-dns01", fmt.Sprintf(":%d", config.dnsPort), + "-defaultIPv6", "", + "-management", fmt.Sprintf(":%d", takeNextPort()), + "-doh", "", + "-http01", "", + "-tlsalpn01", "", + "-https01", "") + + waitForServer(t, config.pebbleConfig.ListenAddress) + waitForServer(t, fmt.Sprintf("127.0.0.1:%d", config.dnsPort)) + + log.Printf("pebble environment ready") + + // Construct a cert pool that contains the CA certificate used by the ACME + // interface's certificate chain. This is separate from the issuing + // hierarchy and is used for the ACME client to interact with the ACME + // interface without cert verification error. + caCertPath := filepath.Join(pebbleDir, "test/certs/pebble.minica.pem") + caCert, err := os.ReadFile(caCertPath) + if err != nil { + t.Fatalf("failed to read CA certificate %s: %v", caCertPath, err) + } + caCertPool := x509.NewCertPool() + if !caCertPool.AppendCertsFromPEM(caCert) { + t.Fatalf("failed to parse CA certificate %s", caCertPath) + } + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: caCertPool, + }, + }, + } + + // Create an ACME account keypair/client and verify it can discover + // the Pebble server's ACME directory without error. + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed to generate account key: %v", err) + } + client := &acme.Client{ + Key: key, + HTTPClient: httpClient, + DirectoryURL: fmt.Sprintf("https://%s/dir", config.ListenAddress), + } + _, err = client.Discover(context.TODO()) + if err != nil { + t.Fatalf("failed to discover ACME directory: %v", err) + } + + return environment{ + config: config, + client: client, + } +} + +func waitForServer(t *testing.T, addr string) { + t.Helper() + + for i := 0; i < 10; i++ { + if conn, err := net.Dial("tcp", addr); err == nil { + conn.Close() + return + } + time.Sleep(time.Duration(i*100) * time.Millisecond) + } + t.Fatalf("failed to connect to %q after 10 tries", addr) +} + +// fetchModule fetches the module at the given version and returns the directory +// containing its source tree. It skips the test if fetching modules is not +// possible in this environment. +// +// Copied from the stdlib cryptotest.FetchModule and adapted to not rely on the +// stdlib internal testenv package. +func fetchModule(t *testing.T, module, version string) string { + // If the default GOMODCACHE doesn't exist, use a temporary directory + // instead. (For example, run.bash sets GOPATH=/nonexist-gopath.) + out, err := exec.Command("go", "env", "GOMODCACHE").Output() + if err != nil { + t.Errorf("go env GOMODCACHE: %v\n%s", err, out) + if ee, ok := err.(*exec.ExitError); ok { + t.Logf("%s", ee.Stderr) + } + t.FailNow() + } + modcacheOk := false + if gomodcache := string(bytes.TrimSpace(out)); gomodcache != "" { + if _, err := os.Stat(gomodcache); err == nil { + modcacheOk = true + } + } + if !modcacheOk { + t.Setenv("GOMODCACHE", t.TempDir()) + // Allow t.TempDir() to clean up subdirectories. + t.Setenv("GOFLAGS", os.Getenv("GOFLAGS")+" -modcacherw") + } + + t.Logf("fetching %s@%s\n", module, version) + + output, err := exec.Command("go", "mod", "download", "-json", module+"@"+version).CombinedOutput() + if err != nil { + t.Fatalf("failed to download %s@%s: %s\n%s\n", module, version, err, output) + } + var j struct { + Dir string + } + if err := json.Unmarshal(output, &j); err != nil { + t.Fatalf("failed to parse 'go mod download': %s\n%s\n", err, output) + } + + return j.Dir +} + +func prepareBinaries(t *testing.T, pebbleDir string) string { + t.Helper() + + // We don't want to build in the module cache dir, which might not be + // writable or to pollute the user's clone with binaries if pebbleLocalDir + //is used. + binDir := t.TempDir() + + build := func(cmd string) { + log.Printf("building %s", cmd) + buildCmd := exec.Command( + "go", + "build", "-o", filepath.Join(binDir, cmd), "-mod", "mod", "./cmd/"+cmd) + buildCmd.Dir = pebbleDir + output, err := buildCmd.CombinedOutput() + if err != nil { + t.Fatalf("failed to build %s: %s\n%s\n", cmd, err, output) + } + } + + build("pebble") + build("pebble-challtestsrv") + + return binDir +} + +func spawnServerProcess(t *testing.T, dir string, cmd string, args ...string) { + t.Helper() + + cmdInstance := exec.Command("./"+cmd, args...) + cmdInstance.Dir = dir + cmdInstance.Stdout = os.Stdout + cmdInstance.Stderr = os.Stderr + if err := cmdInstance.Start(); err != nil { + t.Fatalf("failed to start %s: %v", cmd, err) + } + t.Cleanup(func() { + cmdInstance.Process.Kill() + }) +} From b3790b8d914304c8187dc2c86800101c329d77cd Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Fri, 30 May 2025 12:42:24 -0400 Subject: [PATCH 6/8] acme: fix TLSALPN01ChallengeCert for IP address identifiers When creating a TLS-ALPN-01 challenge response certificate for an IP address identifier we need to configure the template IPAddresses field, not the DNSNames/Subject.CommonName. Along the way we can do some small tidying: * Updating the draft TLS-ALPN-01 reference to the finalized RFC * Adding a reference to the IP address identifier ACME RFC * Adding a mention of the form the challenge validation request's SNI will take when verifying an IP address identifier * Tidying the private tlsChallengeCert() function to take a single identifier as arg since the only call-sites provide singular values since the removal of the TLS-SNI-[01|02] challenge helpers. This allows enabling an IP address identifier in the Pebble integration tests that otherwise caused a validation failure for TLS-ALPN-01 challenge types because the IP address was used as a DNS SAN. Updates golang/go#73914 Cq-Include-Trybots: luci.golang.try:x_crypto-gotip-linux-amd64-longtest Change-Id: Ic671e41b585f424f821db65206c7ffcc6dd386a0 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/677576 Reviewed-by: Ian Stapleton Cordasco Auto-Submit: Daniel McCarney Reviewed-by: Roland Shoemaker LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov --- acme/acme.go | 35 ++++++++++++++++++++++++----------- acme/pebble_test.go | 12 ++++-------- acme/types.go | 2 +- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/acme/acme.go b/acme/acme.go index 6be00d1d0c..7a51284f91 100644 --- a/acme/acme.go +++ b/acme/acme.go @@ -35,6 +35,7 @@ import ( "errors" "fmt" "math/big" + "net" "net/http" "strings" "sync" @@ -589,8 +590,9 @@ func (c *Client) TLSSNI02ChallengeCert(token string, opt ...CertOption) (tls.Cer // TLSALPN01ChallengeCert creates a certificate for TLS-ALPN-01 challenge response. // Servers can present the certificate to validate the challenge and prove control -// over a domain name. For more details on TLS-ALPN-01 see -// https://tools.ietf.org/html/draft-shoemaker-acme-tls-alpn-00#section-3 +// over an identifier (either a DNS name or the textual form of an IPv4 or IPv6 +// address). For more details on TLS-ALPN-01 see +// https://www.rfc-editor.org/rfc/rfc8737 and https://www.rfc-editor.org/rfc/rfc8738 // // The token argument is a Challenge.Token value. // If a WithKey option is provided, its private part signs the returned cert, @@ -598,9 +600,13 @@ func (c *Client) TLSSNI02ChallengeCert(token string, opt ...CertOption) (tls.Cer // If no WithKey option is provided, a new ECDSA key is generated using P-256 curve. // // The returned certificate is valid for the next 24 hours and must be presented only when -// the server name in the TLS ClientHello matches the domain, and the special acme-tls/1 ALPN protocol +// the server name in the TLS ClientHello matches the identifier, and the special acme-tls/1 ALPN protocol // has been specified. -func (c *Client) TLSALPN01ChallengeCert(token, domain string, opt ...CertOption) (cert tls.Certificate, err error) { +// +// Validation requests for IP address identifiers will use the reverse DNS form in the server name +// in the TLS ClientHello since the SNI extension is not supported for IP addresses. +// See RFC 8738 Section 6 for more information. +func (c *Client) TLSALPN01ChallengeCert(token, identifier string, opt ...CertOption) (cert tls.Certificate, err error) { ka, err := keyAuth(c.Key.Public(), token) if err != nil { return tls.Certificate{}, err @@ -630,7 +636,7 @@ func (c *Client) TLSALPN01ChallengeCert(token, domain string, opt ...CertOption) } tmpl.ExtraExtensions = append(tmpl.ExtraExtensions, acmeExtension) newOpt = append(newOpt, WithTemplate(tmpl)) - return tlsChallengeCert([]string{domain}, newOpt) + return tlsChallengeCert(identifier, newOpt) } // popNonce returns a nonce value previously stored with c.addNonce @@ -749,10 +755,14 @@ func defaultTLSChallengeCertTemplate() *x509.Certificate { } // tlsChallengeCert creates a temporary certificate for TLS-ALPN challenges -// with the given SANs and auto-generated public/private key pair. -// The Subject Common Name is set to the first SAN to aid debugging. +// for the given identifier, using an auto-generated public/private key pair. +// +// If the provided identifier is a domain name, it will be used as a DNS type SAN and for the +// subject common name. If the provided identifier is an IP address it will be used as an IP type +// SAN. +// // To create a cert with a custom key pair, specify WithKey option. -func tlsChallengeCert(san []string, opt []CertOption) (tls.Certificate, error) { +func tlsChallengeCert(identifier string, opt []CertOption) (tls.Certificate, error) { var key crypto.Signer tmpl := defaultTLSChallengeCertTemplate() for _, o := range opt { @@ -776,9 +786,12 @@ func tlsChallengeCert(san []string, opt []CertOption) (tls.Certificate, error) { return tls.Certificate{}, err } } - tmpl.DNSNames = san - if len(san) > 0 { - tmpl.Subject.CommonName = san[0] + + if ip := net.ParseIP(identifier); ip != nil { + tmpl.IPAddresses = []net.IP{ip} + } else { + tmpl.DNSNames = []string{identifier} + tmpl.Subject.CommonName = identifier } der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, key.Public(), key) diff --git a/acme/pebble_test.go b/acme/pebble_test.go index 625e20b65a..63e6cc5836 100644 --- a/acme/pebble_test.go +++ b/acme/pebble_test.go @@ -334,14 +334,10 @@ func testIssuance(t *testing.T, env *environment, challSrv challengeServer) { Type: "dns", Value: "www.example.com", }, - // TODO(@cpu): enable this identifier once IP addresses are handled correctly - // by acme.TLSALPN01ChallengeCert - /* - { - Type: "ip", - Value: "127.0.0.1", - }, - */ + { + Type: "ip", + Value: "127.0.0.1", + }, } order, err := client.AuthorizeOrder(ctx, identifiers) if err != nil { diff --git a/acme/types.go b/acme/types.go index 640223cb7d..c466645ca1 100644 --- a/acme/types.go +++ b/acme/types.go @@ -619,7 +619,7 @@ func (*certOptKey) privateCertOpt() {} // // In TLS ChallengeCert methods, the template is also used as parent, // resulting in a self-signed certificate. -// The DNSNames field of t is always overwritten for tls-sni challenge certs. +// The DNSNames or IPAddresses fields of t are always overwritten for tls-alpn challenge certs. func WithTemplate(t *x509.Certificate) CertOption { return (*certOptTemplate)(t) } From 74e709ad8a8068445173aa5f3e8d7c89caf510c3 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 27 Jan 2024 19:29:59 +0100 Subject: [PATCH 7/8] ssh: add AlgorithmNegotiationError Fixes golang/go#61536 Change-Id: Id38cc6d46879dbe2bdea04dec061596387ec6cfe Reviewed-on: https://go-review.googlesource.com/c/crypto/+/559056 LUCI-TryBot-Result: Go LUCI Reviewed-by: Filippo Valsorda Reviewed-by: David Chase Auto-Submit: Nicola Murino Reviewed-by: Carlos Amedee --- ssh/client_auth.go | 2 +- ssh/common.go | 45 +++++++++++++++++++++------- ssh/handshake_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 11 deletions(-) diff --git a/ssh/client_auth.go b/ssh/client_auth.go index b86dde151d..c12818fdc5 100644 --- a/ssh/client_auth.go +++ b/ssh/client_auth.go @@ -289,7 +289,7 @@ func pickSignatureAlgorithm(signer Signer, extensions map[string][]byte) (MultiA } } - algo, err := findCommon("public key signature algorithm", keyAlgos, serverAlgos) + algo, err := findCommon("public key signature algorithm", keyAlgos, serverAlgos, true) if err != nil { // If there is no overlap, return the fallback algorithm to support // servers that fail to list all supported algorithms. diff --git a/ssh/common.go b/ssh/common.go index 0415d33968..f2ec0896c2 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -336,7 +336,7 @@ func parseError(tag uint8) error { return fmt.Errorf("ssh: parse error in message type %d", tag) } -func findCommon(what string, client []string, server []string) (common string, err error) { +func findCommon(what string, client []string, server []string, isClient bool) (string, error) { for _, c := range client { for _, s := range server { if c == s { @@ -344,7 +344,32 @@ func findCommon(what string, client []string, server []string) (common string, e } } } - return "", fmt.Errorf("ssh: no common algorithm for %s; client offered: %v, server offered: %v", what, client, server) + err := &AlgorithmNegotiationError{ + What: what, + } + if isClient { + err.SupportedAlgorithms = client + err.RequestedAlgorithms = server + } else { + err.SupportedAlgorithms = server + err.RequestedAlgorithms = client + } + return "", err +} + +// AlgorithmNegotiationError defines the error returned if the client and the +// server cannot agree on an algorithm for key exchange, host key, cipher, MAC. +type AlgorithmNegotiationError struct { + What string + // RequestedAlgorithms lists the algorithms supported by the peer. + RequestedAlgorithms []string + // SupportedAlgorithms lists the algorithms supported on our side. + SupportedAlgorithms []string +} + +func (a *AlgorithmNegotiationError) Error() string { + return fmt.Sprintf("ssh: no common algorithm for %s; we offered: %v, peer offered: %v", + a.What, a.SupportedAlgorithms, a.RequestedAlgorithms) } // DirectionAlgorithms defines the algorithms negotiated in one direction @@ -379,12 +404,12 @@ var aeadCiphers = map[string]bool{ func findAgreedAlgorithms(isClient bool, clientKexInit, serverKexInit *kexInitMsg) (algs *NegotiatedAlgorithms, err error) { result := &NegotiatedAlgorithms{} - result.KeyExchange, err = findCommon("key exchange", clientKexInit.KexAlgos, serverKexInit.KexAlgos) + result.KeyExchange, err = findCommon("key exchange", clientKexInit.KexAlgos, serverKexInit.KexAlgos, isClient) if err != nil { return } - result.HostKey, err = findCommon("host key", clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos) + result.HostKey, err = findCommon("host key", clientKexInit.ServerHostKeyAlgos, serverKexInit.ServerHostKeyAlgos, isClient) if err != nil { return } @@ -394,36 +419,36 @@ func findAgreedAlgorithms(isClient bool, clientKexInit, serverKexInit *kexInitMs ctos, stoc = stoc, ctos } - ctos.Cipher, err = findCommon("client to server cipher", clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer) + ctos.Cipher, err = findCommon("client to server cipher", clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer, isClient) if err != nil { return } - stoc.Cipher, err = findCommon("server to client cipher", clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient) + stoc.Cipher, err = findCommon("server to client cipher", clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient, isClient) if err != nil { return } if !aeadCiphers[ctos.Cipher] { - ctos.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer) + ctos.MAC, err = findCommon("client to server MAC", clientKexInit.MACsClientServer, serverKexInit.MACsClientServer, isClient) if err != nil { return } } if !aeadCiphers[stoc.Cipher] { - stoc.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient) + stoc.MAC, err = findCommon("server to client MAC", clientKexInit.MACsServerClient, serverKexInit.MACsServerClient, isClient) if err != nil { return } } - ctos.compression, err = findCommon("client to server compression", clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer) + ctos.compression, err = findCommon("client to server compression", clientKexInit.CompressionClientServer, serverKexInit.CompressionClientServer, isClient) if err != nil { return } - stoc.compression, err = findCommon("server to client compression", clientKexInit.CompressionServerClient, serverKexInit.CompressionServerClient) + stoc.compression, err = findCommon("server to client compression", clientKexInit.CompressionServerClient, serverKexInit.CompressionServerClient, isClient) if err != nil { return } diff --git a/ssh/handshake_test.go b/ssh/handshake_test.go index 7f0ecef05f..61f978491f 100644 --- a/ssh/handshake_test.go +++ b/ssh/handshake_test.go @@ -1294,3 +1294,73 @@ func TestNegotiatedAlgorithms(t *testing.T) { } } } + +func TestAlgorithmNegotiationError(t *testing.T) { + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + serverConf := &ServerConfig{ + Config: Config{ + Ciphers: []string{CipherAES128CTR, CipherAES256CTR}, + }, + PasswordCallback: func(conn ConnMetadata, password []byte) (*Permissions, error) { + return &Permissions{}, nil + }, + } + serverConf.AddHostKey(testSigners["rsa"]) + + srvErrCh := make(chan error, 1) + go func() { + _, _, _, err := NewServerConn(c1, serverConf) + srvErrCh <- err + }() + + clientConf := &ClientConfig{ + Config: Config{ + Ciphers: []string{CipherAES128GCM, CipherAES256GCM}, + }, + User: "test", + Auth: []AuthMethod{Password("testpw")}, + HostKeyCallback: FixedHostKey(testSigners["rsa"].PublicKey()), + } + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err == nil { + t.Fatal("client connection succeded expected algorithm negotiation error") + } + var negotiationError *AlgorithmNegotiationError + if !errors.As(err, &negotiationError) { + t.Fatalf("expected algorithm negotiation error, got %v", err) + } + expectedErrorString := fmt.Sprintf("ssh: handshake failed: ssh: no common algorithm for client to server cipher; we offered: %v, peer offered: %v", + clientConf.Ciphers, serverConf.Ciphers) + if err.Error() != expectedErrorString { + t.Fatalf("expected error string %q, got %q", expectedErrorString, err.Error()) + } + if !reflect.DeepEqual(negotiationError.RequestedAlgorithms, serverConf.Ciphers) { + t.Fatalf("expected requested algorithms %v, got %v", serverConf.Ciphers, negotiationError.RequestedAlgorithms) + } + if !reflect.DeepEqual(negotiationError.SupportedAlgorithms, clientConf.Ciphers) { + t.Fatalf("expected supported algorithms %v, got %v", clientConf.Ciphers, negotiationError.SupportedAlgorithms) + } + err = <-srvErrCh + negotiationError = nil + if !errors.As(err, &negotiationError) { + t.Fatalf("expected algorithm negotiation error, got %v", err) + } + expectedErrorString = fmt.Sprintf("ssh: no common algorithm for client to server cipher; we offered: %v, peer offered: %v", + serverConf.Ciphers, clientConf.Ciphers) + if err.Error() != expectedErrorString { + t.Fatalf("expected error string %q, got %q", expectedErrorString, err.Error()) + } + if !reflect.DeepEqual(negotiationError.RequestedAlgorithms, clientConf.Ciphers) { + t.Fatalf("expected requested algorithms %v, got %v", clientConf.Ciphers, negotiationError.RequestedAlgorithms) + } + if !reflect.DeepEqual(negotiationError.SupportedAlgorithms, serverConf.Ciphers) { + t.Fatalf("expected supported algorithms %v, got %v", serverConf.Ciphers, negotiationError.SupportedAlgorithms) + } +} From 459a9db11b9c43bb1d61722bfd371751d6de05c9 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Wed, 9 Jul 2025 14:06:42 -0700 Subject: [PATCH 8/8] go.mod: update golang.org/x dependencies Update golang.org/x dependencies to their latest tagged versions. Change-Id: I3a89b1890ad2f7d2b2c23e1efce60c19e43dd381 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/687017 Reviewed-by: Dmitri Shuralyov Auto-Submit: Gopher Robot LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-by: David Chase --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 90256c0493..5cd0c6410d 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module golang.org/x/crypto go 1.23.0 require ( - golang.org/x/net v0.21.0 // tagx:ignore - golang.org/x/sys v0.33.0 - golang.org/x/term v0.32.0 + golang.org/x/net v0.41.0 // tagx:ignore + golang.org/x/sys v0.34.0 + golang.org/x/term v0.33.0 ) -require golang.org/x/text v0.26.0 // indirect +require golang.org/x/text v0.27.0 // indirect diff --git a/go.sum b/go.sum index 0eb4cba189..93d2855a8c 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= -golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= -golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= -golang.org/x/term v0.32.0 h1:DR4lr0TjUs3epypdhTOkMmuF5CDFJ/8pOnbzMZPQ7bg= -golang.org/x/term v0.32.0/go.mod h1:uZG1FhGx848Sqfsq4/DlJr3xGGsYMu/L5GW4abiaEPQ= -golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= -golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= +golang.org/x/net v0.41.0 h1:vBTly1HeNPEn3wtREYfy4GZ/NECgw2Cnl+nK6Nz3uvw= +golang.org/x/net v0.41.0/go.mod h1:B/K4NNqkfmg07DQYrbwvSluqCJOOXwUjeb/5lOisjbA= +golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA= +golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/term v0.33.0 h1:NuFncQrRcaRvVmgRkvM3j/F00gWIAlcmlB8ACEKmGIg= +golang.org/x/term v0.33.0/go.mod h1:s18+ql9tYWp1IfpV9DmCtQDDSRBUjKaw9M1eAv5UeF0= +golang.org/x/text v0.27.0 h1:4fGWRpyh641NLlecmyl4LOe6yDdfaYNrGb2zdfo4JV4= +golang.org/x/text v0.27.0/go.mod h1:1D28KMCvyooCX9hBiosv5Tz/+YLxj0j7XhWjpSUF7CU=