open-vault/changelog/18899.txt
Alexander Scheel 2b9a8c6c49
Fix race in tidy status with cert counting (#18899)
* Read total cert counts with atomic.LoadUint32(...)

When generating the tidy status, we read the values of two backend
atomics, b.certCount and b.revokedCertCount, without using the atomic
load operation. This resulted in a data race when the status was read
at the same time as an on-going tidy operation:

    WARNING: DATA RACE
    Write at 0x00c00c77680c by goroutine 90522:
      sync/atomic.AddInt32()
          /usr/local/go/src/runtime/race_amd64.s:281 +0xb
      sync/atomic.AddUint32()
          <autogenerated>:1 +0x1a
      github.com/hashicorp/vault/builtin/logical/pki.(*backend).tidyStatusIncRevokedCertCount()
          /home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:1236 +0x107
      github.com/hashicorp/vault/builtin/logical/pki.(*backend).doTidyRevocationStore()
          /home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:525 +0x1404
      github.com/hashicorp/vault/builtin/logical/pki.(*backend).startTidyOperation.func1.1()
          /home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:290 +0x1a4
      github.com/hashicorp/vault/builtin/logical/pki.(*backend).startTidyOperation.func1()
          /home/circleci/go/src/github.com/hashicorp/vault/builtin/logical/pki/path_tidy.go:342 +0x278

    Previous read at 0x00c00c77680c by goroutine 90528:
      reflect.Value.Uint()
          /usr/local/go/src/reflect/value.go:2584 +0x195
      encoding/json.uintEncoder()
          /usr/local/go/src/encoding/json/encode.go:562 +0x45
      encoding/json.ptrEncoder.encode()
          /usr/local/go/src/encoding/json/encode.go:944 +0x3c2
      encoding/json.ptrEncoder.encode-fm()
          <autogenerated>:1 +0x90
      encoding/json.(*encodeState).reflectValue()
          /usr/local/go/src/encoding/json/encode.go:359 +0x88
      encoding/json.interfaceEncoder()
          /usr/local/go/src/encoding/json/encode.go:715 +0x17b
      encoding/json.mapEncoder.encode()
          /usr/local/go/src/encoding/json/encode.go:813 +0x854
      ... more stack trace pointing into JSON encoding and http
      handler...

In particular, because the tidy status was directly reading the uint
value without resorting to the atomic side, the JSON serialization could
race with a later atomic update.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Also use atomic load in tests

Because no tidy operation is running here, it should be safe to read the
pointed value directly, but use the safer atomic.Load for consistency.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
2023-01-30 14:13:40 -05:00

4 lines
102 B
Plaintext

```release-note:bug
secrets/pki: fix race between tidy's cert counting and tidy status reporting.
```