rolandshoemaker / stapled

OCSP stapling daemon

stapled - A OCSP stapling daemon

Note: This is still a work in progress, idk if I'd actually use it yet!

A caching OCSP daemon that makes stapling less painful. Inspired in large part by the notes written on the topic by Ryan Sleevi.

Intended to be easily proxyabe and distributable (and make life at least somewhat easier for applications implementing OCSP stapling in a less than ideal way).

stapled's Issues

Write documentation for Apache/nginx usage

Responder picker type should be configurable

i.e. random, round-robin, ...something else?

Max cache size should be configurable

Eating memory is bad.

Add stableCache interface

And make the on-disk cache satisfy it. The stapled struct should then hold a list of these and use them to write a response to a stable medium on updates.

This will allow adding a SQL backed interface later.

Push vs. poll stats (or both?)

I've done a little work on a quick counters/timers implementation in stats.go that could be used for a poll style stats interface (i.e. via a HTTP API). While that's nice for some stuff it might be better to have a push style stats system (i.e. StatsD) that allows offloading a lot of the statistical crap/stats state upstream.

Or hey, why not both...

Implement issuer caching

Instead of attempting to read a issuer from disk/AIA on each load implement a in-memory cache of issuers which can be checked before having to do disk/network IO. This cache should be seedable from a configured directory (and should probably implement a directory watcher like the certificate folder does...).

Each entry should store a pointer to a issuer in this cache which can be used for response/intermediate signature validation.

Write configuration documentation

A number of config behaviors may be non-obvious and could use some explanation (e.g. random proxy selection). Some kind of markdown configuration documentation would be good to have in tree so it doesn't drift out of sync.

This could also take the form of a better/fully annotated version of the current example.yml.

Option to accept & store responses for expired certs

There is an occasional use case where it makes sense to deliberately cache and serve an OCSP response for an expired certificate. It would be nice if I could provide a parameter to stapled (for an individual cert) that would ignore a stale response and cache it anyway.

Move refresh checking from per Entry to single goroutine

Instead of spawning a single goroutine per Entry via Entry.monitor we should split move this logic to the main stapled struct and just loop over each entry, check if it is ready to update, and update it in a single goroutine.

As @jsha points out this will make debugging things when there are a large number of entries much easier. This can be done at the same time as #11.

Make supported request hash functions configurable

Cleanup configuration format

There are a number of unused flags and some of the sections probably don't need to be their own sections.

Split up Entry

Currently Entry contains a bunch of state covering both the response and upstream request. Really these should be split out into two separate requests request/response structs which contain the related state, we can then easily refactor some of the functions to just take a request/response instead of a bunch of random args or move them off Entry itself.

Check for cert revocation

