Skip to content

revoke: race condition accessing CRLSet #1031

@mdempsky

Description

@mdempsky

The Go memory model allows concurrent reads of variables, but a write cannot be concurrent with any other operation whether read or write. This includes maps.

For example, see the Go 1.6 release notes [emphasis added]:

The runtime has added lightweight, best-effort detection of concurrent misuse of maps. As always, if one goroutine is writing to a map, no other goroutine should be reading or writing the map concurrently. If the runtime detects this condition, it prints a diagnosis and crashes the program. The best way to find out more about the problem is to run the program under the race detector, which will more reliably identify the race and give more detail.

Package revoke declares:

var CRLSet = map[string]*pkix.CertificateList{}
var crlLock = new(sync.Mutex)

Within certIsRevokedCRL, crlLock is used to protect delete(CRLSet, url) and CRLSet[url] = crl (i.e., write operations on CRLSet), but there's no protection on the CRLSet[url] read operation at the top of the function.

Moreover, since crlLock isn't exported, there's no way for users of package revoke to safely access CRLSet unless they know there aren't any concurrent certificate revocation checks happening (e.g., through higher-level application mutexes).

Two possible options (not exhaustive):

  1. If you can safely rely on Go 1.9 and can break your API, you can switch to using sync.Map.

  2. If backwards compatibility is important, you can at least add locking around the CRLSet[url] read operation; export crlLock (e.g., as CRLLock); and document to users that they need to protect access to CRLSet using CRLLock.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions