cloudflare / cloudflare/web-bot-auth

Caddy plugin example: Provision swallows directory-fetch errors (nil-validator panic) and only ever loads the first key

Open
#88 1 comment 0 reactions 1 assignee Claimed by @thibmeu View on GitHub
Dominant language
Rust
Stars
160
Forks
40
Avg merge
3d 3h
Merged PRs (30d)
11

Description

**Area:** `examples/caddy-plugin/` (`httpsig.go`, `handler.go`)

## Summary

The example Caddy plugin has three robustness issues that make it fail in ways that are surprising for anyone adapting it as a starting point. The most serious causes a nil-pointer panic on the first request whenever the signature directory is unreachable at startup. The plugin also silently ignores every key in the directory except the first, and never refreshes the directory, so key rotation requires a full Caddy reload.

I understand this is an *example* and not audited — filing because each of these is a quiet failure mode that’s easy to inherit when using this as a template, and the first one is a crash rather than a graceful error.

## 1. `Provision` swallows the fetch error, leaving `validator` nil → panic in `ServeHTTP`

`httpsig.go:60-62`:

```go
resp, err := http.Get("https://" + m.DirectoryBase + "/.well-known/http-message-signatures-directory")
if err != nil {
return nil // error discarded; m.validator is never set
}
```

If the directory host is unreachable at provision time, `Provision` returns `nil` (success) but leaves `m.validator == nil`. The first request then hits `httpsig.go:82`:

```go
if err := m.validator.Validate(r); err != nil { // nil pointer dereference
```

…which panics instead of returning an error. Expected behaviour: either fail provisioning loudly (`return err`) so Caddy reports a bad config, or set the module into a state that returns `503`/`401` for requests rather than dereferencing nil.

## 2. Only the first directory key is ever used

`httpsig.go:72`:

```go
validator, err := NewValidator(dir.Keys[0])
```

A directory may legitimately publish multiple keys (multiple agents, or a current + next key during rotation). Only `Keys[0]` is loaded, so any request signed with another listed key fails verification with no indication why. This also panics if `dir.Keys` is empty (no length check). Suggest iterating over all keys and registering each into the verifier’s keyring, keyed by JWK thumbprint.

## 3. Directory is fetched once at startup; no refresh

The fetch happens only in `Provision`, so the trusted key set is frozen at startup. Rotating or adding a key in the published directory has no effect until Caddy is reloaded/restarted. A periodic refresh (or refetch-on-unknown-keyid) would make rotation work without downtime.

## Minor

- `handler.go:50,56` hardcodes `Algo_ED25519`. The README already documents Ed25519-only as an example limitation, so this is expected — noting it only for completeness.
- `httpsig.go:59` comment typo: “the directory ios localhost”.

## Reproduction (issue #1)

1. Point `directory_base` at a host that is down or unresolvable.
1. `xcaddy build` the plugin and `./caddy run --config Caddyfile`. Provisioning succeeds.
1. Send any request to the guarded route → panic from nil `m.validator` rather than a clean error/`401`.

## Suggested direction

- `Provision`: propagate the fetch/parse error (or guard `ServeHTTP` against a nil validator and return an HTTP error).
- `NewValidator`: accept the full `[]json.RawMessage` and register every valid key; guard against an empty list.
- Optionally add a refresh interval for the directory.

Happy to open a PR for #1 and #2 if that’s welcome.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.