search

square / certstrap

Tools to bootstrap CAs, certificate requests, and signed certificates.
Apache License 2.0
2.31k stars 207 forks source link

pkix package functions are not safe for concurrent use #72

Closed sunjayBhatia closed 4 years ago

sunjayBhatia commented 5 years ago

We are using some of the functions in the pkix package to generate CAs, certs, and keys in tests. Running the tests with race detection and in parallel triggered failures as there are package level variables that are written to by different goroutines:

WARNING: DATA RACE
Write at 0x000000b8b598 by goroutine 7:
  github.com/square/certstrap/pkix.CreateCertificateAuthority()
      /tmp/build/80754af9/diego-release/src/github.com/square/certstrap/pkix/cert_auth.go:85 +0xb3
...

Previous write at 0x000000b8b598 by goroutine 10:
  github.com/square/certstrap/pkix.CreateCertificateAuthority()
      /tmp/build/80754af9/diego-release/src/github.com/square/certstrap/pkix/cert_auth.go:85 +0xb3
...

The pkix package seems to use a pattern that makes its functions unsafe for concurrent use, e.g. https://github.com/square/certstrap/blob/a0ea37d7c3834c63c247ab2083acf9d4657b19cd/pkix/cert_auth.go#L85

Is there an existing track of work to ensure the pkix package is safe for concurrency?

sunjayBhatia commented 5 years ago

cc @n4wei

csstaub commented 5 years ago

I don't believe that we have an existing track of work for this. The package was written for the CLI so thread-safety wasn't originally a concern. It would be nice to fix this though.

xoebus commented 4 years ago

I think this is now fixed with the removal of global variables in https://github.com/square/certstrap/commit/8cbba964811bfc0d1532896669d3430758d897f7?

mbyczkowski commented 4 years ago

Yes, the global state has been removed in #77 and the change was part of the 1.2.0 release.