diff --git a/acme/acme_test.go b/acme/acme_test.go index a748d88ffd..3f6e2748f3 100644 --- a/acme/acme_test.go +++ b/acme/acme_test.go @@ -354,11 +354,12 @@ func TestWaitAuthorization(t *testing.T) { }) } t.Run("context cancel", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + ctx, cancel := context.WithCancel(context.Background()) defer cancel() _, err := runWaitAuthorization(ctx, t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Retry-After", "60") fmt.Fprintf(w, `{"status":"pending"}`) + time.AfterFunc(1*time.Millisecond, cancel) }) if err == nil { t.Error("err is nil") @@ -373,28 +374,14 @@ func runWaitAuthorization(ctx context.Context, t *testing.T, h http.HandlerFunc) h(w, r) })) defer ts.Close() - type res struct { - authz *Authorization - err error - } - ch := make(chan res, 1) - go func() { - client := &Client{ - Key: testKey, - DirectoryURL: ts.URL, - dir: &Directory{}, - KID: "some-key-id", // set to avoid lookup attempt - } - a, err := client.WaitAuthorization(ctx, ts.URL) - ch <- res{a, err} - }() - select { - case <-time.After(3 * time.Second): - t.Fatal("WaitAuthorization took too long to return") - case v := <-ch: - return v.authz, v.err + + client := &Client{ + Key: testKey, + DirectoryURL: ts.URL, + dir: &Directory{}, + KID: "some-key-id", // set to avoid lookup attempt } - panic("runWaitAuthorization: out of select") + return client.WaitAuthorization(ctx, ts.URL) } func TestRevokeAuthorization(t *testing.T) { diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go index 789a44147b..725677574b 100644 --- a/acme/autocert/autocert_test.go +++ b/acme/autocert/autocert_test.go @@ -427,18 +427,7 @@ func TestGetCertificate(t *testing.T) { man.Client = &acme.Client{DirectoryURL: s.URL()} - var tlscert *tls.Certificate - var err error - done := make(chan struct{}) - go func() { - tlscert, err = man.GetCertificate(tt.hello) - close(done) - }() - select { - case <-time.After(time.Minute): - t.Fatal("man.GetCertificate took too long to return") - case <-done: - } + tlscert, err := man.GetCertificate(tt.hello) if tt.expectError != "" { if err == nil { t.Fatal("expected error, got certificate") @@ -515,15 +504,12 @@ func TestGetCertificate_failedAttempt(t *testing.T) { if _, err := man.GetCertificate(hello); err == nil { t.Error("GetCertificate: err is nil") } - select { - case <-time.After(5 * time.Second): - t.Errorf("took too long to remove the %q state", exampleCertKey) - case <-done: - man.stateMu.Lock() - defer man.stateMu.Unlock() - if v, exist := man.state[exampleCertKey]; exist { - t.Errorf("state exists for %v: %+v", exampleCertKey, v) - } + + <-done + man.stateMu.Lock() + defer man.stateMu.Unlock() + if v, exist := man.state[exampleCertKey]; exist { + t.Errorf("state exists for %v: %+v", exampleCertKey, v) } } @@ -542,8 +528,9 @@ func TestRevokeFailedAuthz(t *testing.T) { t.Fatal("expected GetCertificate to fail") } - start := time.Now() - for time.Since(start) < 3*time.Second { + logTicker := time.NewTicker(3 * time.Second) + defer logTicker.Stop() + for { authz, err := m.Client.GetAuthorization(context.Background(), ca.URL()+"/authz/0") if err != nil { t.Fatal(err) @@ -551,10 +538,14 @@ func TestRevokeFailedAuthz(t *testing.T) { if authz.Status == acme.StatusDeactivated { return } + + select { + case <-logTicker.C: + t.Logf("still waiting on revocations") + default: + } time.Sleep(50 * time.Millisecond) } - t.Error("revocations took too long") - } func TestHTTPHandlerDefaultFallback(t *testing.T) { diff --git a/acme/rfc8555_test.go b/acme/rfc8555_test.go index e92f6583e6..d65720a356 100644 --- a/acme/rfc8555_test.go +++ b/acme/rfc8555_test.go @@ -177,8 +177,7 @@ func TestRFC_postKID(t *testing.T) { })) defer ts.Close() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{ Key: testKey, DirectoryURL: ts.URL, @@ -316,8 +315,7 @@ func TestRFC_Register(t *testing.T) { s.start() defer s.close() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{ Key: testKeyEC, DirectoryURL: s.url("/"), @@ -454,8 +452,7 @@ func TestRFC_RegisterExternalAccountBinding(t *testing.T) { s.start() defer s.close() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{ Key: testKeyEC, DirectoryURL: s.url("/"), @@ -867,24 +864,13 @@ func testWaitOrderStatus(t *testing.T, okStatus string) { s.start() defer s.close() - var order *Order - var err error - done := make(chan struct{}) - go func() { - cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} - order, err = cl.WaitOrder(context.Background(), s.url("/orders/1")) - close(done) - }() - select { - case <-time.After(3 * time.Second): - t.Fatal("WaitOrder took too long to return") - case <-done: - if err != nil { - t.Fatalf("WaitOrder: %v", err) - } - if order.Status != okStatus { - t.Errorf("order.Status = %q; want %q", order.Status, okStatus) - } + cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} + order, err := cl.WaitOrder(context.Background(), s.url("/orders/1")) + if err != nil { + t.Fatalf("WaitOrder: %v", err) + } + if order.Status != okStatus { + t.Errorf("order.Status = %q; want %q", order.Status, okStatus) } } @@ -909,30 +895,20 @@ func TestRFC_WaitOrderError(t *testing.T) { s.start() defer s.close() - var err error - done := make(chan struct{}) - go func() { - cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} - _, err = cl.WaitOrder(context.Background(), s.url("/orders/1")) - close(done) - }() - select { - case <-time.After(3 * time.Second): - t.Fatal("WaitOrder took too long to return") - case <-done: - if err == nil { - t.Fatal("WaitOrder returned nil error") - } - e, ok := err.(*OrderError) - if !ok { - t.Fatalf("err = %v (%T); want OrderError", err, err) - } - if e.OrderURL != s.url("/orders/1") { - t.Errorf("e.OrderURL = %q; want %q", e.OrderURL, s.url("/orders/1")) - } - if e.Status != StatusInvalid { - t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid) - } + cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} + _, err := cl.WaitOrder(context.Background(), s.url("/orders/1")) + if err == nil { + t.Fatal("WaitOrder returned nil error") + } + e, ok := err.(*OrderError) + if !ok { + t.Fatalf("err = %v (%T); want OrderError", err, err) + } + if e.OrderURL != s.url("/orders/1") { + t.Errorf("e.OrderURL = %q; want %q", e.OrderURL, s.url("/orders/1")) + } + if e.Status != StatusInvalid { + t.Errorf("e.Status = %q; want %q", e.Status, StatusInvalid) } } @@ -972,8 +948,7 @@ func TestRFC_CreateOrderCert(t *testing.T) { }) s.start() defer s.close() - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) - defer cancel() + ctx := context.Background() cl := &Client{Key: testKeyEC, DirectoryURL: s.url("/")} cert, curl, err := cl.CreateOrderCert(ctx, s.url("/pleaseissue"), csr, true) diff --git a/bcrypt/bcrypt.go b/bcrypt/bcrypt.go index addf56b435..5577c0f939 100644 --- a/bcrypt/bcrypt.go +++ b/bcrypt/bcrypt.go @@ -82,11 +82,20 @@ type hashed struct { minor byte } +// ErrPasswordTooLong is returned when the password passed to +// GenerateFromPassword is too long (i.e. > 72 bytes). +var ErrPasswordTooLong = errors.New("bcrypt: password length exceeds 72 bytes") + // GenerateFromPassword returns the bcrypt hash of the password at the given // cost. If the cost given is less than MinCost, the cost will be set to // DefaultCost, instead. Use CompareHashAndPassword, as defined in this package, // to compare the returned hashed password with its cleartext version. +// GenerateFromPassword does not accept passwords longer than 72 bytes, which +// is the longest password bcrypt will operate on. func GenerateFromPassword(password []byte, cost int) ([]byte, error) { + if len(password) > 72 { + return nil, ErrPasswordTooLong + } p, err := newFromPassword(password, cost) if err != nil { return nil, err diff --git a/bcrypt/bcrypt_test.go b/bcrypt/bcrypt_test.go index b7162d8217..8b589e3652 100644 --- a/bcrypt/bcrypt_test.go +++ b/bcrypt/bcrypt_test.go @@ -241,3 +241,10 @@ func TestNoSideEffectsFromCompare(t *testing.T) { t.Errorf("got=%q want=%q", got, want) } } + +func TestPasswordTooLong(t *testing.T) { + _, err := GenerateFromPassword(make([]byte, 73), 1) + if err != ErrPasswordTooLong { + t.Errorf("unexpected error: got %q, want %q", err, ErrPasswordTooLong) + } +} diff --git a/go.mod b/go.mod index 3ec54a3654..e6ceaa4555 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,9 @@ module golang.org/x/crypto go 1.17 require ( - golang.org/x/net v0.3.0 - golang.org/x/sys v0.3.0 - golang.org/x/term v0.3.0 + golang.org/x/net v0.5.0 + golang.org/x/sys v0.4.0 + golang.org/x/term v0.4.0 ) -require golang.org/x/text v0.5.0 // indirect +require golang.org/x/text v0.6.0 // indirect diff --git a/go.sum b/go.sum index f068dd32d5..95a9be68b0 100644 --- a/go.sum +++ b/go.sum @@ -5,8 +5,8 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.3.0 h1:VWL6FNY2bEEmsGVKabSlHu5Irp34xmMRoqb/9lF9lxk= -golang.org/x/net v0.3.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE= +golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw= +golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -14,17 +14,17 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18= +golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.3.0 h1:qoo4akIqOcDME5bhc/NgxUdovd6BSS2uMsVjB56q1xI= -golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA= +golang.org/x/term v0.4.0 h1:O7UWfv5+A2qiuulQk30kVinPoMtoIPeVaKLEgLpVkvg= +golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.5.0 h1:OLmvp0KP+FVG99Ct/qFiL/Fhk4zp4QQnZ7b2U+5piUM= -golang.org/x/text v0.5.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.6.0 h1:3XmdazWV+ubf7QgHSTWeykHOci5oeekaGJBLkrkaw4k= +golang.org/x/text v0.6.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/internal/wycheproof/ecdsa_test.go b/internal/wycheproof/ecdsa_test.go index 42f3285fc4..80125ada75 100644 --- a/internal/wycheproof/ecdsa_test.go +++ b/internal/wycheproof/ecdsa_test.go @@ -6,7 +6,11 @@ package wycheproof import ( "crypto/ecdsa" + "math/big" "testing" + + "golang.org/x/crypto/cryptobyte" + "golang.org/x/crypto/cryptobyte/asn1" ) func TestECDSA(t *testing.T) { @@ -76,9 +80,25 @@ func TestECDSA(t *testing.T) { h.Reset() h.Write(decodeHex(sig.Msg)) hashed := h.Sum(nil) - got := ecdsa.VerifyASN1(pub, hashed, decodeHex(sig.Sig)) + sigBytes := decodeHex(sig.Sig) + got := ecdsa.VerifyASN1(pub, hashed, sigBytes) + if want := shouldPass(sig.Result, sig.Flags, flagsShouldPass); got != want { + t.Errorf("tcid: %d, type: %s, comment: %q, VerifyASN1 wanted success: %t", sig.TcID, sig.Result, sig.Comment, want) + } + + var r, s big.Int + var inner cryptobyte.String + input := cryptobyte.String(sigBytes) + if !input.ReadASN1(&inner, asn1.SEQUENCE) || + !input.Empty() || + !inner.ReadASN1Integer(&r) || + !inner.ReadASN1Integer(&s) || + !inner.Empty() { + continue + } + got = ecdsa.Verify(pub, hashed, &r, &s) if want := shouldPass(sig.Result, sig.Flags, flagsShouldPass); got != want { - t.Errorf("tcid: %d, type: %s, comment: %q, wanted success: %t", sig.TcID, sig.Result, sig.Comment, want) + t.Errorf("tcid: %d, type: %s, comment: %q, Verify wanted success: %t", sig.TcID, sig.Result, sig.Comment, want) } } } diff --git a/ssh/handshake.go b/ssh/handshake.go index 2b84c35716..07a1843e0a 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -58,11 +58,13 @@ type handshakeTransport struct { incoming chan []byte readError error - mu sync.Mutex - writeError error - sentInitPacket []byte - sentInitMsg *kexInitMsg - pendingPackets [][]byte // Used when a key exchange is in progress. + mu sync.Mutex + writeError error + sentInitPacket []byte + sentInitMsg *kexInitMsg + pendingPackets [][]byte // Used when a key exchange is in progress. + writePacketsLeft uint32 + writeBytesLeft int64 // If the read loop wants to schedule a kex, it pings this // channel, and the write loop will send out a kex @@ -71,7 +73,8 @@ type handshakeTransport struct { // If the other side requests or confirms a kex, its kexInit // packet is sent here for the write loop to find it. - startKex chan *pendingKex + startKex chan *pendingKex + kexLoopDone chan struct{} // closed (with writeError non-nil) when kexLoop exits // data for host key checking hostKeyCallback HostKeyCallback @@ -86,12 +89,10 @@ type handshakeTransport struct { // Algorithms agreed in the last key exchange. algorithms *algorithms + // Counters exclusively owned by readLoop. readPacketsLeft uint32 readBytesLeft int64 - writePacketsLeft uint32 - writeBytesLeft int64 - // The session ID or nil if first kex did not complete yet. sessionID []byte } @@ -108,7 +109,8 @@ func newHandshakeTransport(conn keyingTransport, config *Config, clientVersion, clientVersion: clientVersion, incoming: make(chan []byte, chanSize), requestKex: make(chan struct{}, 1), - startKex: make(chan *pendingKex, 1), + startKex: make(chan *pendingKex), + kexLoopDone: make(chan struct{}), config: config, } @@ -340,16 +342,17 @@ write: t.mu.Unlock() } + // Unblock reader. + t.conn.Close() + // drain startKex channel. We don't service t.requestKex // because nobody does blocking sends there. - go func() { - for init := range t.startKex { - init.done <- t.writeError - } - }() + for request := range t.startKex { + request.done <- t.getWriteError() + } - // Unblock reader. - t.conn.Close() + // Mark that the loop is done so that Close can return. + close(t.kexLoopDone) } // The protocol uses uint32 for packet counters, so we can't let them @@ -545,7 +548,16 @@ func (t *handshakeTransport) writePacket(p []byte) error { } func (t *handshakeTransport) Close() error { - return t.conn.Close() + // Close the connection. This should cause the readLoop goroutine to wake up + // and close t.startKex, which will shut down kexLoop if running. + err := t.conn.Close() + + // Wait for the kexLoop goroutine to complete. + // At that point we know that the readLoop goroutine is complete too, + // because kexLoop itself waits for readLoop to close the startKex channel. + <-t.kexLoopDone + + return err } func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error {