diff --git a/VERSION b/VERSION index cf1f2f4e..4097adc4 100644 --- a/VERSION +++ b/VERSION @@ -1,2 +1,2 @@ -go1.25.1 -time 2025-08-27T15:49:40Z +go1.25.2 +time 2025-10-02T18:00:14Z diff --git a/doc/go_spec.html b/doc/go_spec.html index 183bc7fb..2b47fb2e 100644 --- a/doc/go_spec.html +++ b/doc/go_spec.html @@ -1,6 +1,6 @@ diff --git a/doc/godebug.md b/doc/godebug.md index aaa0f9dd..c12ce531 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -153,6 +153,16 @@ for example, see the [runtime documentation](/pkg/runtime#hdr-Environment_Variables) and the [go command documentation](/cmd/go#hdr-Build_and_test_caching). +### Go 1.26 + +Go 1.26 added a new `httpcookiemaxnum` setting that controls the maximum number +of cookies that net/http will accept when parsing HTTP headers. If the number of +cookie in a header exceeds the number set in `httpcookiemaxnum`, cookie parsing +will fail early. The default value is `httpcookiemaxnum=3000`. Setting +`httpcookiemaxnum=0` will allow the cookie parsing to accept an indefinite +number of cookies. To avoid denial of service attacks, this setting and default +was backported to Go 1.25.2 and Go 1.24.8. + ### Go 1.25 Go 1.25 added a new `decoratemappings` setting that controls whether the Go diff --git a/lib/fips140/fips140.sum b/lib/fips140/fips140.sum index 66b1e23d..703d1dc6 100644 --- a/lib/fips140/fips140.sum +++ b/lib/fips140/fips140.sum @@ -9,4 +9,4 @@ # # go test cmd/go/internal/fips140 -update # -v1.0.0.zip b50508feaeff05d22516b21e1fd210bbf5d6a1e422eaf2cfa23fe379342713b8 +v1.0.0-c2097c7c.zip daf3614e0406f67ae6323c902db3f953a1effb199142362a039e7526dfb9368b diff --git a/lib/fips140/inprocess.txt b/lib/fips140/inprocess.txt index 0ec25f75..efd3caba 100644 --- a/lib/fips140/inprocess.txt +++ b/lib/fips140/inprocess.txt @@ -1 +1 @@ -v1.0.0 +v1.0.0-c2097c7c diff --git a/lib/fips140/v1.0.0.zip b/lib/fips140/v1.0.0-c2097c7c.zip similarity index 78% rename from lib/fips140/v1.0.0.zip rename to lib/fips140/v1.0.0-c2097c7c.zip index bd9d3c19..aabf762d 100644 Binary files a/lib/fips140/v1.0.0.zip and b/lib/fips140/v1.0.0-c2097c7c.zip differ diff --git a/lib/fips140/v1.0.0.txt b/lib/fips140/v1.0.0.txt new file mode 100644 index 00000000..efd3caba --- /dev/null +++ b/lib/fips140/v1.0.0.txt @@ -0,0 +1 @@ +v1.0.0-c2097c7c diff --git a/src/archive/tar/common.go b/src/archive/tar/common.go index 7b3945ff..ad31bbb6 100644 --- a/src/archive/tar/common.go +++ b/src/archive/tar/common.go @@ -39,6 +39,7 @@ var ( errMissData = errors.New("archive/tar: sparse file references non-existent data") errUnrefData = errors.New("archive/tar: sparse file contains unreferenced data") errWriteHole = errors.New("archive/tar: write non-NUL byte in sparse hole") + errSparseTooLong = errors.New("archive/tar: sparse map too long") ) type headerError []string diff --git a/src/archive/tar/reader.go b/src/archive/tar/reader.go index 8483fb52..16ac2f5b 100644 --- a/src/archive/tar/reader.go +++ b/src/archive/tar/reader.go @@ -531,12 +531,17 @@ func readGNUSparseMap1x0(r io.Reader) (sparseDatas, error) { cntNewline int64 buf bytes.Buffer blk block + totalSize int ) // feedTokens copies data in blocks from r into buf until there are // at least cnt newlines in buf. It will not read more blocks than needed. feedTokens := func(n int64) error { for cntNewline < n { + totalSize += len(blk) + if totalSize > maxSpecialFileSize { + return errSparseTooLong + } if _, err := mustReadFull(r, blk[:]); err != nil { return err } @@ -569,8 +574,8 @@ func readGNUSparseMap1x0(r io.Reader) (sparseDatas, error) { } // Parse for all member entries. - // numEntries is trusted after this since a potential attacker must have - // committed resources proportional to what this library used. + // numEntries is trusted after this since feedTokens limits the number of + // tokens based on maxSpecialFileSize. if err := feedTokens(2 * numEntries); err != nil { return nil, err } diff --git a/src/archive/tar/reader_test.go b/src/archive/tar/reader_test.go index 99340a30..fca53dae 100644 --- a/src/archive/tar/reader_test.go +++ b/src/archive/tar/reader_test.go @@ -621,6 +621,11 @@ func TestReader(t *testing.T) { }, Format: FormatPAX, }}, + }, { + // Small compressed file that uncompresses to + // a file with a very large GNU 1.0 sparse map. + file: "testdata/gnu-sparse-many-zeros.tar.bz2", + err: errSparseTooLong, }} for _, v := range vectors { diff --git a/src/archive/tar/testdata/gnu-sparse-many-zeros.tar.bz2 b/src/archive/tar/testdata/gnu-sparse-many-zeros.tar.bz2 new file mode 100644 index 00000000..751d7fd4 Binary files /dev/null and b/src/archive/tar/testdata/gnu-sparse-many-zeros.tar.bz2 differ diff --git a/src/cmd/compile/internal/dwarfgen/dwarf.go b/src/cmd/compile/internal/dwarfgen/dwarf.go index 7d75c0c5..4ded846a 100644 --- a/src/cmd/compile/internal/dwarfgen/dwarf.go +++ b/src/cmd/compile/internal/dwarfgen/dwarf.go @@ -128,14 +128,29 @@ func Info(ctxt *obj.Link, fnsym *obj.LSym, infosym *obj.LSym, curfn obj.Func) (s // already referenced by a dwarf var, attach an R_USETYPE relocation to // the function symbol to insure that the type included in DWARF // processing during linking. + // Do the same with R_USEIFACE relocations from the function symbol for the + // same reason. + // All these R_USETYPE relocations are only looked at if the function + // survives deadcode elimination in the linker. typesyms := []*obj.LSym{} for t := range fnsym.Func().Autot { typesyms = append(typesyms, t) } + for i := range fnsym.R { + if fnsym.R[i].Type == objabi.R_USEIFACE && !strings.HasPrefix(fnsym.R[i].Sym.Name, "go:itab.") { + // Types referenced through itab will be referenced from somewhere else + typesyms = append(typesyms, fnsym.R[i].Sym) + } + } slices.SortFunc(typesyms, func(a, b *obj.LSym) int { return strings.Compare(a.Name, b.Name) }) + var lastsym *obj.LSym for _, sym := range typesyms { + if sym == lastsym { + continue + } + lastsym = sym infosym.AddRel(ctxt, obj.Reloc{Type: objabi.R_USETYPE, Sym: sym}) } fnsym.Func().Autot = nil diff --git a/src/cmd/compile/internal/ssa/tighten.go b/src/cmd/compile/internal/ssa/tighten.go index eb5007b2..0a4b56d5 100644 --- a/src/cmd/compile/internal/ssa/tighten.go +++ b/src/cmd/compile/internal/ssa/tighten.go @@ -124,18 +124,21 @@ func tighten(f *Func) { // If the target location is inside a loop, // move the target location up to just before the loop head. - for _, b := range f.Blocks { - origloop := loops.b2l[b.ID] - for _, v := range b.Values { - t := target[v.ID] - if t == nil { - continue - } - targetloop := loops.b2l[t.ID] - for targetloop != nil && (origloop == nil || targetloop.depth > origloop.depth) { - t = idom[targetloop.header.ID] - target[v.ID] = t - targetloop = loops.b2l[t.ID] + if !loops.hasIrreducible { + // Loop info might not be correct for irreducible loops. See issue 75569. + for _, b := range f.Blocks { + origloop := loops.b2l[b.ID] + for _, v := range b.Values { + t := target[v.ID] + if t == nil { + continue + } + targetloop := loops.b2l[t.ID] + for targetloop != nil && (origloop == nil || targetloop.depth > origloop.depth) { + t = idom[targetloop.header.ID] + target[v.ID] = t + targetloop = loops.b2l[t.ID] + } } } } diff --git a/src/cmd/go/internal/fips140/mkzip.go b/src/cmd/go/internal/fips140/mkzip.go index 7a6ba803..a139a0f2 100644 --- a/src/cmd/go/internal/fips140/mkzip.go +++ b/src/cmd/go/internal/fips140/mkzip.go @@ -27,10 +27,10 @@ import ( "log" "os" "path/filepath" - "regexp" "strings" "golang.org/x/mod/module" + "golang.org/x/mod/semver" modzip "golang.org/x/mod/zip" ) @@ -61,7 +61,7 @@ func main() { // Must have valid version, and must not overwrite existing file. version := flag.Arg(0) - if !regexp.MustCompile(`^v\d+\.\d+\.\d+$`).MatchString(version) { + if semver.Canonical(version) != version { log.Fatalf("invalid version %q; must be vX.Y.Z", version) } if _, err := os.Stat(version + ".zip"); err == nil { @@ -117,7 +117,9 @@ func main() { if !bytes.Contains(contents, []byte(returnLine)) { log.Fatalf("did not find %q in fips140.go", returnLine) } - newLine := `return "` + version + `"` + // Use only the vX.Y.Z part of a possible vX.Y.Z-hash version. + v, _, _ := strings.Cut(version, "-") + newLine := `return "` + v + `"` contents = bytes.ReplaceAll(contents, []byte(returnLine), []byte(newLine)) wf, err := zw.Create(f.Name) if err != nil { diff --git a/src/cmd/go/testdata/script/fipssnap.txt b/src/cmd/go/testdata/script/fipssnap.txt index 9888bc82..4d96aedf 100644 --- a/src/cmd/go/testdata/script/fipssnap.txt +++ b/src/cmd/go/testdata/script/fipssnap.txt @@ -1,4 +1,4 @@ -env snap=v1.0.0 +env snap=v1.0.0-c2097c7c env alias=inprocess env GOFIPS140=$snap @@ -23,8 +23,7 @@ stdout crypto/internal/fips140/$snap/sha256 ! stdout crypto/internal/fips140/check # again with GOFIPS140=$alias -# TODO: enable when we add inprocess.txt -# env GOFIPS140=$alias +env GOFIPS140=$alias # default GODEBUG includes fips140=on go list -f '{{.DefaultGODEBUG}}' diff --git a/src/context/context.go b/src/context/context.go index 4f150f6a..24bb18ab 100644 --- a/src/context/context.go +++ b/src/context/context.go @@ -463,6 +463,8 @@ func (c *cancelCtx) Done() <-chan struct{} { func (c *cancelCtx) Err() error { // An atomic load is ~5x faster than a mutex, which can matter in tight loops. if err := c.err.Load(); err != nil { + // Ensure the done channel has been closed before returning a non-nil error. + <-c.Done() return err.(error) } return nil diff --git a/src/context/x_test.go b/src/context/x_test.go index 937cab14..0cf19688 100644 --- a/src/context/x_test.go +++ b/src/context/x_test.go @@ -1177,3 +1177,23 @@ func (c *customContext) Err() error { func (c *customContext) Value(key any) any { return c.parent.Value(key) } + +// Issue #75533. +func TestContextErrDoneRace(t *testing.T) { + // 4 iterations reliably reproduced #75533. + for range 10 { + ctx, cancel := WithCancel(Background()) + donec := ctx.Done() + go cancel() + for ctx.Err() == nil { + if runtime.GOARCH == "wasm" { + runtime.Gosched() // need to explicitly yield + } + } + select { + case <-donec: + default: + t.Fatalf("ctx.Err is non-nil, but ctx.Done is not closed") + } + } +} diff --git a/src/crypto/internal/cryptotest/hash.go b/src/crypto/internal/cryptotest/hash.go index f00e9c80..37fd96a2 100644 --- a/src/crypto/internal/cryptotest/hash.go +++ b/src/crypto/internal/cryptotest/hash.go @@ -20,7 +20,7 @@ type MakeHash func() hash.Hash // TestHash performs a set of tests on hash.Hash implementations, checking the // documented requirements of Write, Sum, Reset, Size, and BlockSize. func TestHash(t *testing.T, mh MakeHash) { - if boring.Enabled || fips140.Version() == "v1.0" { + if boring.Enabled || fips140.Version() == "v1.0.0" { testhash.TestHashWithoutClone(t, testhash.MakeHash(mh)) return } diff --git a/src/crypto/internal/fips140/cast.go b/src/crypto/internal/fips140/cast.go index 66e21d8a..3968dcad 100644 --- a/src/crypto/internal/fips140/cast.go +++ b/src/crypto/internal/fips140/cast.go @@ -56,9 +56,10 @@ func CAST(name string, f func() error) { } // PCT runs the named Pairwise Consistency Test (if operated in FIPS mode) and -// returns any errors. If an error is returned, the key must not be used. +// aborts the program (stopping the module input/output and entering the "error +// state") if the test fails. // -// PCTs are mandatory for every key pair that is generated/imported, including +// PCTs are mandatory for every generated (but not imported) key pair, including // ephemeral keys (which effectively doubles the cost of key establishment). See // Implementation Guidance 10.3.A Additional Comment 1. // @@ -66,17 +67,23 @@ func CAST(name string, f func() error) { // // If a package p calls PCT during key generation, an invocation of that // function should be added to fipstest.TestConditionals. -func PCT(name string, f func() error) error { +func PCT(name string, f func() error) { if strings.ContainsAny(name, ",#=:") { panic("fips: invalid self-test name: " + name) } if !Enabled { - return nil + return } err := f() if name == failfipscast { err = errors.New("simulated PCT failure") } - return err + if err != nil { + fatal("FIPS 140-3 self-test failed: " + name + ": " + err.Error()) + panic("unreachable") + } + if debug { + println("FIPS 140-3 PCT passed:", name) + } } diff --git a/src/crypto/internal/fips140/ecdh/ecdh.go b/src/crypto/internal/fips140/ecdh/ecdh.go index bf71c75a..967032aa 100644 --- a/src/crypto/internal/fips140/ecdh/ecdh.go +++ b/src/crypto/internal/fips140/ecdh/ecdh.go @@ -161,6 +161,27 @@ func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) { if err != nil { continue } + + // A "Pairwise Consistency Test" makes no sense if we just generated the + // public key from an ephemeral private key. Moreover, there is no way to + // check it aside from redoing the exact same computation again. SP 800-56A + // Rev. 3, Section 5.6.2.1.4 acknowledges that, and doesn't require it. + // However, ISO 19790:2012, Section 7.10.3.3 has a blanket requirement for a + // PCT for all generated keys (AS10.35) and FIPS 140-3 IG 10.3.A, Additional + // Comment 1 goes out of its way to say that "the PCT shall be performed + // consistent [...], even if the underlying standard does not require a + // PCT". So we do it. And make ECDH nearly 50% slower (only) in FIPS mode. + fips140.PCT("ECDH PCT", func() error { + p1, err := c.newPoint().ScalarBaseMult(privateKey.d) + if err != nil { + return err + } + if !bytes.Equal(p1.Bytes(), privateKey.pub.q) { + return errors.New("crypto/ecdh: public key does not match private key") + } + return nil + }) + return privateKey, nil } } @@ -188,28 +209,6 @@ func NewPrivateKey[P Point[P]](c *Curve[P], key []byte) (*PrivateKey, error) { panic("crypto/ecdh: internal error: public key is the identity element") } - // A "Pairwise Consistency Test" makes no sense if we just generated the - // public key from an ephemeral private key. Moreover, there is no way to - // check it aside from redoing the exact same computation again. SP 800-56A - // Rev. 3, Section 5.6.2.1.4 acknowledges that, and doesn't require it. - // However, ISO 19790:2012, Section 7.10.3.3 has a blanket requirement for a - // PCT for all generated keys (AS10.35) and FIPS 140-3 IG 10.3.A, Additional - // Comment 1 goes out of its way to say that "the PCT shall be performed - // consistent [...], even if the underlying standard does not require a - // PCT". So we do it. And make ECDH nearly 50% slower (only) in FIPS mode. - if err := fips140.PCT("ECDH PCT", func() error { - p1, err := c.newPoint().ScalarBaseMult(key) - if err != nil { - return err - } - if !bytes.Equal(p1.Bytes(), publicKey) { - return errors.New("crypto/ecdh: public key does not match private key") - } - return nil - }); err != nil { - panic(err) - } - k := &PrivateKey{d: bytes.Clone(key), pub: PublicKey{curve: c.curve, q: publicKey}} return k, nil } diff --git a/src/crypto/internal/fips140/ecdsa/cast.go b/src/crypto/internal/fips140/ecdsa/cast.go index 219b7211..6bc9fd1f 100644 --- a/src/crypto/internal/fips140/ecdsa/cast.go +++ b/src/crypto/internal/fips140/ecdsa/cast.go @@ -51,8 +51,8 @@ func testHash() []byte { } } -func fipsPCT[P Point[P]](c *Curve[P], k *PrivateKey) error { - return fips140.PCT("ECDSA PCT", func() error { +func fipsPCT[P Point[P]](c *Curve[P], k *PrivateKey) { + fips140.PCT("ECDSA PCT", func() error { hash := testHash() drbg := newDRBG(sha512.New, k.d, bits2octets(P256(), hash), nil) sig, err := sign(c, k, drbg, hash) diff --git a/src/crypto/internal/fips140/ecdsa/ecdsa.go b/src/crypto/internal/fips140/ecdsa/ecdsa.go index 47c1b244..81179de4 100644 --- a/src/crypto/internal/fips140/ecdsa/ecdsa.go +++ b/src/crypto/internal/fips140/ecdsa/ecdsa.go @@ -167,11 +167,6 @@ func NewPrivateKey[P Point[P]](c *Curve[P], D, Q []byte) (*PrivateKey, error) { return nil, err } priv := &PrivateKey{pub: *pub, d: d.Bytes(c.N)} - if err := fipsPCT(c, priv); err != nil { - // This can happen if the application went out of its way to make an - // ecdsa.PrivateKey with a mismatching PublicKey. - return nil, err - } return priv, nil } @@ -204,10 +199,7 @@ func GenerateKey[P Point[P]](c *Curve[P], rand io.Reader) (*PrivateKey, error) { }, d: k.Bytes(c.N), } - if err := fipsPCT(c, priv); err != nil { - // This clearly can't happen, but FIPS 140-3 mandates that we check it. - panic(err) - } + fipsPCT(c, priv) return priv, nil } diff --git a/src/crypto/internal/fips140/ecdsa/hmacdrbg.go b/src/crypto/internal/fips140/ecdsa/hmacdrbg.go index fa82ce39..698c23bc 100644 --- a/src/crypto/internal/fips140/ecdsa/hmacdrbg.go +++ b/src/crypto/internal/fips140/ecdsa/hmacdrbg.go @@ -122,7 +122,7 @@ func newDRBG[H hash.Hash](hash func() H, entropy, nonce []byte, s personalizatio // // This should only be used for ACVP testing. hmacDRBG is not intended to be // used directly. -func TestingOnlyNewDRBG(hash func() hash.Hash, entropy, nonce []byte, s []byte) *hmacDRBG { +func TestingOnlyNewDRBG[H hash.Hash](hash func() H, entropy, nonce []byte, s []byte) *hmacDRBG { return newDRBG(hash, entropy, nonce, plainPersonalizationString(s)) } diff --git a/src/crypto/internal/fips140/ed25519/cast.go b/src/crypto/internal/fips140/ed25519/cast.go index a680c251..2a3426bd 100644 --- a/src/crypto/internal/fips140/ed25519/cast.go +++ b/src/crypto/internal/fips140/ed25519/cast.go @@ -12,8 +12,8 @@ import ( "sync" ) -func fipsPCT(k *PrivateKey) error { - return fips140.PCT("Ed25519 sign and verify PCT", func() error { +func fipsPCT(k *PrivateKey) { + fips140.PCT("Ed25519 sign and verify PCT", func() error { return pairwiseTest(k) }) } diff --git a/src/crypto/internal/fips140/ed25519/ed25519.go b/src/crypto/internal/fips140/ed25519/ed25519.go index bbdc5b4a..8beda341 100644 --- a/src/crypto/internal/fips140/ed25519/ed25519.go +++ b/src/crypto/internal/fips140/ed25519/ed25519.go @@ -69,10 +69,7 @@ func generateKey(priv *PrivateKey) (*PrivateKey, error) { fips140.RecordApproved() drbg.Read(priv.seed[:]) precomputePrivateKey(priv) - if err := fipsPCT(priv); err != nil { - // This clearly can't happen, but FIPS 140-3 requires that we check. - panic(err) - } + fipsPCT(priv) return priv, nil } @@ -88,10 +85,6 @@ func newPrivateKeyFromSeed(priv *PrivateKey, seed []byte) (*PrivateKey, error) { } copy(priv.seed[:], seed) precomputePrivateKey(priv) - if err := fipsPCT(priv); err != nil { - // This clearly can't happen, but FIPS 140-3 requires that we check. - panic(err) - } return priv, nil } @@ -137,12 +130,6 @@ func newPrivateKey(priv *PrivateKey, privBytes []byte) (*PrivateKey, error) { copy(priv.prefix[:], h[32:]) - if err := fipsPCT(priv); err != nil { - // This can happen if the application messed with the private key - // encoding, and the public key doesn't match the seed anymore. - return nil, err - } - return priv, nil } diff --git a/src/crypto/internal/fips140/fips140.go b/src/crypto/internal/fips140/fips140.go index 050967f4..e48706fb 100644 --- a/src/crypto/internal/fips140/fips140.go +++ b/src/crypto/internal/fips140/fips140.go @@ -7,7 +7,6 @@ package fips140 import ( "crypto/internal/fips140deps/godebug" "errors" - "hash" "runtime" ) @@ -63,16 +62,10 @@ func Name() string { return "Go Cryptographic Module" } -// Version returns the formal version (such as "v1.0") if building against a +// Version returns the formal version (such as "v1.0.0") if building against a // frozen module with GOFIPS140. Otherwise, it returns "latest". func Version() string { // This return value is replaced by mkzip.go, it must not be changed or // moved to a different file. return "latest" //mkzip:version } - -// Hash is a legacy compatibility alias for hash.Hash. -// -// It's only here because [crypto/internal/fips140/ecdsa.TestingOnlyNewDRBG] -// takes a "func() fips140.Hash" in v1.0.0, instead of being generic. -type Hash = hash.Hash diff --git a/src/crypto/internal/fips140/mlkem/mlkem1024.go b/src/crypto/internal/fips140/mlkem/mlkem1024.go index 034bf3b5..1419cf20 100644 --- a/src/crypto/internal/fips140/mlkem/mlkem1024.go +++ b/src/crypto/internal/fips140/mlkem/mlkem1024.go @@ -118,10 +118,7 @@ func generateKey1024(dk *DecapsulationKey1024) (*DecapsulationKey1024, error) { var z [32]byte drbg.Read(z[:]) kemKeyGen1024(dk, &d, &z) - if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT1024(dk) }); err != nil { - // This clearly can't happen, but FIPS 140-3 requires us to check. - panic(err) - } + fips140.PCT("ML-KEM PCT", func() error { return kemPCT1024(dk) }) fips140.RecordApproved() return dk, nil } @@ -149,10 +146,6 @@ func newKeyFromSeed1024(dk *DecapsulationKey1024, seed []byte) (*DecapsulationKe d := (*[32]byte)(seed[:32]) z := (*[32]byte)(seed[32:]) kemKeyGen1024(dk, d, z) - if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT1024(dk) }); err != nil { - // This clearly can't happen, but FIPS 140-3 requires us to check. - panic(err) - } fips140.RecordApproved() return dk, nil } diff --git a/src/crypto/internal/fips140/mlkem/mlkem768.go b/src/crypto/internal/fips140/mlkem/mlkem768.go index 77043830..298660e4 100644 --- a/src/crypto/internal/fips140/mlkem/mlkem768.go +++ b/src/crypto/internal/fips140/mlkem/mlkem768.go @@ -177,10 +177,7 @@ func generateKey(dk *DecapsulationKey768) (*DecapsulationKey768, error) { var z [32]byte drbg.Read(z[:]) kemKeyGen(dk, &d, &z) - if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT(dk) }); err != nil { - // This clearly can't happen, but FIPS 140-3 requires us to check. - panic(err) - } + fips140.PCT("ML-KEM PCT", func() error { return kemPCT(dk) }) fips140.RecordApproved() return dk, nil } @@ -208,10 +205,6 @@ func newKeyFromSeed(dk *DecapsulationKey768, seed []byte) (*DecapsulationKey768, d := (*[32]byte)(seed[:32]) z := (*[32]byte)(seed[32:]) kemKeyGen(dk, d, z) - if err := fips140.PCT("ML-KEM PCT", func() error { return kemPCT(dk) }); err != nil { - // This clearly can't happen, but FIPS 140-3 requires us to check. - panic(err) - } fips140.RecordApproved() return dk, nil } diff --git a/src/crypto/internal/fips140/rsa/keygen.go b/src/crypto/internal/fips140/rsa/keygen.go index 7c027223..00b325d2 100644 --- a/src/crypto/internal/fips140/rsa/keygen.go +++ b/src/crypto/internal/fips140/rsa/keygen.go @@ -105,7 +105,28 @@ func GenerateKey(rand io.Reader, bits int) (*PrivateKey, error) { // negligible chance of failure we can defer the check to the end of key // generation and return an error if it fails. See [checkPrivateKey]. - return newPrivateKey(N, 65537, d, P, Q) + k, err := newPrivateKey(N, 65537, d, P, Q) + if err != nil { + return nil, err + } + + if k.fipsApproved { + fips140.PCT("RSA sign and verify PCT", func() error { + hash := []byte{ + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, + 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, + 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, + } + sig, err := signPKCS1v15(k, "SHA-256", hash) + if err != nil { + return err + } + return verifyPKCS1v15(k.PublicKey(), "SHA-256", hash, sig) + }) + } + + return k, nil } } diff --git a/src/crypto/internal/fips140/rsa/rsa.go b/src/crypto/internal/fips140/rsa/rsa.go index 0bbf7010..76433894 100644 --- a/src/crypto/internal/fips140/rsa/rsa.go +++ b/src/crypto/internal/fips140/rsa/rsa.go @@ -310,26 +310,6 @@ func checkPrivateKey(priv *PrivateKey) error { return errors.New("crypto/rsa: d too small") } - // If the key is still in scope for FIPS mode, perform a Pairwise - // Consistency Test. - if priv.fipsApproved { - if err := fips140.PCT("RSA sign and verify PCT", func() error { - hash := []byte{ - 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, - 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, - 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, - 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, - } - sig, err := signPKCS1v15(priv, "SHA-256", hash) - if err != nil { - return err - } - return verifyPKCS1v15(priv.PublicKey(), "SHA-256", hash, sig) - }); err != nil { - return err - } - } - return nil } diff --git a/src/crypto/internal/fips140test/acvp_test.go b/src/crypto/internal/fips140test/acvp_test.go index 5871bde8..47a42cce 100644 --- a/src/crypto/internal/fips140test/acvp_test.go +++ b/src/crypto/internal/fips140test/acvp_test.go @@ -1624,7 +1624,7 @@ func cmdHmacDrbgAft(h func() hash.Hash) command { // * Uninstantiate // See Table 7 in draft-vassilev-acvp-drbg out := make([]byte, outLen) - drbg := ecdsa.TestingOnlyNewDRBG(func() fips140.Hash { return h() }, entropy, nonce, personalization) + drbg := ecdsa.TestingOnlyNewDRBG(h, entropy, nonce, personalization) drbg.Generate(out) drbg.Generate(out) diff --git a/src/crypto/internal/fips140test/cast_test.go b/src/crypto/internal/fips140test/cast_test.go index 1818b583..14ab72d1 100644 --- a/src/crypto/internal/fips140test/cast_test.go +++ b/src/crypto/internal/fips140test/cast_test.go @@ -5,9 +5,9 @@ package fipstest import ( + "crypto" + "crypto/internal/fips140" "crypto/rand" - "crypto/x509" - "encoding/pem" "fmt" "internal/testenv" "io/fs" @@ -50,8 +50,6 @@ var allCASTs = []string{ "KAS-ECC-SSC P-256", "ML-KEM PCT", "ML-KEM PCT", - "ML-KEM PCT", - "ML-KEM PCT", "ML-KEM-768", "PBKDF2", "RSA sign and verify PCT", @@ -107,60 +105,65 @@ func TestAllCASTs(t *testing.T) { // TestConditionals causes the conditional CASTs and PCTs to be invoked. func TestConditionals(t *testing.T) { mlkem.GenerateKey768() - k, err := ecdh.GenerateKey(ecdh.P256(), rand.Reader) + kDH, err := ecdh.GenerateKey(ecdh.P256(), rand.Reader) if err != nil { - t.Fatal(err) + t.Error(err) + } else { + ecdh.ECDH(ecdh.P256(), kDH, kDH.PublicKey()) } - ecdh.ECDH(ecdh.P256(), k, k.PublicKey()) kDSA, err := ecdsa.GenerateKey(ecdsa.P256(), rand.Reader) if err != nil { - t.Fatal(err) + t.Error(err) + } else { + ecdsa.SignDeterministic(ecdsa.P256(), sha256.New, kDSA, make([]byte, 32)) } - ecdsa.SignDeterministic(ecdsa.P256(), sha256.New, kDSA, make([]byte, 32)) k25519, err := ed25519.GenerateKey() if err != nil { - t.Fatal(err) + t.Error(err) + } else { + ed25519.Sign(k25519, make([]byte, 32)) } - ed25519.Sign(k25519, make([]byte, 32)) - rsa.VerifyPKCS1v15(&rsa.PublicKey{}, "", nil, nil) - // Parse an RSA key to hit the PCT rather than generating one (which is slow). - block, _ := pem.Decode([]byte(strings.ReplaceAll( - `-----BEGIN RSA TESTING KEY----- -MIIEowIBAAKCAQEAsPnoGUOnrpiSqt4XynxA+HRP7S+BSObI6qJ7fQAVSPtRkqso -tWxQYLEYzNEx5ZSHTGypibVsJylvCfuToDTfMul8b/CZjP2Ob0LdpYrNH6l5hvFE -89FU1nZQF15oVLOpUgA7wGiHuEVawrGfey92UE68mOyUVXGweJIVDdxqdMoPvNNU -l86BU02vlBiESxOuox+dWmuVV7vfYZ79Toh/LUK43YvJh+rhv4nKuF7iHjVjBd9s -B6iDjj70HFldzOQ9r8SRI+9NirupPTkF5AKNe6kUhKJ1luB7S27ZkvB3tSTT3P59 -3VVJvnzOjaA1z6Cz+4+eRvcysqhrRgFlwI9TEwIDAQABAoIBAEEYiyDP29vCzx/+ -dS3LqnI5BjUuJhXUnc6AWX/PCgVAO+8A+gZRgvct7PtZb0sM6P9ZcLrweomlGezI -FrL0/6xQaa8bBr/ve/a8155OgcjFo6fZEw3Dz7ra5fbSiPmu4/b/kvrg+Br1l77J -aun6uUAs1f5B9wW+vbR7tzbT/mxaUeDiBzKpe15GwcvbJtdIVMa2YErtRjc1/5B2 -BGVXyvlJv0SIlcIEMsHgnAFOp1ZgQ08aDzvilLq8XVMOahAhP1O2A3X8hKdXPyrx -IVWE9bS9ptTo+eF6eNl+d7htpKGEZHUxinoQpWEBTv+iOoHsVunkEJ3vjLP3lyI/ -fY0NQ1ECgYEA3RBXAjgvIys2gfU3keImF8e/TprLge1I2vbWmV2j6rZCg5r/AS0u -pii5CvJ5/T5vfJPNgPBy8B/yRDs+6PJO1GmnlhOkG9JAIPkv0RBZvR0PMBtbp6nT -Y3yo1lwamBVBfY6rc0sLTzosZh2aGoLzrHNMQFMGaauORzBFpY5lU50CgYEAzPHl -u5DI6Xgep1vr8QvCUuEesCOgJg8Yh1UqVoY/SmQh6MYAv1I9bLGwrb3WW/7kqIoD -fj0aQV5buVZI2loMomtU9KY5SFIsPV+JuUpy7/+VE01ZQM5FdY8wiYCQiVZYju9X -Wz5LxMNoz+gT7pwlLCsC4N+R8aoBk404aF1gum8CgYAJ7VTq7Zj4TFV7Soa/T1eE -k9y8a+kdoYk3BASpCHJ29M5R2KEA7YV9wrBklHTz8VzSTFTbKHEQ5W5csAhoL5Fo -qoHzFFi3Qx7MHESQb9qHyolHEMNx6QdsHUn7rlEnaTTyrXh3ifQtD6C0yTmFXUIS -CW9wKApOrnyKJ9nI0HcuZQKBgQCMtoV6e9VGX4AEfpuHvAAnMYQFgeBiYTkBKltQ -XwozhH63uMMomUmtSG87Sz1TmrXadjAhy8gsG6I0pWaN7QgBuFnzQ/HOkwTm+qKw -AsrZt4zeXNwsH7QXHEJCFnCmqw9QzEoZTrNtHJHpNboBuVnYcoueZEJrP8OnUG3r -UjmopwKBgAqB2KYYMUqAOvYcBnEfLDmyZv9BTVNHbR2lKkMYqv5LlvDaBxVfilE0 -2riO4p6BaAdvzXjKeRrGNEKoHNBpOSfYCOM16NjL8hIZB1CaV3WbT5oY+jp7Mzd5 -7d56RZOE+ERK2uz/7JX9VSsM/LbH9pJibd4e8mikDS9ntciqOH/3 ------END RSA TESTING KEY-----`, "TESTING KEY", "PRIVATE KEY"))) - if _, err := x509.ParsePKCS1PrivateKey(block.Bytes); err != nil { - t.Fatal(err) + kRSA, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Error(err) + } else { + rsa.SignPKCS1v15(kRSA, crypto.SHA256.String(), make([]byte, 32)) } t.Log("completed successfully") } +func TestCASTPasses(t *testing.T) { + moduleStatus(t) + testenv.MustHaveExec(t) + if err := fips140.Supported(); err != nil { + t.Skipf("test requires FIPS 140 mode: %v", err) + } + + cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestConditionals$", "-test.v") + cmd.Env = append(cmd.Env, "GODEBUG=fips140=debug") + out, err := cmd.CombinedOutput() + t.Logf("%s", out) + if err != nil || !strings.Contains(string(out), "completed successfully") { + t.Errorf("TestConditionals did not complete successfully") + } + + for _, name := range allCASTs { + t.Run(name, func(t *testing.T) { + if !strings.Contains(string(out), fmt.Sprintf("passed: %s\n", name)) { + t.Errorf("CAST/PCT %s success was not logged", name) + } else { + t.Logf("CAST/PCT succeeded: %s", name) + } + }) + } +} + func TestCASTFailures(t *testing.T) { moduleStatus(t) testenv.MustHaveExec(t) + if err := fips140.Supported(); err != nil { + t.Skipf("test requires FIPS 140 mode: %v", err) + } for _, name := range allCASTs { t.Run(name, func(t *testing.T) { @@ -169,7 +172,6 @@ func TestCASTFailures(t *testing.T) { if !testing.Verbose() { t.Parallel() } - t.Logf("CAST/PCT succeeded: %s", name) t.Logf("Testing CAST/PCT failure...") cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestConditionals$", "-test.v") cmd.Env = append(cmd.Env, fmt.Sprintf("GODEBUG=failfipscast=%s,fips140=on", name)) @@ -180,6 +182,8 @@ func TestCASTFailures(t *testing.T) { } if strings.Contains(string(out), "completed successfully") { t.Errorf("CAST/PCT %s failure did not stop the program", name) + } else if !strings.Contains(string(out), "self-test failed: "+name) { + t.Errorf("CAST/PCT %s failure did not log the expected message", name) } else { t.Logf("CAST/PCT %s failed as expected and caused the program to exit", name) } diff --git a/src/crypto/internal/fips140test/fips_test.go b/src/crypto/internal/fips140test/fips_test.go index 08d60933..52fc9d34 100644 --- a/src/crypto/internal/fips140test/fips_test.go +++ b/src/crypto/internal/fips140test/fips_test.go @@ -74,11 +74,9 @@ func TestVersion(t *testing.T) { continue } exp := setting.Value - if exp == "v1.0.0" { - // Unfortunately we enshrined the version of the first module as - // v1.0 before deciding to go for full versions. - exp = "v1.0" - } + // Remove the -hash suffix, if any. + // The version from fips140.Version omits it. + exp, _, _ = strings.Cut(exp, "-") if v := fips140.Version(); v != exp { t.Errorf("Version is %q, expected %q", v, exp) } diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index 1e0b5f06..088c66fa 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -357,7 +357,7 @@ func negotiateALPN(serverProtos, clientProtos []string, quic bool) (string, erro if http11fallback { return "", nil } - return "", fmt.Errorf("tls: client requested unsupported application protocols (%s)", clientProtos) + return "", fmt.Errorf("tls: client requested unsupported application protocols (%q)", clientProtos) } // supportsECDHE returns whether ECDHE key exchanges can be used with this diff --git a/src/crypto/x509/name_constraints_test.go b/src/crypto/x509/name_constraints_test.go index a5851845..831fcbc8 100644 --- a/src/crypto/x509/name_constraints_test.go +++ b/src/crypto/x509/name_constraints_test.go @@ -1456,63 +1456,7 @@ var nameConstraintsTests = []nameConstraintsTest{ expectedError: "incompatible key usage", }, - // An invalid DNS SAN should be detected only at validation time so - // that we can process CA certificates in the wild that have invalid SANs. - // See https://github.com/golang/go/issues/23995 - - // #77: an invalid DNS or mail SAN will not be detected if name constraint - // checking is not triggered. - { - roots: make([]constraintsSpec, 1), - intermediates: [][]constraintsSpec{ - { - {}, - }, - }, - leaf: leafSpec{ - sans: []string{"dns:this is invalid", "email:this @ is invalid"}, - }, - }, - - // #78: an invalid DNS SAN will be detected if any name constraint checking - // is triggered. - { - roots: []constraintsSpec{ - { - bad: []string{"uri:"}, - }, - }, - intermediates: [][]constraintsSpec{ - { - {}, - }, - }, - leaf: leafSpec{ - sans: []string{"dns:this is invalid"}, - }, - expectedError: "cannot parse dnsName", - }, - - // #79: an invalid email SAN will be detected if any name constraint - // checking is triggered. - { - roots: []constraintsSpec{ - { - bad: []string{"uri:"}, - }, - }, - intermediates: [][]constraintsSpec{ - { - {}, - }, - }, - leaf: leafSpec{ - sans: []string{"email:this @ is invalid"}, - }, - expectedError: "cannot parse rfc822Name", - }, - - // #80: if several EKUs are requested, satisfying any of them is sufficient. + // #77: if several EKUs are requested, satisfying any of them is sufficient. { roots: make([]constraintsSpec, 1), intermediates: [][]constraintsSpec{ @@ -1527,7 +1471,7 @@ var nameConstraintsTests = []nameConstraintsTest{ requestedEKUs: []ExtKeyUsage{ExtKeyUsageClientAuth, ExtKeyUsageEmailProtection}, }, - // #81: EKUs that are not asserted in VerifyOpts are not required to be + // #78: EKUs that are not asserted in VerifyOpts are not required to be // nested. { roots: make([]constraintsSpec, 1), @@ -1546,7 +1490,7 @@ var nameConstraintsTests = []nameConstraintsTest{ }, }, - // #82: a certificate without SANs and CN is accepted in a constrained chain. + // #79: a certificate without SANs and CN is accepted in a constrained chain. { roots: []constraintsSpec{ { @@ -1563,7 +1507,7 @@ var nameConstraintsTests = []nameConstraintsTest{ }, }, - // #83: a certificate without SANs and with a CN that does not parse as a + // #80: a certificate without SANs and with a CN that does not parse as a // hostname is accepted in a constrained chain. { roots: []constraintsSpec{ @@ -1582,7 +1526,7 @@ var nameConstraintsTests = []nameConstraintsTest{ }, }, - // #84: a certificate with SANs and CN is accepted in a constrained chain. + // #81: a certificate with SANs and CN is accepted in a constrained chain. { roots: []constraintsSpec{ { @@ -1600,14 +1544,7 @@ var nameConstraintsTests = []nameConstraintsTest{ }, }, - // #85: .example.com is an invalid DNS name, it should not match the - // constraint example.com. - { - roots: []constraintsSpec{{ok: []string{"dns:example.com"}}}, - leaf: leafSpec{sans: []string{"dns:.example.com"}}, - expectedError: "cannot parse dnsName \".example.com\"", - }, - // #86: URIs with IPv6 addresses with zones and ports are rejected + // #82: URIs with IPv6 addresses with zones and ports are rejected { roots: []constraintsSpec{ { diff --git a/src/crypto/x509/parser.go b/src/crypto/x509/parser.go index 4abcc1b7..9d6bfd6e 100644 --- a/src/crypto/x509/parser.go +++ b/src/crypto/x509/parser.go @@ -413,10 +413,14 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string if err := isIA5String(email); err != nil { return errors.New("x509: SAN rfc822Name is malformed") } + parsed, ok := parseRFC2821Mailbox(email) + if !ok || (ok && !domainNameValid(parsed.domain, false)) { + return errors.New("x509: SAN rfc822Name is malformed") + } emailAddresses = append(emailAddresses, email) case nameTypeDNS: name := string(data) - if err := isIA5String(name); err != nil { + if err := isIA5String(name); err != nil || (err == nil && !domainNameValid(name, false)) { return errors.New("x509: SAN dNSName is malformed") } dnsNames = append(dnsNames, string(name)) @@ -426,14 +430,9 @@ func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string return errors.New("x509: SAN uniformResourceIdentifier is malformed") } uri, err := url.Parse(uriStr) - if err != nil { + if err != nil || (err == nil && uri.Host != "" && !domainNameValid(uri.Host, false)) { return fmt.Errorf("x509: cannot parse URI %q: %s", uriStr, err) } - if len(uri.Host) > 0 { - if _, ok := domainToReverseLabels(uri.Host); !ok { - return fmt.Errorf("x509: cannot parse URI %q: invalid domain", uriStr) - } - } uris = append(uris, uri) case nameTypeIP: switch len(data) { @@ -598,15 +597,7 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandle return nil, nil, nil, nil, errors.New("x509: invalid constraint value: " + err.Error()) } - trimmedDomain := domain - if len(trimmedDomain) > 0 && trimmedDomain[0] == '.' { - // constraints can have a leading - // period to exclude the domain - // itself, but that's not valid in a - // normal domain name. - trimmedDomain = trimmedDomain[1:] - } - if _, ok := domainToReverseLabels(trimmedDomain); !ok { + if !domainNameValid(domain, true) { return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse dnsName constraint %q", domain) } dnsNames = append(dnsNames, domain) @@ -647,12 +638,7 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandle return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint) } } else { - // Otherwise it's a domain name. - domain := constraint - if len(domain) > 0 && domain[0] == '.' { - domain = domain[1:] - } - if _, ok := domainToReverseLabels(domain); !ok { + if !domainNameValid(constraint, true) { return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse rfc822Name constraint %q", constraint) } } @@ -668,15 +654,7 @@ func parseNameConstraintsExtension(out *Certificate, e pkix.Extension) (unhandle return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q: cannot be IP address", domain) } - trimmedDomain := domain - if len(trimmedDomain) > 0 && trimmedDomain[0] == '.' { - // constraints can have a leading - // period to exclude the domain itself, - // but that's not valid in a normal - // domain name. - trimmedDomain = trimmedDomain[1:] - } - if _, ok := domainToReverseLabels(trimmedDomain); !ok { + if !domainNameValid(domain, true) { return nil, nil, nil, nil, fmt.Errorf("x509: failed to parse URI constraint %q", domain) } uriDomains = append(uriDomains, domain) @@ -1317,3 +1295,40 @@ func ParseRevocationList(der []byte) (*RevocationList, error) { return rl, nil } + +// domainNameValid does minimal domain name validity checking. In particular it +// enforces the following properties: +// - names cannot have the trailing period +// - names can only have a leading period if constraint is true +// - names must be <= 253 characters +// - names cannot have empty labels +// - names cannot labels that are longer than 63 characters +// +// Note that this does not enforce the LDH requirements for domain names. +func domainNameValid(s string, constraint bool) bool { + if len(s) == 0 && constraint { + return true + } + if len(s) == 0 || (!constraint && s[0] == '.') || s[len(s)-1] == '.' || len(s) > 253 { + return false + } + lastDot := -1 + if constraint && s[0] == '.' { + s = s[1:] + } + + for i := 0; i <= len(s); i++ { + if i == len(s) || s[i] == '.' { + labelLen := i + if lastDot >= 0 { + labelLen -= lastDot + 1 + } + if labelLen == 0 || labelLen > 63 { + return false + } + lastDot = i + } + } + + return true +} diff --git a/src/crypto/x509/parser_test.go b/src/crypto/x509/parser_test.go index 3b9d9aed..1b553e36 100644 --- a/src/crypto/x509/parser_test.go +++ b/src/crypto/x509/parser_test.go @@ -8,6 +8,7 @@ import ( "encoding/asn1" "encoding/pem" "os" + "strings" "testing" cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" @@ -251,3 +252,45 @@ d5l1tRhScKu2NBgm74nYmJxJYgvuTA38wGhRrGU= } } } + +func TestDomainNameValid(t *testing.T) { + for _, tc := range []struct { + name string + dnsName string + constraint bool + valid bool + }{ + {"empty name, name", "", false, false}, + {"empty name, constraint", "", true, true}, + {"empty label, name", "a..a", false, false}, + {"empty label, constraint", "a..a", true, false}, + {"period, name", ".", false, false}, + {"period, constraint", ".", true, false}, // TODO(roland): not entirely clear if this is a valid constraint (require at least one label?) + {"valid, name", "a.b.c", false, true}, + {"valid, constraint", "a.b.c", true, true}, + {"leading period, name", ".a.b.c", false, false}, + {"leading period, constraint", ".a.b.c", true, true}, + {"trailing period, name", "a.", false, false}, + {"trailing period, constraint", "a.", true, false}, + {"bare label, name", "a", false, true}, + {"bare label, constraint", "a", true, true}, + {"254 char label, name", strings.Repeat("a.a", 84) + "aaa", false, false}, + {"254 char label, constraint", strings.Repeat("a.a", 84) + "aaa", true, false}, + {"253 char label, name", strings.Repeat("a.a", 84) + "aa", false, false}, + {"253 char label, constraint", strings.Repeat("a.a", 84) + "aa", true, false}, + {"64 char single label, name", strings.Repeat("a", 64), false, false}, + {"64 char single label, constraint", strings.Repeat("a", 64), true, false}, + {"63 char single label, name", strings.Repeat("a", 63), false, true}, + {"63 char single label, constraint", strings.Repeat("a", 63), true, true}, + {"64 char label, name", "a." + strings.Repeat("a", 64), false, false}, + {"64 char label, constraint", "a." + strings.Repeat("a", 64), true, false}, + {"63 char label, name", "a." + strings.Repeat("a", 63), false, true}, + {"63 char label, constraint", "a." + strings.Repeat("a", 63), true, true}, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.valid != domainNameValid(tc.dnsName, tc.constraint) { + t.Errorf("domainNameValid(%q, %t) = %v; want %v", tc.dnsName, tc.constraint, !tc.valid, tc.valid) + } + }) + } +} diff --git a/src/crypto/x509/verify.go b/src/crypto/x509/verify.go index 7cc0fb2e..058153fb 100644 --- a/src/crypto/x509/verify.go +++ b/src/crypto/x509/verify.go @@ -391,6 +391,7 @@ func parseRFC2821Mailbox(in string) (mailbox rfc2821Mailbox, ok bool) { // domainToReverseLabels converts a textual domain name like foo.example.com to // the list of labels in reverse order, e.g. ["com", "example", "foo"]. func domainToReverseLabels(domain string) (reverseLabels []string, ok bool) { + reverseLabels = make([]string, 0, strings.Count(domain, ".")+1) for len(domain) > 0 { if i := strings.LastIndexByte(domain, '.'); i == -1 { reverseLabels = append(reverseLabels, domain) @@ -927,7 +928,10 @@ func alreadyInChain(candidate *Certificate, chain []*Certificate) bool { if !bytes.Equal(candidate.RawSubject, cert.RawSubject) { continue } - if !candidate.PublicKey.(pubKeyEqual).Equal(cert.PublicKey) { + // We enforce the canonical encoding of SPKI (by only allowing the + // correct AI paremeter encodings in parseCertificate), so it's safe to + // directly compare the raw bytes. + if !bytes.Equal(candidate.RawSubjectPublicKeyInfo, cert.RawSubjectPublicKeyInfo) { continue } var certSAN *pkix.Extension diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go index 7991f499..5595f99e 100644 --- a/src/crypto/x509/verify_test.go +++ b/src/crypto/x509/verify_test.go @@ -6,6 +6,7 @@ package x509 import ( "crypto" + "crypto/dsa" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" @@ -3048,3 +3049,129 @@ func TestInvalidPolicyWithAnyKeyUsage(t *testing.T) { t.Fatalf("unexpected error, got %q, want %q", err, expectedErr) } } + +func TestCertificateChainSignedByECDSA(t *testing.T) { + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + root := &Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "X"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(365 * 24 * time.Hour), + IsCA: true, + KeyUsage: KeyUsageCertSign | KeyUsageCRLSign, + BasicConstraintsValid: true, + } + caDER, err := CreateCertificate(rand.Reader, root, root, &caKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + root, err = ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + + leafKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + leaf := &Certificate{ + SerialNumber: big.NewInt(42), + Subject: pkix.Name{CommonName: "leaf"}, + NotBefore: time.Now().Add(-10 * time.Minute), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: KeyUsageDigitalSignature, + ExtKeyUsage: []ExtKeyUsage{ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + } + leafDER, err := CreateCertificate(rand.Reader, leaf, root, &leafKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + leaf, err = ParseCertificate(leafDER) + if err != nil { + t.Fatal(err) + } + + inter, err := ParseCertificate(dsaSelfSignedCNX(t)) + if err != nil { + t.Fatal(err) + } + + inters := NewCertPool() + inters.AddCert(root) + inters.AddCert(inter) + + wantErr := "certificate signed by unknown authority" + _, err = leaf.Verify(VerifyOptions{Intermediates: inters, Roots: NewCertPool()}) + if !strings.Contains(err.Error(), wantErr) { + t.Errorf("got %v, want %q", err, wantErr) + } +} + +// dsaSelfSignedCNX produces DER-encoded +// certificate with the properties: +// +// Subject=Issuer=CN=X +// DSA SPKI +// Matching inner/outer signature OIDs +// Dummy ECDSA signature +func dsaSelfSignedCNX(t *testing.T) []byte { + t.Helper() + var params dsa.Parameters + if err := dsa.GenerateParameters(¶ms, rand.Reader, dsa.L1024N160); err != nil { + t.Fatal(err) + } + + var dsaPriv dsa.PrivateKey + dsaPriv.Parameters = params + if err := dsa.GenerateKey(&dsaPriv, rand.Reader); err != nil { + t.Fatal(err) + } + dsaPub := &dsaPriv.PublicKey + + type dsaParams struct{ P, Q, G *big.Int } + paramDER, err := asn1.Marshal(dsaParams{dsaPub.P, dsaPub.Q, dsaPub.G}) + if err != nil { + t.Fatal(err) + } + yDER, err := asn1.Marshal(dsaPub.Y) + if err != nil { + t.Fatal(err) + } + + spki := publicKeyInfo{ + Algorithm: pkix.AlgorithmIdentifier{ + Algorithm: oidPublicKeyDSA, + Parameters: asn1.RawValue{FullBytes: paramDER}, + }, + PublicKey: asn1.BitString{Bytes: yDER, BitLength: 8 * len(yDER)}, + } + + rdn := pkix.Name{CommonName: "X"}.ToRDNSequence() + b, err := asn1.Marshal(rdn) + if err != nil { + t.Fatal(err) + } + rawName := asn1.RawValue{FullBytes: b} + + algoIdent := pkix.AlgorithmIdentifier{Algorithm: oidSignatureDSAWithSHA256} + tbs := tbsCertificate{ + Version: 0, + SerialNumber: big.NewInt(1002), + SignatureAlgorithm: algoIdent, + Issuer: rawName, + Validity: validity{NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(24 * time.Hour)}, + Subject: rawName, + PublicKey: spki, + } + c := certificate{ + TBSCertificate: tbs, + SignatureAlgorithm: algoIdent, + SignatureValue: asn1.BitString{Bytes: []byte{0}, BitLength: 8}, + } + dsaDER, err := asn1.Marshal(c) + if err != nil { + t.Fatal(err) + } + return dsaDER +} diff --git a/src/debug/pe/symbol.go b/src/debug/pe/symbol.go index 6e8d9d16..80acebe9 100644 --- a/src/debug/pe/symbol.go +++ b/src/debug/pe/symbol.go @@ -98,7 +98,12 @@ func readCOFFSymbols(fh *FileHeader, r io.ReadSeeker) ([]COFFSymbol, error) { // isSymNameOffset checks symbol name if it is encoded as offset into string table. func isSymNameOffset(name [8]byte) (bool, uint32) { if name[0] == 0 && name[1] == 0 && name[2] == 0 && name[3] == 0 { - return true, binary.LittleEndian.Uint32(name[4:]) + offset := binary.LittleEndian.Uint32(name[4:]) + if offset == 0 { + // symbol has no name + return false, 0 + } + return true, offset } return false, 0 } diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go index 0b64f06d..f4be515b 100644 --- a/src/encoding/asn1/asn1.go +++ b/src/encoding/asn1/asn1.go @@ -22,6 +22,7 @@ package asn1 import ( "errors" "fmt" + "internal/saferio" "math" "math/big" "reflect" @@ -666,10 +667,17 @@ func parseSequenceOf(bytes []byte, sliceType reflect.Type, elemType reflect.Type offset += t.length numElements++ } - ret = reflect.MakeSlice(sliceType, numElements, numElements) + elemSize := uint64(elemType.Size()) + safeCap := saferio.SliceCapWithSize(elemSize, uint64(numElements)) + if safeCap < 0 { + err = SyntaxError{fmt.Sprintf("%s slice too big: %d elements of %d bytes", elemType.Kind(), numElements, elemSize)} + return + } + ret = reflect.MakeSlice(sliceType, 0, safeCap) params := fieldParameters{} offset := 0 for i := 0; i < numElements; i++ { + ret = reflect.Append(ret, reflect.Zero(elemType)) offset, err = parseField(ret.Index(i), bytes, offset, params) if err != nil { return diff --git a/src/encoding/asn1/asn1_test.go b/src/encoding/asn1/asn1_test.go index 0597740b..41cc0ba5 100644 --- a/src/encoding/asn1/asn1_test.go +++ b/src/encoding/asn1/asn1_test.go @@ -7,10 +7,12 @@ package asn1 import ( "bytes" "encoding/hex" + "errors" "fmt" "math" "math/big" "reflect" + "runtime" "strings" "testing" "time" @@ -1216,3 +1218,39 @@ func TestImplicitTypeRoundtrip(t *testing.T) { t.Fatalf("Unexpected diff after roundtripping struct\na: %#v\nb: %#v", a, b) } } + +func TestParsingMemoryConsumption(t *testing.T) { + // Craft a syntatically valid, but empty, ~10 MB DER bomb. A successful + // unmarshal of this bomb should yield ~280 MB. However, the parsing should + // fail due to the empty content; and, in such cases, we want to make sure + // that we do not unnecessarily allocate memories. + derBomb := make([]byte, 10_000_000) + for i := range derBomb { + derBomb[i] = 0x30 + } + derBomb = append([]byte{0x30, 0x83, 0x98, 0x96, 0x80}, derBomb...) + + var m runtime.MemStats + runtime.GC() + runtime.ReadMemStats(&m) + memBefore := m.TotalAlloc + + var out []struct { + Id []int + Critical bool `asn1:"optional"` + Value []byte + } + _, err := Unmarshal(derBomb, &out) + if !errors.As(err, &SyntaxError{}) { + t.Fatalf("Incorrect error result: want (%v), but got (%v) instead", &SyntaxError{}, err) + } + + runtime.ReadMemStats(&m) + memDiff := m.TotalAlloc - memBefore + + // Ensure that the memory allocated does not exceed 10<<21 (~20 MB) when + // the parsing fails. + if memDiff > 10<<21 { + t.Errorf("Too much memory allocated while parsing DER: %v MiB", memDiff/1024/1024) + } +} diff --git a/src/encoding/pem/pem.go b/src/encoding/pem/pem.go index dcc7416e..21887008 100644 --- a/src/encoding/pem/pem.go +++ b/src/encoding/pem/pem.go @@ -37,7 +37,7 @@ type Block struct { // line bytes. The remainder of the byte array (also not including the new line // bytes) is also returned and this will always be smaller than the original // argument. -func getLine(data []byte) (line, rest []byte) { +func getLine(data []byte) (line, rest []byte, consumed int) { i := bytes.IndexByte(data, '\n') var j int if i < 0 { @@ -49,7 +49,7 @@ func getLine(data []byte) (line, rest []byte) { i-- } } - return bytes.TrimRight(data[0:i], " \t"), data[j:] + return bytes.TrimRight(data[0:i], " \t"), data[j:], j } // removeSpacesAndTabs returns a copy of its input with all spaces and tabs @@ -90,20 +90,32 @@ func Decode(data []byte) (p *Block, rest []byte) { // pemStart begins with a newline. However, at the very beginning of // the byte array, we'll accept the start string without it. rest = data + for { - if bytes.HasPrefix(rest, pemStart[1:]) { - rest = rest[len(pemStart)-1:] - } else if _, after, ok := bytes.Cut(rest, pemStart); ok { - rest = after - } else { + // Find the first END line, and then find the last BEGIN line before + // the end line. This lets us skip any repeated BEGIN lines that don't + // have a matching END. + endIndex := bytes.Index(rest, pemEnd) + if endIndex < 0 { return nil, data } + endTrailerIndex := endIndex + len(pemEnd) + beginIndex := bytes.LastIndex(rest[:endIndex], pemStart[1:]) + if beginIndex < 0 || beginIndex > 0 && rest[beginIndex-1] != '\n' { + return nil, data + } + rest = rest[beginIndex+len(pemStart)-1:] + endIndex -= beginIndex + len(pemStart) - 1 + endTrailerIndex -= beginIndex + len(pemStart) - 1 var typeLine []byte - typeLine, rest = getLine(rest) + var consumed int + typeLine, rest, consumed = getLine(rest) if !bytes.HasSuffix(typeLine, pemEndOfLine) { continue } + endIndex -= consumed + endTrailerIndex -= consumed typeLine = typeLine[0 : len(typeLine)-len(pemEndOfLine)] p = &Block{ @@ -117,7 +129,7 @@ func Decode(data []byte) (p *Block, rest []byte) { if len(rest) == 0 { return nil, data } - line, next := getLine(rest) + line, next, consumed := getLine(rest) key, val, ok := bytes.Cut(line, colon) if !ok { @@ -129,21 +141,13 @@ func Decode(data []byte) (p *Block, rest []byte) { val = bytes.TrimSpace(val) p.Headers[string(key)] = string(val) rest = next + endIndex -= consumed + endTrailerIndex -= consumed } - var endIndex, endTrailerIndex int - - // If there were no headers, the END line might occur - // immediately, without a leading newline. - if len(p.Headers) == 0 && bytes.HasPrefix(rest, pemEnd[1:]) { - endIndex = 0 - endTrailerIndex = len(pemEnd) - 1 - } else { - endIndex = bytes.Index(rest, pemEnd) - endTrailerIndex = endIndex + len(pemEnd) - } - - if endIndex < 0 { + // If there were headers, there must be a newline between the headers + // and the END line, so endIndex should be >= 0. + if len(p.Headers) > 0 && endIndex < 0 { continue } @@ -163,21 +167,24 @@ func Decode(data []byte) (p *Block, rest []byte) { } // The line must end with only whitespace. - if s, _ := getLine(restOfEndLine); len(s) != 0 { + if s, _, _ := getLine(restOfEndLine); len(s) != 0 { continue } - base64Data := removeSpacesAndTabs(rest[:endIndex]) - p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data))) - n, err := base64.StdEncoding.Decode(p.Bytes, base64Data) - if err != nil { - continue + p.Bytes = []byte{} + if endIndex > 0 { + base64Data := removeSpacesAndTabs(rest[:endIndex]) + p.Bytes = make([]byte, base64.StdEncoding.DecodedLen(len(base64Data))) + n, err := base64.StdEncoding.Decode(p.Bytes, base64Data) + if err != nil { + continue + } + p.Bytes = p.Bytes[:n] } - p.Bytes = p.Bytes[:n] // the -1 is because we might have only matched pemEnd without the // leading newline if the PEM block was empty. - _, rest = getLine(rest[endIndex+len(pemEnd)-1:]) + _, rest, _ = getLine(rest[endIndex+len(pemEnd)-1:]) return p, rest } } diff --git a/src/encoding/pem/pem_test.go b/src/encoding/pem/pem_test.go index e252ffd8..2c9b3eab 100644 --- a/src/encoding/pem/pem_test.go +++ b/src/encoding/pem/pem_test.go @@ -34,7 +34,7 @@ var getLineTests = []GetLineTest{ func TestGetLine(t *testing.T) { for i, test := range getLineTests { - x, y := getLine([]byte(test.in)) + x, y, _ := getLine([]byte(test.in)) if string(x) != test.out1 || string(y) != test.out2 { t.Errorf("#%d got:%+v,%+v want:%s,%s", i, x, y, test.out1, test.out2) } @@ -46,6 +46,7 @@ func TestDecode(t *testing.T) { if !reflect.DeepEqual(result, certificate) { t.Errorf("#0 got:%#v want:%#v", result, certificate) } + result, remainder = Decode(remainder) if !reflect.DeepEqual(result, privateKey) { t.Errorf("#1 got:%#v want:%#v", result, privateKey) @@ -68,7 +69,7 @@ func TestDecode(t *testing.T) { } result, remainder = Decode(remainder) - if result == nil || result.Type != "HEADERS" || len(result.Headers) != 1 { + if result == nil || result.Type != "VALID HEADERS" || len(result.Headers) != 1 { t.Errorf("#5 expected single header block but got :%v", result) } @@ -381,15 +382,15 @@ ZWAaUoVtWIQ52aKS0p19G99hhb+IVANC4akkdHV4SP8i7MVNZhfUmg== # This shouldn't be recognised because of the missing newline after the headers. ------BEGIN HEADERS----- +-----BEGIN INVALID HEADERS----- Header: 1 ------END HEADERS----- +-----END INVALID HEADERS----- # This should be valid, however. ------BEGIN HEADERS----- +-----BEGIN VALID HEADERS----- Header: 1 ------END HEADERS-----`) +-----END VALID HEADERS-----`) var certificate = &Block{Type: "CERTIFICATE", Headers: map[string]string{}, diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 6d92542e..641d1a32 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -235,7 +235,6 @@ var depsRules = ` internal/types/errors, mime/quotedprintable, net/internal/socktest, - net/url, runtime/trace, text/scanner, text/tabwriter; @@ -298,6 +297,12 @@ var depsRules = ` FMT < text/template/parse; + internal/bytealg, internal/itoa, math/bits, slices, strconv, unique + < net/netip; + + FMT, net/netip + < net/url; + net/url, text/template/parse < text/template < internal/lazytemplate; @@ -412,9 +417,6 @@ var depsRules = ` < golang.org/x/net/dns/dnsmessage, golang.org/x/net/lif; - internal/bytealg, internal/itoa, math/bits, slices, strconv, unique - < net/netip; - os, net/netip < internal/routebsd; @@ -557,7 +559,7 @@ var depsRules = ` # CRYPTO-MATH is crypto that exposes math/big APIs - no cgo, net; fmt now ok. - CRYPTO, FMT, math/big + CRYPTO, FMT, math/big, internal/saferio < crypto/internal/boring/bbig < crypto/internal/fips140cache < crypto/rand diff --git a/src/internal/buildcfg/cfg.go b/src/internal/buildcfg/cfg.go index 5ae4c0c7..50b122fd 100644 --- a/src/internal/buildcfg/cfg.go +++ b/src/internal/buildcfg/cfg.go @@ -85,7 +85,7 @@ func gofips140() string { } // isFIPSVersion reports whether v is a valid FIPS version, -// of the form vX.Y.Z. +// of the form vX.Y.Z or vX.Y.Z-hash. func isFIPSVersion(v string) bool { if !strings.HasPrefix(v, "v") { return false @@ -99,7 +99,8 @@ func isFIPSVersion(v string) bool { return false } v, ok = skipNum(v[len("."):]) - return ok && v == "" + hasHash := strings.HasPrefix(v, "-") && len(v) == len("-")+8 + return ok && (v == "" || hasHash) } // skipNum skips the leading text matching [0-9]+ diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 2d008825..852305e8 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -42,6 +42,7 @@ var All = []Info{ {Name: "http2client", Package: "net/http"}, {Name: "http2debug", Package: "net/http", Opaque: true}, {Name: "http2server", Package: "net/http"}, + {Name: "httpcookiemaxnum", Package: "net/http", Changed: 24, Old: "0"}, {Name: "httplaxcontentlength", Package: "net/http", Changed: 22, Old: "1"}, {Name: "httpmuxgo121", Package: "net/http", Changed: 22, Old: "1"}, {Name: "httpservecontentkeepheaders", Package: "net/http", Changed: 23, Old: "1"}, diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go index 74188c05..dd833afd 100644 --- a/src/internal/poll/fd_windows.go +++ b/src/internal/poll/fd_windows.go @@ -636,12 +636,22 @@ func (fd *FD) Pread(b []byte, off int64) (int, error) { fd.l.Lock() defer fd.l.Unlock() - curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent) - if err != nil { - return 0, err + if fd.isBlocking { + curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent) + if err != nil { + return 0, err + } + defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart) + defer fd.setOffset(curoffset) + } else { + // Overlapped handles don't have the file pointer updated + // when performing I/O operations, so there is no need to + // call Seek to reset the file pointer. + // Also, some overlapped file handles don't support seeking. + // See https://go.dev/issues/74951. + curoffset := fd.offset + defer fd.setOffset(curoffset) } - defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart) - defer fd.setOffset(curoffset) o := &fd.rop o.InitBuf(b) fd.setOffset(off) @@ -852,12 +862,22 @@ func (fd *FD) Pwrite(buf []byte, off int64) (int, error) { fd.l.Lock() defer fd.l.Unlock() - curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent) - if err != nil { - return 0, err + if fd.isBlocking { + curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent) + if err != nil { + return 0, err + } + defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart) + defer fd.setOffset(curoffset) + } else { + // Overlapped handles don't have the file pointer updated + // when performing I/O operations, so there is no need to + // call Seek to reset the file pointer. + // Also, some overlapped file handles don't support seeking. + // See https://go.dev/issues/74951. + curoffset := fd.offset + defer fd.setOffset(curoffset) } - defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart) - defer fd.setOffset(curoffset) var ntotal int for { diff --git a/src/internal/synctest/synctest_test.go b/src/internal/synctest/synctest_test.go index 307eee62..73a0a1c4 100644 --- a/src/internal/synctest/synctest_test.go +++ b/src/internal/synctest/synctest_test.go @@ -779,6 +779,28 @@ func TestWaitGroupHeapAllocated(t *testing.T) { }) } +// Issue #75134: Many racing bubble associations. +func TestWaitGroupManyBubbles(t *testing.T) { + var wg sync.WaitGroup + for range 100 { + wg.Go(func() { + synctest.Run(func() { + cancelc := make(chan struct{}) + var wg2 sync.WaitGroup + for range 100 { + wg2.Go(func() { + <-cancelc + }) + } + synctest.Wait() + close(cancelc) + wg2.Wait() + }) + }) + } + wg.Wait() +} + func TestHappensBefore(t *testing.T) { // Use two parallel goroutines accessing different vars to ensure that // we correctly account for multiple goroutines in the bubble. diff --git a/src/net/http/cookie.go b/src/net/http/cookie.go index 408fe884..d3c5c168 100644 --- a/src/net/http/cookie.go +++ b/src/net/http/cookie.go @@ -7,6 +7,7 @@ package http import ( "errors" "fmt" + "internal/godebug" "log" "net" "net/http/internal/ascii" @@ -16,6 +17,8 @@ import ( "time" ) +var httpcookiemaxnum = godebug.New("httpcookiemaxnum") + // A Cookie represents an HTTP cookie as sent in the Set-Cookie header of an // HTTP response or the Cookie header of an HTTP request. // @@ -58,16 +61,37 @@ const ( ) var ( - errBlankCookie = errors.New("http: blank cookie") - errEqualNotFoundInCookie = errors.New("http: '=' not found in cookie") - errInvalidCookieName = errors.New("http: invalid cookie name") - errInvalidCookieValue = errors.New("http: invalid cookie value") + errBlankCookie = errors.New("http: blank cookie") + errEqualNotFoundInCookie = errors.New("http: '=' not found in cookie") + errInvalidCookieName = errors.New("http: invalid cookie name") + errInvalidCookieValue = errors.New("http: invalid cookie value") + errCookieNumLimitExceeded = errors.New("http: number of cookies exceeded limit") ) +const defaultCookieMaxNum = 3000 + +func cookieNumWithinMax(cookieNum int) bool { + withinDefaultMax := cookieNum <= defaultCookieMaxNum + if httpcookiemaxnum.Value() == "" { + return withinDefaultMax + } + if customMax, err := strconv.Atoi(httpcookiemaxnum.Value()); err == nil { + withinCustomMax := customMax == 0 || cookieNum <= customMax + if withinDefaultMax != withinCustomMax { + httpcookiemaxnum.IncNonDefault() + } + return withinCustomMax + } + return withinDefaultMax +} + // ParseCookie parses a Cookie header value and returns all the cookies // which were set in it. Since the same cookie name can appear multiple times // the returned Values can contain more than one value for a given key. func ParseCookie(line string) ([]*Cookie, error) { + if !cookieNumWithinMax(strings.Count(line, ";") + 1) { + return nil, errCookieNumLimitExceeded + } parts := strings.Split(textproto.TrimString(line), ";") if len(parts) == 1 && parts[0] == "" { return nil, errBlankCookie @@ -197,11 +221,21 @@ func ParseSetCookie(line string) (*Cookie, error) { // readSetCookies parses all "Set-Cookie" values from // the header h and returns the successfully parsed Cookies. +// +// If the amount of cookies exceeds CookieNumLimit, and httpcookielimitnum +// GODEBUG option is not explicitly turned off, this function will silently +// fail and return an empty slice. func readSetCookies(h Header) []*Cookie { cookieCount := len(h["Set-Cookie"]) if cookieCount == 0 { return []*Cookie{} } + // Cookie limit was unfortunately introduced at a later point in time. + // As such, we can only fail by returning an empty slice rather than + // explicit error. + if !cookieNumWithinMax(cookieCount) { + return []*Cookie{} + } cookies := make([]*Cookie, 0, cookieCount) for _, line := range h["Set-Cookie"] { if cookie, err := ParseSetCookie(line); err == nil { @@ -329,13 +363,28 @@ func (c *Cookie) Valid() error { // readCookies parses all "Cookie" values from the header h and // returns the successfully parsed Cookies. // -// if filter isn't empty, only cookies of that name are returned. +// If filter isn't empty, only cookies of that name are returned. +// +// If the amount of cookies exceeds CookieNumLimit, and httpcookielimitnum +// GODEBUG option is not explicitly turned off, this function will silently +// fail and return an empty slice. func readCookies(h Header, filter string) []*Cookie { lines := h["Cookie"] if len(lines) == 0 { return []*Cookie{} } + // Cookie limit was unfortunately introduced at a later point in time. + // As such, we can only fail by returning an empty slice rather than + // explicit error. + cookieCount := 0 + for _, line := range lines { + cookieCount += strings.Count(line, ";") + 1 + } + if !cookieNumWithinMax(cookieCount) { + return []*Cookie{} + } + cookies := make([]*Cookie, 0, len(lines)+strings.Count(lines[0], ";")) for _, line := range lines { line = textproto.TrimString(line) diff --git a/src/net/http/cookie_test.go b/src/net/http/cookie_test.go index aac69563..d028725f 100644 --- a/src/net/http/cookie_test.go +++ b/src/net/http/cookie_test.go @@ -11,6 +11,7 @@ import ( "log" "os" "reflect" + "slices" "strings" "testing" "time" @@ -255,16 +256,17 @@ func TestAddCookie(t *testing.T) { } var readSetCookiesTests = []struct { - Header Header - Cookies []*Cookie + header Header + cookies []*Cookie + godebug string }{ { - Header{"Set-Cookie": {"Cookie-1=v$1"}}, - []*Cookie{{Name: "Cookie-1", Value: "v$1", Raw: "Cookie-1=v$1"}}, + header: Header{"Set-Cookie": {"Cookie-1=v$1"}}, + cookies: []*Cookie{{Name: "Cookie-1", Value: "v$1", Raw: "Cookie-1=v$1"}}, }, { - Header{"Set-Cookie": {"NID=99=YsDT5i3E-CXax-; expires=Wed, 23-Nov-2011 01:05:03 GMT; path=/; domain=.google.ch; HttpOnly"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"NID=99=YsDT5i3E-CXax-; expires=Wed, 23-Nov-2011 01:05:03 GMT; path=/; domain=.google.ch; HttpOnly"}}, + cookies: []*Cookie{{ Name: "NID", Value: "99=YsDT5i3E-CXax-", Path: "/", @@ -276,8 +278,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {".ASPXAUTH=7E3AA; expires=Wed, 07-Mar-2012 14:25:06 GMT; path=/; HttpOnly"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {".ASPXAUTH=7E3AA; expires=Wed, 07-Mar-2012 14:25:06 GMT; path=/; HttpOnly"}}, + cookies: []*Cookie{{ Name: ".ASPXAUTH", Value: "7E3AA", Path: "/", @@ -288,8 +290,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {"ASP.NET_SessionId=foo; path=/; HttpOnly"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"ASP.NET_SessionId=foo; path=/; HttpOnly"}}, + cookies: []*Cookie{{ Name: "ASP.NET_SessionId", Value: "foo", Path: "/", @@ -298,8 +300,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {"samesitedefault=foo; SameSite"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"samesitedefault=foo; SameSite"}}, + cookies: []*Cookie{{ Name: "samesitedefault", Value: "foo", SameSite: SameSiteDefaultMode, @@ -307,8 +309,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {"samesiteinvalidisdefault=foo; SameSite=invalid"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"samesiteinvalidisdefault=foo; SameSite=invalid"}}, + cookies: []*Cookie{{ Name: "samesiteinvalidisdefault", Value: "foo", SameSite: SameSiteDefaultMode, @@ -316,8 +318,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {"samesitelax=foo; SameSite=Lax"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"samesitelax=foo; SameSite=Lax"}}, + cookies: []*Cookie{{ Name: "samesitelax", Value: "foo", SameSite: SameSiteLaxMode, @@ -325,8 +327,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {"samesitestrict=foo; SameSite=Strict"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"samesitestrict=foo; SameSite=Strict"}}, + cookies: []*Cookie{{ Name: "samesitestrict", Value: "foo", SameSite: SameSiteStrictMode, @@ -334,8 +336,8 @@ var readSetCookiesTests = []struct { }}, }, { - Header{"Set-Cookie": {"samesitenone=foo; SameSite=None"}}, - []*Cookie{{ + header: Header{"Set-Cookie": {"samesitenone=foo; SameSite=None"}}, + cookies: []*Cookie{{ Name: "samesitenone", Value: "foo", SameSite: SameSiteNoneMode, @@ -345,47 +347,66 @@ var readSetCookiesTests = []struct { // Make sure we can properly read back the Set-Cookie headers we create // for values containing spaces or commas: { - Header{"Set-Cookie": {`special-1=a z`}}, - []*Cookie{{Name: "special-1", Value: "a z", Raw: `special-1=a z`}}, + header: Header{"Set-Cookie": {`special-1=a z`}}, + cookies: []*Cookie{{Name: "special-1", Value: "a z", Raw: `special-1=a z`}}, }, { - Header{"Set-Cookie": {`special-2=" z"`}}, - []*Cookie{{Name: "special-2", Value: " z", Quoted: true, Raw: `special-2=" z"`}}, + header: Header{"Set-Cookie": {`special-2=" z"`}}, + cookies: []*Cookie{{Name: "special-2", Value: " z", Quoted: true, Raw: `special-2=" z"`}}, }, { - Header{"Set-Cookie": {`special-3="a "`}}, - []*Cookie{{Name: "special-3", Value: "a ", Quoted: true, Raw: `special-3="a "`}}, + header: Header{"Set-Cookie": {`special-3="a "`}}, + cookies: []*Cookie{{Name: "special-3", Value: "a ", Quoted: true, Raw: `special-3="a "`}}, }, { - Header{"Set-Cookie": {`special-4=" "`}}, - []*Cookie{{Name: "special-4", Value: " ", Quoted: true, Raw: `special-4=" "`}}, + header: Header{"Set-Cookie": {`special-4=" "`}}, + cookies: []*Cookie{{Name: "special-4", Value: " ", Quoted: true, Raw: `special-4=" "`}}, }, { - Header{"Set-Cookie": {`special-5=a,z`}}, - []*Cookie{{Name: "special-5", Value: "a,z", Raw: `special-5=a,z`}}, + header: Header{"Set-Cookie": {`special-5=a,z`}}, + cookies: []*Cookie{{Name: "special-5", Value: "a,z", Raw: `special-5=a,z`}}, }, { - Header{"Set-Cookie": {`special-6=",z"`}}, - []*Cookie{{Name: "special-6", Value: ",z", Quoted: true, Raw: `special-6=",z"`}}, + header: Header{"Set-Cookie": {`special-6=",z"`}}, + cookies: []*Cookie{{Name: "special-6", Value: ",z", Quoted: true, Raw: `special-6=",z"`}}, }, { - Header{"Set-Cookie": {`special-7=a,`}}, - []*Cookie{{Name: "special-7", Value: "a,", Raw: `special-7=a,`}}, + header: Header{"Set-Cookie": {`special-7=a,`}}, + cookies: []*Cookie{{Name: "special-7", Value: "a,", Raw: `special-7=a,`}}, }, { - Header{"Set-Cookie": {`special-8=","`}}, - []*Cookie{{Name: "special-8", Value: ",", Quoted: true, Raw: `special-8=","`}}, + header: Header{"Set-Cookie": {`special-8=","`}}, + cookies: []*Cookie{{Name: "special-8", Value: ",", Quoted: true, Raw: `special-8=","`}}, }, // Make sure we can properly read back the Set-Cookie headers // for names containing spaces: { - Header{"Set-Cookie": {`special-9 =","`}}, - []*Cookie{{Name: "special-9", Value: ",", Quoted: true, Raw: `special-9 =","`}}, + header: Header{"Set-Cookie": {`special-9 =","`}}, + cookies: []*Cookie{{Name: "special-9", Value: ",", Quoted: true, Raw: `special-9 =","`}}, }, // Quoted values (issue #46443) { - Header{"Set-Cookie": {`cookie="quoted"`}}, - []*Cookie{{Name: "cookie", Value: "quoted", Quoted: true, Raw: `cookie="quoted"`}}, + header: Header{"Set-Cookie": {`cookie="quoted"`}}, + cookies: []*Cookie{{Name: "cookie", Value: "quoted", Quoted: true, Raw: `cookie="quoted"`}}, + }, + { + header: Header{"Set-Cookie": slices.Repeat([]string{"a="}, defaultCookieMaxNum+1)}, + cookies: []*Cookie{}, + }, + { + header: Header{"Set-Cookie": slices.Repeat([]string{"a="}, 10)}, + cookies: []*Cookie{}, + godebug: "httpcookiemaxnum=5", + }, + { + header: Header{"Set-Cookie": strings.Split(strings.Repeat(";a=", defaultCookieMaxNum+1)[1:], ";")}, + cookies: slices.Repeat([]*Cookie{{Name: "a", Value: "", Quoted: false, Raw: "a="}}, defaultCookieMaxNum+1), + godebug: "httpcookiemaxnum=0", + }, + { + header: Header{"Set-Cookie": strings.Split(strings.Repeat(";a=", defaultCookieMaxNum+1)[1:], ";")}, + cookies: slices.Repeat([]*Cookie{{Name: "a", Value: "", Quoted: false, Raw: "a="}}, defaultCookieMaxNum+1), + godebug: fmt.Sprintf("httpcookiemaxnum=%v", defaultCookieMaxNum+1), }, // TODO(bradfitz): users have reported seeing this in the @@ -405,79 +426,103 @@ func toJSON(v any) string { func TestReadSetCookies(t *testing.T) { for i, tt := range readSetCookiesTests { + t.Setenv("GODEBUG", tt.godebug) for n := 0; n < 2; n++ { // to verify readSetCookies doesn't mutate its input - c := readSetCookies(tt.Header) - if !reflect.DeepEqual(c, tt.Cookies) { - t.Errorf("#%d readSetCookies: have\n%s\nwant\n%s\n", i, toJSON(c), toJSON(tt.Cookies)) + c := readSetCookies(tt.header) + if !reflect.DeepEqual(c, tt.cookies) { + t.Errorf("#%d readSetCookies: have\n%s\nwant\n%s\n", i, toJSON(c), toJSON(tt.cookies)) } } } } var readCookiesTests = []struct { - Header Header - Filter string - Cookies []*Cookie + header Header + filter string + cookies []*Cookie + godebug string }{ { - Header{"Cookie": {"Cookie-1=v$1", "c2=v2"}}, - "", - []*Cookie{ + header: Header{"Cookie": {"Cookie-1=v$1", "c2=v2"}}, + filter: "", + cookies: []*Cookie{ {Name: "Cookie-1", Value: "v$1"}, {Name: "c2", Value: "v2"}, }, }, { - Header{"Cookie": {"Cookie-1=v$1", "c2=v2"}}, - "c2", - []*Cookie{ + header: Header{"Cookie": {"Cookie-1=v$1", "c2=v2"}}, + filter: "c2", + cookies: []*Cookie{ {Name: "c2", Value: "v2"}, }, }, { - Header{"Cookie": {"Cookie-1=v$1; c2=v2"}}, - "", - []*Cookie{ + header: Header{"Cookie": {"Cookie-1=v$1; c2=v2"}}, + filter: "", + cookies: []*Cookie{ {Name: "Cookie-1", Value: "v$1"}, {Name: "c2", Value: "v2"}, }, }, { - Header{"Cookie": {"Cookie-1=v$1; c2=v2"}}, - "c2", - []*Cookie{ + header: Header{"Cookie": {"Cookie-1=v$1; c2=v2"}}, + filter: "c2", + cookies: []*Cookie{ {Name: "c2", Value: "v2"}, }, }, { - Header{"Cookie": {`Cookie-1="v$1"; c2="v2"`}}, - "", - []*Cookie{ + header: Header{"Cookie": {`Cookie-1="v$1"; c2="v2"`}}, + filter: "", + cookies: []*Cookie{ {Name: "Cookie-1", Value: "v$1", Quoted: true}, {Name: "c2", Value: "v2", Quoted: true}, }, }, { - Header{"Cookie": {`Cookie-1="v$1"; c2=v2;`}}, - "", - []*Cookie{ + header: Header{"Cookie": {`Cookie-1="v$1"; c2=v2;`}}, + filter: "", + cookies: []*Cookie{ {Name: "Cookie-1", Value: "v$1", Quoted: true}, {Name: "c2", Value: "v2"}, }, }, { - Header{"Cookie": {``}}, - "", - []*Cookie{}, + header: Header{"Cookie": {``}}, + filter: "", + cookies: []*Cookie{}, + }, + // GODEBUG=httpcookiemaxnum should work regardless if all cookies are sent + // via one "Cookie" field, or multiple fields. + { + header: Header{"Cookie": {strings.Repeat(";a=", defaultCookieMaxNum+1)[1:]}}, + cookies: []*Cookie{}, + }, + { + header: Header{"Cookie": slices.Repeat([]string{"a="}, 10)}, + cookies: []*Cookie{}, + godebug: "httpcookiemaxnum=5", + }, + { + header: Header{"Cookie": {strings.Repeat(";a=", defaultCookieMaxNum+1)[1:]}}, + cookies: slices.Repeat([]*Cookie{{Name: "a", Value: "", Quoted: false}}, defaultCookieMaxNum+1), + godebug: "httpcookiemaxnum=0", + }, + { + header: Header{"Cookie": slices.Repeat([]string{"a="}, defaultCookieMaxNum+1)}, + cookies: slices.Repeat([]*Cookie{{Name: "a", Value: "", Quoted: false}}, defaultCookieMaxNum+1), + godebug: fmt.Sprintf("httpcookiemaxnum=%v", defaultCookieMaxNum+1), }, } func TestReadCookies(t *testing.T) { for i, tt := range readCookiesTests { + t.Setenv("GODEBUG", tt.godebug) for n := 0; n < 2; n++ { // to verify readCookies doesn't mutate its input - c := readCookies(tt.Header, tt.Filter) - if !reflect.DeepEqual(c, tt.Cookies) { - t.Errorf("#%d readCookies:\nhave: %s\nwant: %s\n", i, toJSON(c), toJSON(tt.Cookies)) + c := readCookies(tt.header, tt.filter) + if !reflect.DeepEqual(c, tt.cookies) { + t.Errorf("#%d readCookies:\nhave: %s\nwant: %s\n", i, toJSON(c), toJSON(tt.cookies)) } } } @@ -689,6 +734,7 @@ func TestParseCookie(t *testing.T) { line string cookies []*Cookie err error + godebug string }{ { line: "Cookie-1=v$1", @@ -722,8 +768,28 @@ func TestParseCookie(t *testing.T) { line: "k1=\\", err: errInvalidCookieValue, }, + { + line: strings.Repeat(";a=", defaultCookieMaxNum+1)[1:], + err: errCookieNumLimitExceeded, + }, + { + line: strings.Repeat(";a=", 10)[1:], + err: errCookieNumLimitExceeded, + godebug: "httpcookiemaxnum=5", + }, + { + line: strings.Repeat(";a=", defaultCookieMaxNum+1)[1:], + cookies: slices.Repeat([]*Cookie{{Name: "a", Value: "", Quoted: false}}, defaultCookieMaxNum+1), + godebug: "httpcookiemaxnum=0", + }, + { + line: strings.Repeat(";a=", defaultCookieMaxNum+1)[1:], + cookies: slices.Repeat([]*Cookie{{Name: "a", Value: "", Quoted: false}}, defaultCookieMaxNum+1), + godebug: fmt.Sprintf("httpcookiemaxnum=%v", defaultCookieMaxNum+1), + }, } for i, tt := range tests { + t.Setenv("GODEBUG", tt.godebug) gotCookies, gotErr := ParseCookie(tt.line) if !errors.Is(gotErr, tt.err) { t.Errorf("#%d ParseCookie got error %v, want error %v", i, gotErr, tt.err) diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 07b3a9e1..2778db33 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1372,7 +1372,10 @@ func (w *wantConn) cancel(t *Transport) { w.done = true w.mu.Unlock() - if pc != nil { + // HTTP/2 connections (pc.alt != nil) aren't removed from the idle pool on use, + // and should not be added back here. If the pconn isn't in the idle pool, + // it's because we removed it due to an error. + if pc != nil && pc.alt == nil { t.putOrCloseIdleConn(pc) } } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 9762f058..28ad3eb6 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -7559,3 +7559,35 @@ func TestTransportServerProtocols(t *testing.T) { }) } } + +func TestIssue61474(t *testing.T) { + run(t, testIssue61474, []testMode{http2Mode}) +} +func testIssue61474(t *testing.T, mode testMode) { + if testing.Short() { + return + } + + // This test reliably exercises the condition causing #61474, + // but requires many iterations to do so. + // Keep the test around for now, but don't run it by default. + t.Skip("test is too large") + + cst := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) { + }), func(tr *Transport) { + tr.MaxConnsPerHost = 1 + }) + var wg sync.WaitGroup + defer wg.Wait() + for range 100000 { + wg.Go(func() { + ctx, cancel := context.WithTimeout(t.Context(), 1*time.Millisecond) + defer cancel() + req, _ := NewRequestWithContext(ctx, "GET", cst.ts.URL, nil) + resp, err := cst.c.Do(req) + if err == nil { + resp.Body.Close() + } + }) + } +} diff --git a/src/net/mail/message.go b/src/net/mail/message.go index 14f839a0..1502b359 100644 --- a/src/net/mail/message.go +++ b/src/net/mail/message.go @@ -724,7 +724,8 @@ func (p *addrParser) consumeDomainLiteral() (string, error) { } // Parse the dtext - var dtext string + dtext := p.s + dtextLen := 0 for { if p.empty() { return "", errors.New("mail: unclosed domain-literal") @@ -741,9 +742,10 @@ func (p *addrParser) consumeDomainLiteral() (string, error) { return "", fmt.Errorf("mail: bad character in domain-literal: %q", r) } - dtext += p.s[:size] + dtextLen += size p.s = p.s[size:] } + dtext = dtext[:dtextLen] // Skip the trailing ] if !p.consume(']') { diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go index d3753401..94e3e0b5 100644 --- a/src/net/textproto/reader.go +++ b/src/net/textproto/reader.go @@ -284,8 +284,10 @@ func (r *Reader) ReadCodeLine(expectCode int) (code int, message string, err err // // An expectCode <= 0 disables the check of the status code. func (r *Reader) ReadResponse(expectCode int) (code int, message string, err error) { - code, continued, message, err := r.readCodeLine(expectCode) + code, continued, first, err := r.readCodeLine(expectCode) multi := continued + var messageBuilder strings.Builder + messageBuilder.WriteString(first) for continued { line, err := r.ReadLine() if err != nil { @@ -296,12 +298,15 @@ func (r *Reader) ReadResponse(expectCode int) (code int, message string, err err var moreMessage string code2, continued, moreMessage, err = parseCodeLine(line, 0) if err != nil || code2 != code { - message += "\n" + strings.TrimRight(line, "\r\n") + messageBuilder.WriteByte('\n') + messageBuilder.WriteString(strings.TrimRight(line, "\r\n")) continued = true continue } - message += "\n" + moreMessage + messageBuilder.WriteByte('\n') + messageBuilder.WriteString(moreMessage) } + message = messageBuilder.String() if err != nil && multi && message != "" { // replace one line error message with all lines (full message) err = &Error{code, message} diff --git a/src/net/udpsock_test.go b/src/net/udpsock_test.go index 7ad8a585..3c8da43b 100644 --- a/src/net/udpsock_test.go +++ b/src/net/udpsock_test.go @@ -710,6 +710,11 @@ func TestIPv6WriteMsgUDPAddrPortTargetAddrIPVersion(t *testing.T) { // WriteMsgUDPAddrPort accepts IPv4 and IPv4-mapped IPv6 destination addresses, // and rejects IPv6 destination addresses on a "udp4" connection. func TestIPv4WriteMsgUDPAddrPortTargetAddrIPVersion(t *testing.T) { + switch runtime.GOOS { + case "plan9": + t.Skipf("not supported on %s", runtime.GOOS) + } + if !testableNetwork("udp4") { t.Skipf("skipping: udp4 not available") } diff --git a/src/net/url/url.go b/src/net/url/url.go index 2a576594..40faa7cb 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -16,6 +16,7 @@ import ( "errors" "fmt" "maps" + "net/netip" "path" "slices" "strconv" @@ -626,40 +627,61 @@ func parseAuthority(authority string) (user *Userinfo, host string, err error) { // parseHost parses host as an authority without user // information. That is, as host[:port]. func parseHost(host string) (string, error) { - if strings.HasPrefix(host, "[") { + if openBracketIdx := strings.LastIndex(host, "["); openBracketIdx != -1 { // Parse an IP-Literal in RFC 3986 and RFC 6874. // E.g., "[fe80::1]", "[fe80::1%25en0]", "[fe80::1]:80". - i := strings.LastIndex(host, "]") - if i < 0 { + closeBracketIdx := strings.LastIndex(host, "]") + if closeBracketIdx < 0 { return "", errors.New("missing ']' in host") } - colonPort := host[i+1:] + + colonPort := host[closeBracketIdx+1:] if !validOptionalPort(colonPort) { return "", fmt.Errorf("invalid port %q after host", colonPort) } + unescapedColonPort, err := unescape(colonPort, encodeHost) + if err != nil { + return "", err + } + hostname := host[openBracketIdx+1 : closeBracketIdx] + var unescapedHostname string // RFC 6874 defines that %25 (%-encoded percent) introduces // the zone identifier, and the zone identifier can use basically // any %-encoding it likes. That's different from the host, which // can only %-encode non-ASCII bytes. // We do impose some restrictions on the zone, to avoid stupidity // like newlines. - zone := strings.Index(host[:i], "%25") - if zone >= 0 { - host1, err := unescape(host[:zone], encodeHost) + zoneIdx := strings.Index(hostname, "%25") + if zoneIdx >= 0 { + hostPart, err := unescape(hostname[:zoneIdx], encodeHost) if err != nil { return "", err } - host2, err := unescape(host[zone:i], encodeZone) + zonePart, err := unescape(hostname[zoneIdx:], encodeZone) if err != nil { return "", err } - host3, err := unescape(host[i:], encodeHost) + unescapedHostname = hostPart + zonePart + } else { + var err error + unescapedHostname, err = unescape(hostname, encodeHost) if err != nil { return "", err } - return host1 + host2 + host3, nil } + + // Per RFC 3986, only a host identified by a valid + // IPv6 address can be enclosed by square brackets. + // This excludes any IPv4 or IPv4-mapped addresses. + addr, err := netip.ParseAddr(unescapedHostname) + if err != nil { + return "", fmt.Errorf("invalid host: %w", err) + } + if addr.Is4() || addr.Is4In6() { + return "", errors.New("invalid IPv6 host") + } + return "[" + unescapedHostname + "]" + unescapedColonPort, nil } else if i := strings.LastIndex(host, ":"); i != -1 { colonPort := host[i:] if !validOptionalPort(colonPort) { diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index 16e08b63..32065583 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -383,6 +383,16 @@ var urltests = []URLTest{ }, "", }, + // valid IPv6 host with port and path + { + "https://[2001:db8::1]:8443/test/path", + &URL{ + Scheme: "https", + Host: "[2001:db8::1]:8443", + Path: "/test/path", + }, + "", + }, // host subcomponent; IPv6 address with zone identifier in RFC 6874 { "http://[fe80::1%25en0]/", // alphanum zone identifier @@ -707,6 +717,24 @@ var parseRequestURLTests = []struct { // RFC 6874. {"http://[fe80::1%en0]/", false}, {"http://[fe80::1%en0]:8080/", false}, + + // Tests exercising RFC 3986 compliance + {"https://[1:2:3:4:5:6:7:8]", true}, // full IPv6 address + {"https://[2001:db8::a:b:c:d]", true}, // compressed IPv6 address + {"https://[fe80::1%25eth0]", true}, // link-local address with zone ID (interface name) + {"https://[fe80::abc:def%254]", true}, // link-local address with zone ID (interface index) + {"https://[2001:db8::1]/path", true}, // compressed IPv6 address with path + {"https://[fe80::1%25eth0]/path?query=1", true}, // link-local with zone, path, and query + + {"https://[::ffff:192.0.2.1]", false}, + {"https://[:1] ", false}, + {"https://[1:2:3:4:5:6:7:8:9]", false}, + {"https://[1::1::1]", false}, + {"https://[1:2:3:]", false}, + {"https://[ffff::127.0.0.4000]", false}, + {"https://[0:0::test.com]:80", false}, + {"https://[2001:db8::test.com]", false}, + {"https://[test.com]", false}, } func TestParseRequestURI(t *testing.T) { @@ -1643,6 +1671,17 @@ func TestParseErrors(t *testing.T) { {"cache_object:foo", true}, {"cache_object:foo/bar", true}, {"cache_object/:foo/bar", false}, + + {"http://[192.168.0.1]/", true}, // IPv4 in brackets + {"http://[192.168.0.1]:8080/", true}, // IPv4 in brackets with port + {"http://[::ffff:192.168.0.1]/", true}, // IPv4-mapped IPv6 in brackets + {"http://[::ffff:192.168.0.1]:8080/", true}, // IPv4-mapped IPv6 in brackets with port + {"http://[::ffff:c0a8:1]/", true}, // IPv4-mapped IPv6 in brackets (hex) + {"http://[not-an-ip]/", true}, // invalid IP string in brackets + {"http://[fe80::1%foo]/", true}, // invalid zone format in brackets + {"http://[fe80::1", true}, // missing closing bracket + {"http://fe80::1]/", true}, // missing opening bracket + {"http://[test.com]/", true}, // domain name in brackets } for _, tt := range tests { u, err := Parse(tt.in) diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go index d9af25d4..6f091c38 100644 --- a/src/os/os_windows_test.go +++ b/src/os/os_windows_test.go @@ -1883,6 +1883,34 @@ func TestFileOverlappedSeek(t *testing.T) { } } +func TestFileOverlappedReadAtVolume(t *testing.T) { + // Test that we can use File.ReadAt with an overlapped volume handle. + // See https://go.dev/issues/74951. + t.Parallel() + name := `\\.\` + filepath.VolumeName(t.TempDir()) + namep, err := syscall.UTF16PtrFromString(name) + if err != nil { + t.Fatal(err) + } + h, err := syscall.CreateFile(namep, + syscall.GENERIC_READ|syscall.GENERIC_WRITE, + syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_READ, + nil, syscall.OPEN_ALWAYS, syscall.FILE_FLAG_OVERLAPPED, 0) + if err != nil { + if errors.Is(err, syscall.ERROR_ACCESS_DENIED) { + t.Skip("skipping test: access denied") + } + t.Fatal(err) + } + f := os.NewFile(uintptr(h), name) + defer f.Close() + + var buf [0]byte + if _, err := f.ReadAt(buf[:], 0); err != nil { + t.Fatal(err) + } +} + func TestPipe(t *testing.T) { t.Parallel() r, w, err := os.Pipe() diff --git a/src/os/root_openat.go b/src/os/root_openat.go index cfc6d906..af953498 100644 --- a/src/os/root_openat.go +++ b/src/os/root_openat.go @@ -131,7 +131,9 @@ func rootMkdirAll(r *Root, fullname string, perm FileMode) error { if try > 0 || !IsNotExist(err) { return 0, &PathError{Op: "openat", Err: err} } - if err := mkdirat(parent, name, perm); err != nil { + // Try again on EEXIST, because the directory may have been created + // by another process or thread between the rootOpenDir and mkdirat calls. + if err := mkdirat(parent, name, perm); err != nil && err != syscall.EEXIST { return 0, &PathError{Op: "mkdirat", Err: err} } } diff --git a/src/os/root_test.go b/src/os/root_test.go index effcdeab..f9fbd115 100644 --- a/src/os/root_test.go +++ b/src/os/root_test.go @@ -1919,3 +1919,36 @@ func TestRootWriteReadFile(t *testing.T) { t.Fatalf("root.ReadFile(%q) = %q, %v; want %q, nil", name, got, err, want) } } + +func TestRootName(t *testing.T) { + dir := t.TempDir() + root, err := os.OpenRoot(dir) + if err != nil { + t.Fatal(err) + } + defer root.Close() + if got, want := root.Name(), dir; got != want { + t.Errorf("root.Name() = %q, want %q", got, want) + } + + f, err := root.Create("file") + if err != nil { + t.Fatal(err) + } + defer f.Close() + if got, want := f.Name(), filepath.Join(dir, "file"); got != want { + t.Errorf(`root.Create("file").Name() = %q, want %q`, got, want) + } + + if err := root.Mkdir("dir", 0o777); err != nil { + t.Fatal(err) + } + subroot, err := root.OpenRoot("dir") + if err != nil { + t.Fatal(err) + } + defer subroot.Close() + if got, want := subroot.Name(), filepath.Join(dir, "dir"); got != want { + t.Errorf(`root.OpenRoot("dir").Name() = %q, want %q`, got, want) + } +} diff --git a/src/os/root_unix.go b/src/os/root_unix.go index 4d6fc19a..c891e81b 100644 --- a/src/os/root_unix.go +++ b/src/os/root_unix.go @@ -75,7 +75,7 @@ func openRootInRoot(r *Root, name string) (*Root, error) { if err != nil { return nil, &PathError{Op: "openat", Path: name, Err: err} } - return newRoot(fd, name) + return newRoot(fd, joinPath(r.Name(), name)) } // rootOpenFileNolog is Root.OpenFile. diff --git a/src/os/root_windows.go b/src/os/root_windows.go index 523ee48d..2ec09cf2 100644 --- a/src/os/root_windows.go +++ b/src/os/root_windows.go @@ -123,7 +123,7 @@ func openRootInRoot(r *Root, name string) (*Root, error) { if err != nil { return nil, &PathError{Op: "openat", Path: name, Err: err} } - return newRoot(fd, name) + return newRoot(fd, joinPath(r.Name(), name)) } // rootOpenFileNolog is Root.OpenFile. diff --git a/src/runtime/decoratemappings_test.go b/src/runtime/decoratemappings_test.go new file mode 100644 index 00000000..7d1121c1 --- /dev/null +++ b/src/runtime/decoratemappings_test.go @@ -0,0 +1,72 @@ +// 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 runtime_test + +import ( + "os" + "regexp" + "runtime" + "testing" +) + +func validateMapLabels(t *testing.T, labels []string) { + // These are the specific region labels that need get added during the + // runtime phase. Hence they are the ones that need to be confirmed as + // present at the time the test reads its own region labels, which + // is sufficient to validate that the default `decoratemappings` value + // (enabled) was set early enough in the init process. + regions := map[string]bool{ + "allspans array": false, + "gc bits": false, + "heap": false, + "heap index": false, + "heap reservation": false, + "immortal metadata": false, + "page alloc": false, + "page alloc index": false, + "page summary": false, + "scavenge index": false, + } + for _, label := range labels { + if _, ok := regions[label]; !ok { + t.Logf("unexpected region label found: \"%s\"", label) + } + regions[label] = true + } + for label, found := range regions { + if !found { + t.Logf("region label missing: \"%s\"", label) + } + } +} + +func TestDecorateMappings(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("decoratemappings is only supported on Linux") + // /proc/self/maps is also Linux-specific + } + + var labels []string + if rawMaps, err := os.ReadFile("/proc/self/maps"); err != nil { + t.Fatalf("failed to read /proc/self/maps: %v", err) + } else { + t.Logf("maps:%s\n", string(rawMaps)) + matches := regexp.MustCompile("[^[]+ \\[anon: Go: (.+)\\]\n").FindAllSubmatch(rawMaps, -1) + for _, match_pair := range matches { + // match_pair consists of the matching substring and the parenthesized group + labels = append(labels, string(match_pair[1])) + } + } + t.Logf("DebugDecorateMappings: %v", *runtime.DebugDecorateMappings) + if *runtime.DebugDecorateMappings != 0 && runtime.SetVMANameSupported() { + validateMapLabels(t, labels) + } else { + if len(labels) > 0 { + t.Errorf("unexpected mapping labels present: %v", labels) + } else { + t.Skip("mapping labels absent as expected") + } + } +} diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index 9a4611e2..6559ecec 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -1927,3 +1927,7 @@ func (t *TraceStackTable) Reset() { func TraceStack(gp *G, tab *TraceStackTable) { traceStack(0, gp, (*traceStackTable)(tab)) } + +var DebugDecorateMappings = &debug.decoratemappings + +func SetVMANameSupported() bool { return setVMANameSupported() } diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index a1902bc6..251ee22f 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -282,6 +282,11 @@ Below is the full list of supported metrics, ordered lexicographically. The number of non-default behaviors executed by the net/http package due to a non-default GODEBUG=http2server=... setting. + /godebug/non-default-behavior/httpcookiemaxnum:events + The number of non-default behaviors executed by the net/http + package due to a non-default GODEBUG=httpcookiemaxnum=... + setting. + /godebug/non-default-behavior/httplaxcontentlength:events The number of non-default behaviors executed by the net/http package due to a non-default GODEBUG=httplaxcontentlength=... diff --git a/src/runtime/proc.go b/src/runtime/proc.go index b41bbe93..d898c966 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -789,7 +789,9 @@ func cpuinit(env string) { // getGodebugEarly extracts the environment variable GODEBUG from the environment on // Unix-like operating systems and returns it. This function exists to extract GODEBUG // early before much of the runtime is initialized. -func getGodebugEarly() string { +// +// Returns nil, false if OS doesn't provide env vars early in the init sequence. +func getGodebugEarly() (string, bool) { const prefix = "GODEBUG=" var env string switch GOOS { @@ -807,12 +809,16 @@ func getGodebugEarly() string { s := unsafe.String(p, findnull(p)) if stringslite.HasPrefix(s, prefix) { - env = gostring(p)[len(prefix):] + env = gostringnocopy(p)[len(prefix):] break } } + break + + default: + return "", false } - return env + return env, true } // The bootstrap sequence is: @@ -859,11 +865,14 @@ func schedinit() { // The world starts stopped. worldStopped() + godebug, parsedGodebug := getGodebugEarly() + if parsedGodebug { + parseRuntimeDebugVars(godebug) + } ticks.init() // run as early as possible moduledataverify() stackinit() mallocinit() - godebug := getGodebugEarly() cpuinit(godebug) // must run before alginit randinit() // must run before alginit, mcommoninit alginit() // maps, hash, rand must not be used before this call @@ -880,7 +889,12 @@ func schedinit() { goenvs() secure() checkfds() - parsedebugvars() + if !parsedGodebug { + // Some platforms, e.g., Windows, didn't make env vars available "early", + // so try again now. + parseRuntimeDebugVars(gogetenv("GODEBUG")) + } + finishDebugVarsSetup() gcinit() // Allocate stack space that can be used when crashing due to bad stack diff --git a/src/runtime/runtime1.go b/src/runtime/runtime1.go index 424745d2..15b54678 100644 --- a/src/runtime/runtime1.go +++ b/src/runtime/runtime1.go @@ -402,7 +402,7 @@ var dbgvars = []*dbgVar{ {name: "updatemaxprocs", value: &debug.updatemaxprocs, def: 1}, } -func parsedebugvars() { +func parseRuntimeDebugVars(godebug string) { // defaults debug.cgocheck = 1 debug.invalidptr = 1 @@ -420,12 +420,6 @@ func parsedebugvars() { } debug.traceadvanceperiod = defaultTraceAdvancePeriod - godebug := gogetenv("GODEBUG") - - p := new(string) - *p = godebug - godebugEnv.Store(p) - // apply runtime defaults, if any for _, v := range dbgvars { if v.def != 0 { @@ -437,7 +431,6 @@ func parsedebugvars() { } } } - // apply compile-time GODEBUG settings parsegodebug(godebugDefault, nil) @@ -463,6 +456,12 @@ func parsedebugvars() { if debug.gccheckmark > 0 { debug.asyncpreemptoff = 1 } +} + +func finishDebugVarsSetup() { + p := new(string) + *p = gogetenv("GODEBUG") + godebugEnv.Store(p) setTraceback(gogetenv("GOTRACEBACK")) traceback_env = traceback_cache diff --git a/src/runtime/set_vma_name_linux.go b/src/runtime/set_vma_name_linux.go index 100c2bfe..792d6ad3 100644 --- a/src/runtime/set_vma_name_linux.go +++ b/src/runtime/set_vma_name_linux.go @@ -14,9 +14,13 @@ import ( var prSetVMAUnsupported atomic.Bool +func setVMANameSupported() bool { + return !prSetVMAUnsupported.Load() +} + // setVMAName calls prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, start, len, name) func setVMAName(start unsafe.Pointer, length uintptr, name string) { - if debug.decoratemappings == 0 || prSetVMAUnsupported.Load() { + if debug.decoratemappings == 0 || !setVMANameSupported() { return } diff --git a/src/runtime/set_vma_name_stub.go b/src/runtime/set_vma_name_stub.go index 38f65fd5..6cb01ebf 100644 --- a/src/runtime/set_vma_name_stub.go +++ b/src/runtime/set_vma_name_stub.go @@ -10,3 +10,5 @@ import "unsafe" // setVMAName isn’t implemented func setVMAName(start unsafe.Pointer, len uintptr, name string) {} + +func setVMANameSupported() bool { return false } diff --git a/src/runtime/synctest.go b/src/runtime/synctest.go index 16af1209..529f69fd 100644 --- a/src/runtime/synctest.go +++ b/src/runtime/synctest.go @@ -410,7 +410,9 @@ func getOrSetBubbleSpecial(p unsafe.Pointer, bubbleid uint64, add bool) (assoc i } else if add { // p is not associated with a bubble, // and we've been asked to add an association. + lock(&mheap_.speciallock) s := (*specialBubble)(mheap_.specialBubbleAlloc.alloc()) + unlock(&mheap_.speciallock) s.bubbleid = bubbleid s.special.kind = _KindSpecialBubble s.special.offset = offset diff --git a/test/fixedbugs/issue75569.go b/test/fixedbugs/issue75569.go new file mode 100644 index 00000000..8420641d --- /dev/null +++ b/test/fixedbugs/issue75569.go @@ -0,0 +1,77 @@ +// run + +// 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 main + +func fff(a []int, b bool, p, q *int) { +outer: + n := a[0] + a = a[1:] + switch n { + case 1: + goto one + case 2: + goto two + case 3: + goto three + case 4: + goto four + } + +one: + goto inner +two: + goto outer +three: + goto inner +four: + goto innerSideEntry + +inner: + n = a[0] + a = a[1:] + switch n { + case 1: + goto outer + case 2: + goto inner + case 3: + goto innerSideEntry + default: + return + } +innerSideEntry: + n = a[0] + a = a[1:] + switch n { + case 1: + goto outer + case 2: + goto inner + case 3: + goto inner + } + ggg(p, q) + goto inner +} + +var b bool + +func ggg(p, q *int) { + n := *p + 5 // this +5 ends up in the entry block, well before the *p load + if b { + *q = 0 + } + *p = n +} + +func main() { + var x, y int + fff([]int{4, 4, 4}, false, &x, &y) + if x != 5 { + panic(x) + } +}