acme/autocert: fix renewal timer issue

Block when creating the renewal timer, rather than doing it in a
goroutine. This fixes an issue where startRenew and stopRenew are called
very closely together, and due to lock ordering, stopRenew may be called
before startRenew, resulting in the appearance that the renewal timer
has been stopped before it has actually been created.

This is only an issue in tests, as that is the only place stopRenew is
actually used. In particular this issue manifests in TestGetCertiifcate
sub-tests, where a httptest server reuses a port across two of the
sub-tests. In this case, the renewal calls end up creating dirty state
for the subsequent test, which can cause confusing behavior (such as
attempting to register an account twice.)

Another solution to this problem would be introducing a bool, protected
by renewalMu, which indicates if renewal has been halted, and to check
it in startRenew to check if stopRenew has already been called, which
would allow us to continue calling startRenew in a goroutine and relying
on renewalMu locking for ordering. That said I don't see a particularly
strong reason to call startRenew concurrently, so this seems like the
simplest solution for now.

Fixes golang/go#52494

Change-Id: I95420d3fd877572a0b9e408d2f8cd353f6a4e80e
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/433016
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>

Author: LewiGoddard

Diff for: acme/autocert/autocert.go

@@ -463,7 +463,7 @@ func (m *Manager) cert(ctx context.Context, ck certKey) (*tls.Certificate, error
 		leaf: cert.Leaf,
 	}
 	m.state[ck] = s
-	go m.startRenew(ck, s.key, s.leaf.NotAfter)
+	m.startRenew(ck, s.key, s.leaf.NotAfter)
 	return cert, nil
 }
 
@@ -609,7 +609,7 @@ func (m *Manager) createCert(ctx context.Context, ck certKey) (*tls.Certificate,
 	}
 	state.cert = der
 	state.leaf = leaf
-	go m.startRenew(ck, state.key, state.leaf.NotAfter)
+	m.startRenew(ck, state.key, state.leaf.NotAfter)
 	return state.tlscert()
 }
 

Diff for: acme/autocert/autocert_test.go

@@ -382,7 +382,7 @@ func TestGetCertificate(t *testing.T) {
 			},
 		},
 		{
-			name:   "expiredCache",
+			name:   "almostExpiredCache",
 			hello:  clientHelloInfo("example.org", algECDSA),
 			domain: "example.org",
 			prepare: func(t *testing.T, man *Manager, s *acmetest.CAServer) {