I noticed that stapled will accept and cache an OCSP response that indicates a certificate has been revoked. In my case, I actually want this to happen (similar to #38), but it might be nice to provide an option (possibly default?) to treat a response as bad if revoked.

Allow per serial responder overrides

Currently there is a bunch of code which allows the user to define a number of entries using only the serial + issuer + responders in the configuration file instead of just providing certificate files/a folder to watch.

I'm not 100% sure this feature is actually that useful, since in basically every case where you'd want to use the daemon you'd probably have a local copy of the certificate on hand.

See comment below.

Should response signature validation be optional

Currently I've done a lot to try to make sure each entry has it's issuer x509.Certificate so response/intermediate signatures can be validated. This is causing some pain in places, should it be optional?

I'm not a huge fan of ignoring the signatures since it could cause us to serve a invalid/incorrect response, but it might make sense to allow a explicit global ignore-signature-validation config flag for cases where this might be okay...

Configurable Alert Hooks

Copying from my defunct repo.

Rough idea: Fire "alerts" to configured binaries/scripts to e.g. post to Slack when a cert is added, or the cached ocsp response is updated, or expires.

Should be some kind of simple arg to a shell script interface. I'm thinking of libvirt hooks for inspiration.

One specific use-case I have is to support HAProxy integration by having a hook that fires when the OCSP response is refreshed. This hook can connect to the HAProxy stats socket and instruct the TLS terminator to reload the new OCSP response from disk.

You can see one approach to live reloading an OCSP response in HAProxy here: https://github.com/pierky/haproxy-ocsp-stapling-updater/blob/master/hapos-upd#L482:L517 Other TLS terminators will likely have their own versions of this feature in the future.

Build error "Request has no field or method Marshal"

I'm seeing the following when I try and build the project & run the tests:

$ go test ./...
# github.com/rolandshoemaker/stapled
./cache.go:319: ocspRequest.Marshal undefined (type *"golang.org/x/crypto/ocsp".Request has no field or method Marshal)
./server.go:29: r.Marshal undefined (type *"golang.org/x/crypto/ocsp".Request has no field or method Marshal)
FAIL    github.com/rolandshoemaker/stapled [build failed]

Is this dependent on a specific version of the upstream golang.org/x/crypto/ocsp package? I couldn't immediately find a commit that removed a Marshal method.

Add tooling for distro packaging

Support multiple proxies

Currently only one proxy can be defined, instead we should allow multiple and randomly pick one to use per request.

Implement YAML Unmarshal wrapper

Add a move current config struct to rawConfig and write a simple wrapper Unmarshal function to do all of the duration parsing etc.

Move http.Client from Entry to stapled

Each entry shouldn't have it's own http.Client for the sake of memory and connection caching, Instead a single client should exist on the main struct.

This could be somewhat complicated if we want to support per-entry proxy overrides/definitions, I would need to go back and look at how the client proxy stuff works in general.

Support NextPublish extension

We should parse NextPublish extensions from responses if present and use the value in the timeToUpdate algorithm instead of Thisupdate + ((ThisUpdate - NextUpdate) / 4).

Split sections into subpackages

A number of large sections should be split out into subpackages, i.e.

• OCSP fetching, verification, timing logic -> stapled/ocsp
• Disk cache should be abstraced to a stableCache interface and moved -> stapled/stableCache and stapled/stableCache/disk
• All of the config stuff -> stapled/config (implement marshaling wrapper)
• Directory watcher -> stapled/watcher

Write integration tests

Review feedback

GitHub doesn't seem to support line comments on already-committed code, so I'll just assembled some feedback here:

Why even have a dont-die-on-stale-response? Seems like that should always be true.

dont-cache also seems unnecessary. You could make the decision to cache or not based on whether disk.cache-folder is set. Similarly, dont-seed-cache-from-disk seems unnecessary and unused. Also, as a general naming thing, I'd use "no" instead of "dont" because it doesn't have the ambiguity introduce by eliding the apostrophe.

Some inconsistency about what's exported vs what's not. This is meant to be a command, so probably nothing should be exported. Alternately: Maybe you want to break it into subpackages for better maintenance, and then some things will need to be exported?

Would be good to add more comments, especially at the top of large functions. Easier to do this as you're writing than once it's all done.

As a stress test, it would be fun to seed this with all currently-issued Let's Encrypt certificates. I've been meaning to write an OCSP-aware monitoring daemon- this would effectively be that, if it could log warnings when responses are stale or don't parse or similar. You would probably need to define an alternate way to bring in certificates, and an alternate way to store responses, since 1M files in a directory is bad news. Even if you split among directories, you would have to make sure the host filesystem has enough inodes.

In Entry:

• maybe break out the "request related" and "response related" sections into their own structs?
• don't create an http.Client for each Entry, to save memory. The http client should probably be a property of the cache. They're safe for concurrent use.
• mu should be a sync.RWMutex instead of *sync.RWMutex, and doesn't need to be initialized in New (the zero value of mutexes is valid an unlocked).
• do you need to keep the x509.Certificate around for the lifetime? When the number of Entry's is large, the memory savings of discarding it could be large.

Entry.loadCertificate: There should be a cache for issuing certificates. Simple version: in-memory cache. Fancier version: config list of PEM files to load (or wildcard of PEM files to load) containing issuer certificates, pre-seeding in-memory cache.

Proxy URI: It should be possible to list multiple upstream proxies to be randomly selected, like the responders.

NewEntry / FromDef / Init: I'm trying to better understand the breakdown between these. It looks like NewEntry is everything that can't error, FromDef is everything that does file or network I/O, and Init is everything that doesn't do file or network I/O, but can error. Is that right? If so I might recommend merging NewEntry and Init. Also, comments. :-)

Entry.monitor: This seems like a mix of techniques: You have a constant-interval ticker, but you also pick an exact amount of time to sleep based on the response. Probably better to pick one. Recommendation: Define a single ticker on cache that loops through all entries and refreshes in a goroutine if it's time to do so. That way you only need a single long-running goroutine versus a goroutine per entry, which will make your stack traces easier to read.

HashNameAndPKI: Can't you use x509.MarshalPKIXPublicKey(key)? Also, why do you create an ocsp.Request manually rather than using ocsp.CreateRequest?

cc also @ccppuu

Offload proxy logic to another library

It looks like the amount of logic dealing with HTTP caching and proxying is relatively small, but it might be worthwhile conceptually to rely on an existing library for it: https://github.com/lox/httpcache.