Skip to content

Commit

Permalink
server: Use tlsconfig mutex when accessing certpool
Browse files Browse the repository at this point in the history
This was missed in open-policy-agent#6415

https://github.com/open-policy-agent/opa/actions/runs/7194437460/job/19595009978?pr=6476

We have a warning of this data race here:

```
==================
WARNING: DATA RACE
Write at 0x00c009b562e8 by goroutine 1815:
  github.com/open-policy-agent/opa/server.(*Server).reloadTLSConfig()
      /src/server/certs.go:65 +0x608
  github.com/open-policy-agent/opa/server.(*Server).getListener.(*Server).certLoopNotify.func2()
      /src/server/certs.go:174 +0x434
  github.com/open-policy-agent/opa/server.TestCertPoolReloading.func1()
      /src/server/server_test.go:5105 +0x4f
  github.com/open-policy-agent/opa/server.TestCertPoolReloading.func2()
      /src/server/server_test.go:5108 +0x41

Previous read at 0x00c009b562e8 by goroutine 1824:
  github.com/open-policy-agent/opa/server.(*Server).getListenerForHTTPSServer.func1()
      /src/server/server.go:649 +0x145
  crypto/tls.(*Conn).readClientHello()
      /usr/local/go/src/crypto/tls/handshake_server.go:149 +0x97d
  crypto/tls.(*Conn).serverHandshake()
      /usr/local/go/src/crypto/tls/handshake_server.go:42 +0x64
  crypto/tls.(*Conn).serverHandshake-fm()
      <autogenerated>:1 +0x47
  crypto/tls.(*Conn).handshakeContext()
      /usr/local/go/src/crypto/tls/conn.go:1552 +0x615
  crypto/tls.(*Conn).HandshakeContext()
      /usr/local/go/src/crypto/tls/conn.go:1492 +0x16b8
  net/http.(*conn).serve()
      /usr/local/go/src/net/http/server.go:1891 +0x16c0
  net/http.(*Server).Serve.func3()
      /usr/local/go/src/net/http/server.go:3086 +0x4f
```

Signed-off-by: Charlie Egan <charlie@styra.com>
  • Loading branch information
charlieegan3 authored and ashutosh-narkar committed Dec 13, 2023
1 parent 913ae2b commit 3f23d08
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,9 @@ func (s *Server) getListenerForHTTPSServer(u *url.URL, h http.Handler, t httpLis
// This is not required, but appears to be how connect time updates config should be done:
// https://github.com/golang/go/issues/16066#issuecomment-250606132
GetConfigForClient: func(info *tls.ClientHelloInfo) (*tls.Config, error) {
s.tlsConfigMtx.Lock()
defer s.tlsConfigMtx.Unlock()

cfg := &tls.Config{
GetCertificate: s.getCertificate,
ClientCAs: s.certPool,
Expand Down

0 comments on commit 3f23d08

Please sign in to comment.