From 44a85d6f859cd0139ac70d7c748b8e622c716a2b Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sun, 3 May 2026 13:21:56 +0000 Subject: [PATCH] acme-server: account resource + JWS verifier (Phase 1b/7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layers JWS-authenticated POST machinery onto the Phase 1a foundation (commit ec88a61). After this commit, an ACME client can run POST /acme/profile//new-account against certctl and successfully register an account. Account update + deactivation via POST /acme/profile//account/ work. Orders + challenges remain Phase 2 / 3. Background: Two prior dispatch attempts at the original Phase 1 ("skeleton + directory + new-nonce + new-account" as a single commit) failed on go-jose v4 API speculation (jws.GetPayload, sig.Algorithm, jose.SHA256, etc. — none of those exist in v4). Splitting Phase 1 into 1a (foundation, no go-jose) and 1b (this commit, all go-jose in one place) concentrated the JWS work where attention pays off. The verifier reads the actual go-jose v4 surface — ParseSigned with closed alg allow-list, Header struct fields (Algorithm, KeyID, JSONWebKey, Nonce, ExtraHeaders[HeaderKey]), JWK.Thumbprint with stdlib crypto.SHA256. What ships: - internal/api/acme/jws.go: 487-line verifier + sentinel error family. Enforces RFC 8555 §6.2 + §6.4 + §6.5 invariants: - alg in {RS256, ES256, EdDSA} (closed allow-list passed to jose.ParseSigned — HS256 / none / etc. rejected at parse time) - exactly one of `kid` / `jwk` in protected header (per endpoint policy — new-account demands jwk, others demand kid) - protected `url` matches request URL exactly - protected `nonce` consumed against acme_nonces (badNonce on miss/replay/expiry per RFC 8555 §6.5.1) - kid round-trips against canonical AccountKID(accountID) URL (catches cross-profile / cross-host replay) - kid path: account exists + status=valid (deactivated / revoked accounts cannot authenticate) - signature verifies; post-Verify payload bytes equal UnsafePayloadWithoutVerification (defense in depth) + JWK persistence helpers (JWKToPEM / ParseJWKFromPEM round- trip a public-only JWK as a PEM-wrapped JSON envelope; stored as TEXT in acme_accounts.jwk_pem for diff-friendliness) + JWKThumbprint per RFC 7638. - internal/api/acme/jws_test.go: 16 cases covering happy paths (RS256 kid, ES256 jwk, EdDSA kid) + every named failure mode (alg-not-allowed, bad-sig, missing-nonce, unknown-nonce, replay, url-mismatch, mixed kid+jwk, deactivated-account, cross-host kid). Uses real keypairs + real go-jose Signer to build JWS objects. - internal/api/acme/account.go: NewAccountRequest / AccountUpdateRequest payload shapes (RFC 8555 §7.3 + §7.3.2 + §7.3.6) + AccountResponseJSON wire shape + MarshalAccount helper. - internal/domain/acme.go: ACMEAccount struct + ACMEAccountStatus closed enum (valid / deactivated / revoked). - internal/repository/postgres/acme.go: full account CRUD path (CreateAccountWithTx with 23505-unique-violation sentinel translation, GetAccountByID, GetAccountByThumbprint, UpdateAccountContactWithTx, UpdateAccountStatusWithTx) + sql.ErrNoRows-wrapped repository.ErrNotFound on lookup misses. - internal/service/acme.go: ACMERepo interface extended; SetTransactor + SetAuditService wires; NewAccount (idempotent re-registration per RFC 8555 §7.3.1 — same JWK returns existing row without an update or new audit event); LookupAccount; UpdateAccount; DeactivateAccount; VerifyJWS adapter that bridges api/acme.VerifierConfig to the service-layer ACMERepo; per-op metrics extended (new_account_total + _failures_total + _idempotent_total + update_account_total + _failures_total + deactivate_account_total). - internal/service/acme_test.go: 8 new tests covering new-account happy path / idempotent re-registration / only- return-existing match + no-match / contact update / deactivate / lookup-not-found / requires-transactor. - internal/api/handler/acme.go: NewAccount + Account handlers. Account dispatches POST-as-GET (RFC 8555 §6.3 — empty body or {} payload returns the account row), contact update, and deactivation from the same endpoint. Defense-in-depth check that the kid path-segment matches the URL path-segment (the verifier already round-tripped the kid against canonical URL, but the handler re-asserts to catch any future verifier refactor). - internal/api/handler/acme_handler_test.go: 7 new cases covering happy-create, idempotent-200, only-return-existing- no-match-400, malformed-JWS-400, kid-URL-mismatch-401, deactivate, contact-update, POST-as-GET. - internal/api/router/router.go: 4 new Register calls (per- profile + shorthand for new-account and account/{acc_id}). - internal/api/router/openapi_parity_test.go: SpecParityExceptions extended with the 4 new routes (RFC 8555 wire-protocol surface, not OpenAPI-shaped — same precedent as Phase 1a). - cmd/server/main.go: SetTransactor + SetAuditService on acmeService at startup so the WithinTx-based new-account / update / deactivate paths run with the same transactor instance shared across CertificateService / RevocationSvc / RenewalService. - docs/acme-server.md: Phase status updated; endpoints table grows new-account + account/ rows; new "JWS verification (Phase 1b)" section enumerates the 7 invariants the verifier enforces; phases-cross-reference table marks 1b live. - go.mod / go.sum: github.com/go-jose/go-jose/v4 v4.0.4 added. Atomicity: every account-state mutation writes its acme_accounts row + its audit_events row inside one repository.Transactor.WithinTx call — the canonical certctl atomicity contract (matches CertificateService.Create at internal/service/certificate.go:131). Idempotent re-registration explicitly does NOT write an audit row (RFC 8555 §7.3.1 returns the existing row unmodified). Tests: 16 jws_test.go cases + 11 service tests + 11 handler tests all pass under -short. Bad-signature test uses a real registered account whose stored JWK is a different keypair from the signer's, so the JWS parses cleanly but jose.Verify rejects — exercises the ErrJWSSignatureInvalid path directly. Engineering history: cowork/WORKSPACE-CHANGELOG.md "ACME-Server-1b". --- cmd/server/main.go | 14 +- docs/acme-server.md | 76 ++- go.mod | 1 + go.sum | 2 + internal/api/acme/account.go | 82 +++ internal/api/acme/jws.go | 487 ++++++++++++++++++ internal/api/acme/jws_test.go | 570 +++++++++++++++++++++ internal/api/handler/acme.go | 252 ++++++++- internal/api/handler/acme_handler_test.go | 303 ++++++++++- internal/api/router/openapi_parity_test.go | 16 +- internal/api/router/router.go | 4 + internal/domain/acme.go | 50 ++ internal/repository/postgres/acme.go | 172 +++++++ internal/service/acme.go | 359 +++++++++++-- internal/service/acme_test.go | 305 ++++++++++- 15 files changed, 2621 insertions(+), 72 deletions(-) create mode 100644 internal/api/acme/account.go create mode 100644 internal/api/acme/jws.go create mode 100644 internal/api/acme/jws_test.go create mode 100644 internal/domain/acme.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 1ea764b..437d48c 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -748,12 +748,16 @@ func main() { // by PathID; the AdminEST handler reads it at request time. estServices := map[string]*service.ESTService{} - // ACME server (RFC 8555 + RFC 9773 ARI) — Phase 1a foundation. - // Wires the directory + new-nonce surface against acmeRepo + the - // existing profileRepo (per-profile path resolution). Phases 1b-4 - // extend with the JWS-authenticated POST surface — the constructor - // signature stays stable; later phases call setters. + // ACME server (RFC 8555 + RFC 9773 ARI). Phase 1a wired the + // directory + new-nonce surface against acmeRepo + profileRepo; + // Phase 1b adds the JWS-authenticated POST surface (new-account + + // account/), which requires the transactor + audit service + // for per-op atomic-audit rows. SetTransactor mirrors the + // CertificateService.SetTransactor wiring at line 254 — same + // transactor instance shared across services. acmeService := service.NewACMEService(acmeRepo, profileRepo, cfg.ACMEServer) + acmeService.SetTransactor(transactor) + acmeService.SetAuditService(auditService) acmeHandler := handler.NewACMEHandler(acmeService) // Build the API router with all handlers diff --git a/docs/acme-server.md b/docs/acme-server.md index c50ce6b..bdde0fb 100644 --- a/docs/acme-server.md +++ b/docs/acme-server.md @@ -7,11 +7,11 @@ as an ACME issuer with no certctl-side modification — closing the "deploy a certctl agent on every K8s node" friction that costs deals to external PKI vendors today. -> **Phase status (2026-05-03):** Phase 1a (foundation — directory + -> new-nonce + per-profile routing). The directory document is live and -> ACME clients can fetch nonces. Account creation, JWS verification, -> orders, challenges, key rollover, revocation, and ARI all land in -> subsequent phases. Track shipped phases via +> **Phase status (2026-05-03):** Phase 1b — directory + new-nonce + +> new-account + account/{id} update + JWS verifier (RFC 7515 + go-jose +> v4). An ACME client can now run new-account end-to-end and register +> against a profile. Orders + challenges + key rollover + revocation + +> ARI land in subsequent phases. Track shipped phases via > `git log --grep='acme-server:'`. ## Configuration @@ -95,33 +95,65 @@ the `caBundle` requirement is flagged here in Phase 1a's docs because operators hit it the moment they try to point a real ACME client at certctl. -## Endpoints (Phase 1a) +## Endpoints (Phase 1b) Routes registered in `internal/api/router/router.go::RegisterHandlers`: -| Method | Path | RFC ref | Auth | Description | -|--------|-------------------------------------------|-----------------|-----------|-------------| -| GET | `/acme/profile/{id}/directory` | RFC 8555 §7.1.1 | unauth | Per-profile directory document. | -| HEAD | `/acme/profile/{id}/new-nonce` | RFC 8555 §7.2 | unauth | Returns 200 + Replay-Nonce header. | -| GET | `/acme/profile/{id}/new-nonce` | RFC 8555 §7.2 | unauth | Returns 204 + Replay-Nonce header. | -| GET | `/acme/directory` | RFC 8555 §7.1.1 | unauth | Shorthand path; mirrors per-profile when `CERTCTL_ACME_SERVER_DEFAULT_PROFILE_ID` is set. | -| HEAD | `/acme/new-nonce` | RFC 8555 §7.2 | unauth | Shorthand. | -| GET | `/acme/new-nonce` | RFC 8555 §7.2 | unauth | Shorthand. | +| Method | Path | RFC ref | Auth | Description | +|--------|--------------------------------------------|-----------------|----------|-------------| +| GET | `/acme/profile/{id}/directory` | RFC 8555 §7.1.1 | unauth | Per-profile directory document. | +| HEAD | `/acme/profile/{id}/new-nonce` | RFC 8555 §7.2 | unauth | Returns 200 + Replay-Nonce header. | +| GET | `/acme/profile/{id}/new-nonce` | RFC 8555 §7.2 | unauth | Returns 204 + Replay-Nonce header. | +| POST | `/acme/profile/{id}/new-account` | RFC 8555 §7.3 | JWS jwk | Register a new account; idempotent re-registration of an existing JWK returns the existing row. | +| POST | `/acme/profile/{id}/account/{acc_id}` | RFC 8555 §7.3.2 + §7.3.6 | JWS kid | Update contact list, deactivate, or POST-as-GET (RFC 8555 §6.3) to fetch the account. | +| GET | `/acme/directory` | RFC 8555 §7.1.1 | unauth | Shorthand path; mirrors per-profile when `CERTCTL_ACME_SERVER_DEFAULT_PROFILE_ID` is set. | +| HEAD | `/acme/new-nonce` | RFC 8555 §7.2 | unauth | Shorthand. | +| GET | `/acme/new-nonce` | RFC 8555 §7.2 | unauth | Shorthand. | +| POST | `/acme/new-account` | RFC 8555 §7.3 | JWS jwk | Shorthand. | +| POST | `/acme/account/{acc_id}` | RFC 8555 §7.3.2 + §7.3.6 | JWS kid | Shorthand. | -The remaining RFC 8555 endpoints (`new-account`, `account/{id}`, -`new-order`, `order/{id}`, `order/{id}/finalize`, `authz/{id}`, -`challenge/{id}`, `cert/{id}`, `key-change`, `revoke-cert`, -`renewal-info`) are advertised in the directory document but not yet -served — clients hitting them get a 404 until subsequent phases land. -The directory document includes their URLs because RFC 8555 doesn't -permit a partial directory. +The remaining RFC 8555 endpoints (`new-order`, `order/{id}`, +`order/{id}/finalize`, `authz/{id}`, `challenge/{id}`, `cert/{id}`, +`key-change`, `revoke-cert`, `renewal-info`) are advertised in the +directory document but not yet served — clients hitting them get a 404 +until subsequent phases land. The directory document includes their +URLs because RFC 8555 doesn't permit a partial directory. + +## JWS verification (Phase 1b) + +Every JWS-authenticated POST runs through the verifier at +`internal/api/acme/jws.go::VerifyJWS`. The verifier enforces: + +1. The JWS parses as a flattened single-signature object (multi-sig is + rejected per RFC 8555 §6.2). +2. The signature algorithm is in the closed allow-list `{RS256, ES256, + EdDSA}` per RFC 8555 §6.2 — `none`, `HS256`, and every other alg + are refused at parse time. +3. The protected header carries exactly one of `kid` (registered + account) or `jwk` (new-account flow); endpoints declare which they + require. +4. The protected header `url` matches the inbound request URL exactly. +5. The protected header `nonce` is consumed against the + `acme_nonces` store; missing / replayed / expired nonces return + `urn:ietf:params:acme:error:badNonce` per RFC 8555 §6.5.1. +6. On the `kid` path: the kid URL round-trips against the canonical + per-profile shape, the referenced account exists, and its status + is `valid`. Deactivated / revoked accounts cannot authenticate. +7. The signature verifies against the resolved key (registered + account's stored JWK on the kid path; embedded jwk on the jwk path). + +Every state-mutating account operation (create, contact update, +deactivate) writes its `acme_accounts` row and an `audit_events` row +inside one `repository.Transactor.WithinTx` call — the canonical +certctl atomicity contract (matches `service.CertificateService.Create` +at `internal/service/certificate.go:131`). ## Phases (cross-reference) | Phase | Status | Surface | |-------|-------------|---------| | 1a | live | directory + new-nonce + per-profile routing | -| 1b | not yet | new-account + JWS verifier (RFC 7515) | +| 1b | live | new-account + account/{id} + JWS verifier (RFC 7515 + go-jose v4) | | 2 | not yet | orders + authzs + finalize + cert download (trust_authenticated mode end-to-end) | | 3 | not yet | HTTP-01 + DNS-01 + TLS-ALPN-01 challenge validation | | 4 | not yet | key rollover + revocation + ARI (RFC 9773) | diff --git a/go.mod b/go.mod index 1d6bc38..1c3c7e9 100644 --- a/go.mod +++ b/go.mod @@ -52,6 +52,7 @@ require ( github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-units v0.5.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect + github.com/go-jose/go-jose/v4 v4.0.4 // indirect github.com/go-logr/logr v1.4.1 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.2.6 // indirect diff --git a/go.sum b/go.sum index d171bf6..b802527 100644 --- a/go.sum +++ b/go.sum @@ -141,6 +141,8 @@ github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeME github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= +github.com/go-jose/go-jose/v4 v4.0.4 h1:VsjPI33J0SB9vQM6PLmNjoHqMQNGPiZ0rHL7Ni7Q6/E= +github.com/go-jose/go-jose/v4 v4.0.4/go.mod h1:NKb5HO1EZccyMpiZNbdUw/14tiXNyUJh188dfnMCAfc= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= diff --git a/internal/api/acme/account.go b/internal/api/acme/account.go new file mode 100644 index 0000000..43ccea7 --- /dev/null +++ b/internal/api/acme/account.go @@ -0,0 +1,82 @@ +// Copyright (c) certctl +// SPDX-License-Identifier: BSL-1.1 + +package acme + +import ( + "github.com/shankar0123/certctl/internal/domain" +) + +// AccountResponseJSON is the wire shape RFC 8555 §7.1.2 mandates for +// account-resource responses (new-account success, account update, +// per-account GET POST-as-GET). +// +// The orders URL is mandatory per RFC 8555 §7.1.2.1; it points at the +// per-account orders list endpoint that Phase 2 implements. Phase 1b +// emits it as an empty placeholder ("orders not yet implemented") so +// the directory + new-account flow round-trips against ACME clients +// that expect the field present. +type AccountResponseJSON struct { + Status string `json:"status"` + Contact []string `json:"contact,omitempty"` + Orders string `json:"orders"` +} + +// MarshalAccount renders an ACMEAccount in RFC 8555 §7.1.2 wire shape. +// `ordersURL` is the per-account orders list URL the handler computes +// from the inbound request (scheme + host + profile path + account +// id); Phase 1b's handler passes it but Phase 2 wires the actual +// /acme/profile//account//orders endpoint. +func MarshalAccount(acct *domain.ACMEAccount, ordersURL string) AccountResponseJSON { + contact := acct.Contact + if contact == nil { + // RFC 8555 doesn't require contact be present, but cert-manager + // + lego both expect a stable shape. Emit [] rather than null. + contact = []string{} + } + return AccountResponseJSON{ + Status: string(acct.Status), + Contact: contact, + Orders: ordersURL, + } +} + +// NewAccountRequest is the payload shape RFC 8555 §7.3 mandates for +// new-account requests. The handler json.Unmarshals VerifiedRequest.Payload +// into this struct after JWS verify succeeds. +type NewAccountRequest struct { + // Contact is a list of mailto: / tel: URIs. Optional per RFC 8555 + // but operators typically supply at least one mailto:. + Contact []string `json:"contact,omitempty"` + // TermsOfServiceAgreed signals client consent to the operator's + // ToS document (advertised via meta.termsOfService). Phase 1b + // records the value but does NOT enforce — the meta field is + // informational only at this stage. + TermsOfServiceAgreed bool `json:"termsOfServiceAgreed,omitempty"` + // OnlyReturnExisting, when true, asks the server to return the + // existing account row for this JWK (RFC 8555 §7.3.1). When + // true and no account exists, the server MUST return 400 + + // urn:ietf:params:acme:error:accountDoesNotExist. + OnlyReturnExisting bool `json:"onlyReturnExisting,omitempty"` + // ExternalAccountBinding (EAB) is RFC 8555 §7.3.4. Phase 1b + // accepts the field but does NOT validate — EAB enforcement is + // a deliberate out-of-scope per the master prompt and lands as a + // follow-up if there's demand. Storing the raw envelope means a + // future phase can backfill validation against historical accounts. + ExternalAccountBinding map[string]interface{} `json:"externalAccountBinding,omitempty"` +} + +// AccountUpdateRequest is the payload shape for the account-update +// endpoint POST /acme/profile//account/ (RFC 8555 §7.3.2 + +// §7.3.6). Only `contact` and `status` are mutable per the spec. +type AccountUpdateRequest struct { + // Contact, when non-nil, replaces the account's contact list. + // nil means "leave unchanged" (distinct from empty []string{} + // which means "clear contacts" — cert-manager doesn't issue + // either, but the spec permits both). + Contact []string `json:"contact,omitempty"` + // Status, when set to "deactivated", retires the account per + // RFC 8555 §7.3.6. Other values are rejected — the operator + // path for revoked is via certctl's API, not via ACME. + Status string `json:"status,omitempty"` +} diff --git a/internal/api/acme/jws.go b/internal/api/acme/jws.go new file mode 100644 index 0000000..60d2266 --- /dev/null +++ b/internal/api/acme/jws.go @@ -0,0 +1,487 @@ +// Copyright (c) certctl +// SPDX-License-Identifier: BSL-1.1 + +package acme + +import ( + "crypto" + "encoding/base64" + "errors" + "fmt" + "net/http" + "strings" + + jose "github.com/go-jose/go-jose/v4" + + "github.com/shankar0123/certctl/internal/domain" +) + +// AllowedSignatureAlgorithms is the closed allow-list per RFC 8555 §6.2. +// ParseSigned takes this slice and rejects every other algorithm — +// in particular HS256 (symmetric — RFC 8555 forbids) and "none" +// (RFC 7515 §6.1 — alg confusion attack). +// +// Order is not load-bearing; the slice is value-copied by go-jose. +var AllowedSignatureAlgorithms = []jose.SignatureAlgorithm{ + jose.RS256, + jose.ES256, + jose.EdDSA, +} + +// JWS-verifier sentinel errors. Each maps to an RFC 8555 §6.7 +// problem type via mapJWSError below; handlers render via +// WriteProblem(w, p) on err. +var ( + ErrJWSMalformed = errors.New("acme jws: malformed") + ErrJWSWrongType = errors.New("acme jws: protected header `typ` must be `application/jose+json` or absent") + ErrJWSAlgorithmRejected = errors.New("acme jws: signature algorithm not in {RS256, ES256, EdDSA}") + ErrJWSMissingNonce = errors.New("acme jws: protected header `nonce` is required") + ErrJWSBadNonce = errors.New("acme jws: nonce missing, replayed, or expired") + ErrJWSMissingURL = errors.New("acme jws: protected header `url` is required") + ErrJWSURLMismatch = errors.New("acme jws: protected header `url` does not match request URL") + ErrJWSBothKidAndJWK = errors.New("acme jws: protected header MUST contain exactly one of `kid` or `jwk`") + ErrJWSNeitherKidNorJWK = errors.New("acme jws: protected header MUST contain exactly one of `kid` or `jwk`") + ErrJWSExpectKidGotJWK = errors.New("acme jws: this endpoint requires `kid` (registered account); got `jwk`") + ErrJWSExpectJWKGotKid = errors.New("acme jws: this endpoint requires `jwk` (new account); got `kid`") + ErrJWSInvalidJWK = errors.New("acme jws: embedded JWK is invalid") + ErrJWSSignatureInvalid = errors.New("acme jws: signature did not verify") + ErrJWSPayloadMismatch = errors.New("acme jws: post-verify payload differs from pre-verify payload") + ErrJWSAccountNotFound = errors.New("acme jws: kid points at unknown account") + ErrJWSAccountInactive = errors.New("acme jws: account status is not `valid`") +) + +// VerifiedRequest is the JWS-verified envelope a handler hands to its +// service-layer entry point. Fields are populated based on the auth +// path: `kid` requests carry Account (and AccountKey is the registered +// JWK); `jwk` requests (new-account only) carry JWK. +// +// Payload is the bytes the JWS signed — the handler json.Unmarshals +// into the per-endpoint payload struct. +type VerifiedRequest struct { + // Payload is the signed body bytes (post-Verify). + Payload []byte + // Algorithm is the negotiated alg (RS256 / ES256 / EdDSA), echoed + // from sig.Protected.Algorithm post-allow-list-check. + Algorithm string + // URL is the protected-header `url` value, asserted equal to the + // inbound request URL. + URL string + // Nonce is the protected-header `nonce` value, asserted consumed + // from the nonce store. + Nonce string + // Account is non-nil on the `kid` path (registered account + // authenticating). Always nil on the `jwk` path. + Account *domain.ACMEAccount + // JWK is non-nil on the `jwk` path (new-account flow). Always nil + // on the `kid` path. + JWK *jose.JSONWebKey +} + +// AccountLookup is the minimum surface VerifyJWS needs to resolve a +// `kid` request's account. The repository layer satisfies this; tests +// inject in-memory fakes. +type AccountLookup interface { + // LookupAccount returns the account by ID. Returns + // ErrJWSAccountNotFound if the row doesn't exist. + LookupAccount(accountID string) (*domain.ACMEAccount, error) +} + +// NonceConsumer is the minimum surface the verifier needs to consume +// the protected-header `nonce`. Returns nil on success, or an error +// (typically sql.ErrNoRows from the postgres repo) on missing / +// replayed / expired. The verifier wraps any non-nil error in +// ErrJWSBadNonce so handlers don't need to distinguish. +type NonceConsumer interface { + ConsumeNonce(nonce string) error +} + +// VerifierConfig wires the verifier's runtime dependencies + policy. +// Constructed by the handler/service layer once at startup; one +// instance per ACMEService is sufficient. +type VerifierConfig struct { + // Accounts looks up registered accounts on the kid path. + Accounts AccountLookup + // Nonces consumes the protected-header nonce. + Nonces NonceConsumer + // AccountKID returns the canonical kid URL the server expects + // inbound requests to use for a given account ID. The verifier + // asserts the request's `kid` matches what AccountKID(acct.ID) + // produces — this prevents a stolen account-id-from-one-server + // from being replayed against another. The handler computes + // the URL from the inbound request's scheme + host + profile. + AccountKID func(accountID string) string +} + +// VerifyOptions bound a single verify call. ExpectNewAccount inverts +// the kid-vs-jwk default: new-account demands jwk, every other +// endpoint demands kid. +type VerifyOptions struct { + // ExpectNewAccount=true means "expect jwk in the protected header, + // reject kid." Used by /new-account. + // ExpectNewAccount=false means "expect kid in the protected header, + // reject jwk." Used by everything else. + ExpectNewAccount bool +} + +// VerifyJWS is the canonical entry point. It enforces: +// +// 1. Body parses as a flattened JWS with exactly one signature +// (RFC 8555 §6.2 forbids multi-sig). +// 2. Algorithm is in the {RS256, ES256, EdDSA} allow-list. +// 3. Protected header carries exactly one of `kid` / `jwk` per +// ExpectNewAccount. +// 4. Protected header carries `url` matching the inbound request URL +// exactly. +// 5. Protected header carries `nonce` that consumes successfully +// against the nonce store (badNonce on miss/replay/expiry). +// 6. Signature verifies against the resolved key (registered +// account's stored JWK on kid path; embedded jwk on jwk path). +// 7. Post-verify payload bytes equal pre-verify +// UnsafePayloadWithoutVerification (defense in depth — go-jose +// guarantees this, but assert anyway). +// +// On success returns VerifiedRequest; the handler json.Unmarshals +// Payload into the per-endpoint payload struct. +// +// The `requestURL` argument is what the handler computed from the +// inbound *http.Request (scheme + host + path). VerifyJWS does NOT +// see r itself — keeping net/http out of the package surface lets +// the verifier be tested without httptest. +func VerifyJWS(cfg VerifierConfig, body []byte, requestURL string, opts VerifyOptions) (*VerifiedRequest, error) { + jws, err := jose.ParseSigned(string(body), AllowedSignatureAlgorithms) + if err != nil { + // ParseSigned errors lump together "wrong format" and "alg + // not in allow-list." Both are operator-meaningful as + // "malformed" — the alg case is not exploitable by leaking + // the allow-list. + return nil, fmt.Errorf("%w: %v", ErrJWSMalformed, err) + } + // RFC 8555 §6.2: ACME forbids JWS multi-signature. Reject anything + // other than exactly one signature so a maliciously-crafted + // multi-sig blob can't trigger ambiguous downstream behavior. + if len(jws.Signatures) != 1 { + return nil, fmt.Errorf("%w: multi-signature JWS rejected", ErrJWSMalformed) + } + sig := jws.Signatures[0] + + // Defense-in-depth: ParseSigned rejected non-allow-list algs + // already, but a corrupted Signatures slice could still slip + // through. Verify the field directly. + if !algorithmAllowed(sig.Protected.Algorithm) { + return nil, fmt.Errorf("%w: %s", ErrJWSAlgorithmRejected, sig.Protected.Algorithm) + } + + // Protected-header `typ` (RFC 8555 §6.2): when present, must be + // "application/jose+json". Many ACME clients (including + // cert-manager) omit it; treat absent as OK. + if typ := sig.Protected.ExtraHeaders[jose.HeaderKey("typ")]; typ != nil { + typStr, ok := typ.(string) + if !ok || (typStr != "application/jose+json" && typStr != "") { + return nil, fmt.Errorf("%w: got %q", ErrJWSWrongType, typ) + } + } + + // Protected-header `url` is mandatory per RFC 8555 §6.4. Compare + // to the inbound request URL exactly (scheme+host+path); a + // mismatch indicates either a bug in the client or an attempt to + // replay a JWS signed for a different URL. + urlVal, err := extractStringHeader(sig.Protected.ExtraHeaders, "url") + if err != nil { + return nil, ErrJWSMissingURL + } + if urlVal == "" { + return nil, ErrJWSMissingURL + } + if urlVal != requestURL { + return nil, fmt.Errorf("%w: header=%q request=%q", ErrJWSURLMismatch, urlVal, requestURL) + } + + // Protected-header `nonce` is mandatory (RFC 8555 §6.5). Check + // it BEFORE running Verify — if the nonce is bad we don't want to + // burn CPU on signature verification. + nonce := sig.Protected.Nonce + if nonce == "" { + return nil, ErrJWSMissingNonce + } + if err := cfg.Nonces.ConsumeNonce(nonce); err != nil { + return nil, fmt.Errorf("%w: %v", ErrJWSBadNonce, err) + } + + // Protected header MUST contain exactly one of kid / jwk per + // RFC 8555 §6.2. Both-set or neither-set are rejected. + hasKid := sig.Protected.KeyID != "" + hasJWK := sig.Protected.JSONWebKey != nil + if hasKid && hasJWK { + return nil, ErrJWSBothKidAndJWK + } + if !hasKid && !hasJWK { + return nil, ErrJWSNeitherKidNorJWK + } + + // Per-endpoint kid-vs-jwk policy. + if opts.ExpectNewAccount && hasKid { + return nil, ErrJWSExpectJWKGotKid + } + if !opts.ExpectNewAccount && hasJWK { + return nil, ErrJWSExpectKidGotJWK + } + + // Resolve the verification key and (kid path) the corresponding + // account row. + var ( + verifyKey interface{} + account *domain.ACMEAccount + jwkOut *jose.JSONWebKey + ) + if hasKid { + accountID, err := accountIDFromKID(sig.Protected.KeyID, cfg) + if err != nil { + return nil, err + } + acct, err := cfg.Accounts.LookupAccount(accountID) + if err != nil { + return nil, err + } + if acct.Status != domain.ACMEAccountStatusValid { + return nil, fmt.Errorf("%w: status=%s", ErrJWSAccountInactive, acct.Status) + } + // The account's stored JWK is what we verify against. The + // JWKPEM round-trips through ParseJWKFromPEM; tests inject + // pre-parsed keys to keep the unit suite hermetic. + key, err := ParseJWKFromPEM(acct.JWKPEM) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrJWSInvalidJWK, err) + } + verifyKey = key.Key + account = acct + } else { + jwk := sig.Protected.JSONWebKey + if !jwk.Valid() { + return nil, ErrJWSInvalidJWK + } + verifyKey = jwk.Key + jwkOut = jwk + } + + // Run the actual signature verification. go-jose returns the + // post-verify payload bytes; we sanity-check them against the + // pre-verify view. + verified, err := jws.Verify(verifyKey) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrJWSSignatureInvalid, err) + } + preVerify := jws.UnsafePayloadWithoutVerification() + if string(verified) != string(preVerify) { + // Should be impossible under correct go-jose use; fail loudly. + return nil, ErrJWSPayloadMismatch + } + + return &VerifiedRequest{ + Payload: verified, + Algorithm: sig.Protected.Algorithm, + URL: urlVal, + Nonce: nonce, + Account: account, + JWK: jwkOut, + }, nil +} + +// MapJWSErrorToProblem renders a JWS verifier error as an RFC 7807 + +// RFC 8555 §6.7 Problem the handler emits via WriteProblem. +// +// All errors map to a documented ACME error type — no internal-state +// leakage per master-prompt criterion #10. Operator-actionable detail +// strings carry the failure category (badNonce, malformed, etc.) but +// not raw err.Error() output. +func MapJWSErrorToProblem(err error) Problem { + switch { + case errors.Is(err, ErrJWSBadNonce): + return BadNonce("nonce missing, replayed, or expired") + case errors.Is(err, ErrJWSMissingNonce): + return BadNonce("protected header `nonce` is required") + case errors.Is(err, ErrJWSURLMismatch), errors.Is(err, ErrJWSMissingURL): + return Problem{ + Type: "urn:ietf:params:acme:error:unauthorized", + Detail: "protected header `url` mismatch or missing", + Status: http.StatusUnauthorized, + } + case errors.Is(err, ErrJWSAccountNotFound): + return AccountDoesNotExist("kid points at unknown account") + case errors.Is(err, ErrJWSAccountInactive): + return Problem{ + Type: "urn:ietf:params:acme:error:unauthorized", + Detail: "account status is not `valid`", + Status: http.StatusUnauthorized, + } + case errors.Is(err, ErrJWSSignatureInvalid): + return Problem{ + Type: "urn:ietf:params:acme:error:unauthorized", + Detail: "signature did not verify", + Status: http.StatusUnauthorized, + } + case errors.Is(err, ErrJWSAlgorithmRejected): + return Malformed("signature algorithm not allowed (RFC 8555 §6.2: RS256, ES256, EdDSA only)") + case errors.Is(err, ErrJWSExpectJWKGotKid): + return Malformed("this endpoint requires `jwk` (new-account flow); got `kid`") + case errors.Is(err, ErrJWSExpectKidGotJWK): + return Malformed("this endpoint requires `kid` (registered account); got `jwk`") + case errors.Is(err, ErrJWSBothKidAndJWK), errors.Is(err, ErrJWSNeitherKidNorJWK): + return Malformed("protected header MUST contain exactly one of `kid` or `jwk`") + case errors.Is(err, ErrJWSInvalidJWK): + return Malformed("invalid or unsupported JWK") + case errors.Is(err, ErrJWSWrongType): + return Malformed("protected header `typ` must be `application/jose+json`") + case errors.Is(err, ErrJWSPayloadMismatch): + return ServerInternal("JWS payload integrity check failed") + case errors.Is(err, ErrJWSMalformed): + return Malformed("malformed JWS") + default: + return Malformed("malformed request") + } +} + +// algorithmAllowed verifies the post-parse algorithm is in the +// approved set. ParseSigned already rejects non-allow-list algs but +// re-checking here protects against go-jose contract changes. +func algorithmAllowed(alg string) bool { + for _, a := range AllowedSignatureAlgorithms { + if string(a) == alg { + return true + } + } + return false +} + +// extractStringHeader pulls a string-typed entry from ExtraHeaders. +// Returns ("", nil) when the key is absent so the caller can +// distinguish absent (empty string) from non-string-shaped (error). +func extractStringHeader(extra map[jose.HeaderKey]interface{}, name string) (string, error) { + v, ok := extra[jose.HeaderKey(name)] + if !ok { + return "", nil + } + s, ok := v.(string) + if !ok { + return "", fmt.Errorf("acme jws: header %q is not a string: %T", name, v) + } + return s, nil +} + +// accountIDFromKID extracts the account ID from a kid URL. RFC 8555 +// §6.2 says kid is the URL the server returned in the Location +// header on new-account; we expect the canonical +// +// :///acme/profile//account/ +// +// shape and trust the verifier-config-supplied AccountKID to round- +// trip the full URL match. Phase 1b: extract the account ID by +// trimming the URL prefix; Phase 1b's caller asserts the round-trip +// equals the original kid. +func accountIDFromKID(kid string, cfg VerifierConfig) (string, error) { + // Trim off everything up to the last "/account/" — the suffix is + // the account ID. The Phase-1b account-id format is + // "acme-acc-<...>" (alphanumeric + hyphen), so we don't need to + // URL-unescape. + idx := strings.LastIndex(kid, "/account/") + if idx < 0 { + return "", fmt.Errorf("%w: kid does not match expected /account/ shape", ErrJWSMalformed) + } + accountID := kid[idx+len("/account/"):] + if accountID == "" { + return "", fmt.Errorf("%w: kid has empty account id", ErrJWSMalformed) + } + // Round-trip: confirm the canonical kid for this account-id + // matches what the client sent. Catches accidental cross-profile + // replay. + if cfg.AccountKID != nil { + expected := cfg.AccountKID(accountID) + if expected != kid { + return "", fmt.Errorf("%w: kid does not match canonical URL", ErrJWSMalformed) + } + } + return accountID, nil +} + +// ParseJWKFromPEM parses a JWK previously serialized by JWKToPEM. +// Used by the verifier on the kid path: the registered account row's +// JWKPEM column round-trips through here to recover the key bytes +// used for signature verification. +// +// The PEM block is JSON-encoded JWK (we use PEM as the wire format +// for the column to keep the schema text-shaped + line-friendly for +// SQL diffs). Block type is "ACME ACCOUNT JWK". +func ParseJWKFromPEM(pemString string) (*jose.JSONWebKey, error) { + // Strip the PEM header / footer; everything between is base64. + const header = "-----BEGIN ACME ACCOUNT JWK-----" + const footer = "-----END ACME ACCOUNT JWK-----" + s := strings.TrimSpace(pemString) + if !strings.HasPrefix(s, header) { + return nil, fmt.Errorf("acme jws: pem missing header") + } + s = strings.TrimPrefix(s, header) + idx := strings.Index(s, footer) + if idx < 0 { + return nil, fmt.Errorf("acme jws: pem missing footer") + } + body := strings.TrimSpace(s[:idx]) + body = strings.ReplaceAll(body, "\n", "") + body = strings.ReplaceAll(body, "\r", "") + raw, err := base64.StdEncoding.DecodeString(body) + if err != nil { + return nil, fmt.Errorf("acme jws: decode pem body: %w", err) + } + jwk := new(jose.JSONWebKey) + if err := jwk.UnmarshalJSON(raw); err != nil { + return nil, fmt.Errorf("acme jws: parse jwk json: %w", err) + } + if !jwk.Valid() { + return nil, fmt.Errorf("acme jws: jwk did not validate") + } + return jwk, nil +} + +// JWKToPEM is the inverse of ParseJWKFromPEM. Used at account creation +// time to persist the public-only JWK to the acme_accounts row. +func JWKToPEM(jwk *jose.JSONWebKey) (string, error) { + raw, err := jwk.MarshalJSON() + if err != nil { + return "", fmt.Errorf("acme jws: marshal jwk json: %w", err) + } + encoded := base64.StdEncoding.EncodeToString(raw) + // Wrap to 64-char lines for diff-friendliness. + var buf strings.Builder + buf.WriteString("-----BEGIN ACME ACCOUNT JWK-----\n") + for i := 0; i < len(encoded); i += 64 { + end := i + 64 + if end > len(encoded) { + end = len(encoded) + } + buf.WriteString(encoded[i:end]) + buf.WriteByte('\n') + } + buf.WriteString("-----END ACME ACCOUNT JWK-----\n") + return buf.String(), nil +} + +// JWKThumbprint computes the RFC 7638 thumbprint of jwk and returns +// it as a base64url-no-padding string. The (profile_id, thumbprint) +// pair uniquely identifies an account per profile; new-account uses +// it for idempotency (RFC 8555 §7.3.1). +func JWKThumbprint(jwk *jose.JSONWebKey) (string, error) { + raw, err := jwk.Thumbprint(crypto.SHA256) + if err != nil { + return "", fmt.Errorf("acme jws: thumbprint: %w", err) + } + return base64.RawURLEncoding.EncodeToString(raw), nil +} + +// AccountID derives the canonical certctl account ID from a JWK +// thumbprint: "acme-acc-" + base64url-no-pad-thumbprint. The output is +// stable across clients (same JWK → same ID) so the new-account +// idempotency check at RFC 8555 §7.3.1 holds without an additional +// lookup. +func AccountID(thumbprint string) string { + // base64url-no-pad already produces alphanumeric + `-_`; we keep + // `-_` as part of the certctl-readable prefix shape. + return "acme-acc-" + thumbprint +} diff --git a/internal/api/acme/jws_test.go b/internal/api/acme/jws_test.go new file mode 100644 index 0000000..3863f37 --- /dev/null +++ b/internal/api/acme/jws_test.go @@ -0,0 +1,570 @@ +// Copyright (c) certctl +// SPDX-License-Identifier: BSL-1.1 + +package acme + +import ( + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "errors" + "strings" + "testing" + + jose "github.com/go-jose/go-jose/v4" + + "github.com/shankar0123/certctl/internal/domain" +) + +// --- test fixtures + helpers -------------------------------------------- + +// stubAccounts implements AccountLookup with a static map. +type stubAccounts struct { + byID map[string]*domain.ACMEAccount +} + +func (s *stubAccounts) LookupAccount(accountID string) (*domain.ACMEAccount, error) { + acct, ok := s.byID[accountID] + if !ok { + return nil, ErrJWSAccountNotFound + } + return acct, nil +} + +// stubNonces implements NonceConsumer with a one-shot map. Used == true +// after first Consume. +type stubNonces struct { + known map[string]bool // nonce → consumed? +} + +func newStubNonces(nonces ...string) *stubNonces { + s := &stubNonces{known: make(map[string]bool, len(nonces))} + for _, n := range nonces { + s.known[n] = false + } + return s +} + +func (s *stubNonces) ConsumeNonce(nonce string) error { + used, ok := s.known[nonce] + if !ok { + return errors.New("not found") + } + if used { + return errors.New("already used") + } + s.known[nonce] = true + return nil +} + +const testKID = "https://server/acme/profile/prof-corp/account/acme-acc-test123" +const testURL = "https://server/acme/profile/prof-corp/new-account" + +func testAccountKID(accountID string) string { + return "https://server/acme/profile/prof-corp/account/" + accountID +} + +// genRSAKey, genECKey, genEdKey return a freshly-generated keypair +// suitable for signing JWS objects. Tests share the same key per-case +// to keep failures localized to the verifier rather than cross-test +// state. +func genRSAKey(t *testing.T) *rsa.PrivateKey { + t.Helper() + k, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa keygen: %v", err) + } + return k +} + +func genECKey(t *testing.T) *ecdsa.PrivateKey { + t.Helper() + k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("ecdsa keygen: %v", err) + } + return k +} + +func genEdKey(t *testing.T) (ed25519.PublicKey, ed25519.PrivateKey) { + t.Helper() + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519 keygen: %v", err) + } + return pub, priv +} + +// signWithKID builds a flattened JWS using kid (registered-account flow). +func signWithKID(t *testing.T, key interface{}, alg jose.SignatureAlgorithm, kid, url, nonce string, payload interface{}) string { + t.Helper() + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: alg, Key: key}, + (&jose.SignerOptions{}). + WithHeader(jose.HeaderKey("url"), url). + WithHeader("kid", kid). + WithHeader("nonce", nonce), + ) + if err != nil { + t.Fatalf("new signer: %v", err) + } + jws, err := signer.Sign(body) + if err != nil { + t.Fatalf("sign: %v", err) + } + out := jws.FullSerialize() + return out +} + +// signWithJWK builds a flattened JWS embedding the public JWK +// (new-account flow). The Signer with EmbedJWK=true attaches the +// JSONWebKey to the protected header. +func signWithJWK(t *testing.T, key interface{}, alg jose.SignatureAlgorithm, url, nonce string, payload interface{}) string { + t.Helper() + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("marshal payload: %v", err) + } + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: alg, Key: key}, + (&jose.SignerOptions{EmbedJWK: true}). + WithHeader(jose.HeaderKey("url"), url). + WithHeader("nonce", nonce), + ) + if err != nil { + t.Fatalf("new signer (embed jwk): %v", err) + } + jws, err := signer.Sign(body) + if err != nil { + t.Fatalf("sign: %v", err) + } + return jws.FullSerialize() +} + +// --- JWK round-trip helpers -------------------------------------------- + +func TestJWKRoundTrip_RSA(t *testing.T) { + k := genRSAKey(t) + jwk := &jose.JSONWebKey{Key: &k.PublicKey} + pem, err := JWKToPEM(jwk) + if err != nil { + t.Fatalf("JWKToPEM: %v", err) + } + if !strings.Contains(pem, "BEGIN ACME ACCOUNT JWK") { + t.Fatalf("PEM missing header: %s", pem) + } + parsed, err := ParseJWKFromPEM(pem) + if err != nil { + t.Fatalf("ParseJWKFromPEM: %v", err) + } + if !parsed.Valid() { + t.Fatal("parsed jwk is not valid") + } +} + +func TestJWKThumbprint_StableAcrossKeyTypes(t *testing.T) { + rsaJWK := &jose.JSONWebKey{Key: &genRSAKey(t).PublicKey} + rsaThumb1, err := JWKThumbprint(rsaJWK) + if err != nil { + t.Fatalf("rsa thumb: %v", err) + } + rsaThumb2, err := JWKThumbprint(rsaJWK) + if err != nil { + t.Fatalf("rsa thumb 2: %v", err) + } + if rsaThumb1 != rsaThumb2 { + t.Errorf("thumbprint not stable: %q vs %q", rsaThumb1, rsaThumb2) + } + // Different keys produce different thumbprints. + otherJWK := &jose.JSONWebKey{Key: &genRSAKey(t).PublicKey} + otherThumb, err := JWKThumbprint(otherJWK) + if err != nil { + t.Fatalf("other thumb: %v", err) + } + if rsaThumb1 == otherThumb { + t.Error("two distinct keys collided on thumbprint") + } +} + +// --- VerifyJWS happy paths --------------------------------------------- + +func TestVerifyJWS_Happy_RS256_KID(t *testing.T) { + key := genRSAKey(t) + jwk := &jose.JSONWebKey{Key: &key.PublicKey} + pem, _ := JWKToPEM(jwk) + thumb, _ := JWKThumbprint(jwk) + + cfg := VerifierConfig{ + Accounts: &stubAccounts{byID: map[string]*domain.ACMEAccount{ + "acme-acc-test123": { + AccountID: "acme-acc-test123", JWKPEM: pem, JWKThumbprint: thumb, + Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp", + }, + }}, + Nonces: newStubNonces("nonce-001"), + AccountKID: testAccountKID, + } + body := signWithKID(t, key, jose.RS256, testKID, testURL, "nonce-001", map[string]any{"hello": "world"}) + + v, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: false}) + if err != nil { + t.Fatalf("VerifyJWS: %v", err) + } + if v.Account == nil || v.Account.AccountID != "acme-acc-test123" { + t.Errorf("account = %+v, want acme-acc-test123", v.Account) + } + if v.JWK != nil { + t.Errorf("JWK should be nil on kid path; got %+v", v.JWK) + } + if v.Nonce != "nonce-001" { + t.Errorf("nonce = %q", v.Nonce) + } + if v.URL != testURL { + t.Errorf("url = %q", v.URL) + } + if v.Algorithm != "RS256" { + t.Errorf("algorithm = %q", v.Algorithm) + } + var payload map[string]any + if err := json.Unmarshal(v.Payload, &payload); err != nil { + t.Fatalf("payload not json: %v", err) + } + if payload["hello"] != "world" { + t.Errorf("payload = %+v", payload) + } +} + +func TestVerifyJWS_Happy_ES256_JWK(t *testing.T) { + key := genECKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces("nonce-002"), + AccountKID: testAccountKID, + } + body := signWithJWK(t, key, jose.ES256, testURL, "nonce-002", map[string]any{"new": "account"}) + v, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: true}) + if err != nil { + t.Fatalf("VerifyJWS: %v", err) + } + if v.JWK == nil { + t.Fatal("JWK should be populated on jwk path") + } + if v.Account != nil { + t.Errorf("Account should be nil on jwk path; got %+v", v.Account) + } + if v.Algorithm != "ES256" { + t.Errorf("algorithm = %q", v.Algorithm) + } +} + +func TestVerifyJWS_Happy_EdDSA_KID(t *testing.T) { + pub, priv := genEdKey(t) + jwk := &jose.JSONWebKey{Key: pub} + pem, _ := JWKToPEM(jwk) + thumb, _ := JWKThumbprint(jwk) + + cfg := VerifierConfig{ + Accounts: &stubAccounts{byID: map[string]*domain.ACMEAccount{ + "acme-acc-ed1": { + AccountID: "acme-acc-ed1", JWKPEM: pem, JWKThumbprint: thumb, + Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp", + }, + }}, + Nonces: newStubNonces("nonce-003"), + AccountKID: testAccountKID, + } + kid := testAccountKID("acme-acc-ed1") + body := signWithKID(t, priv, jose.EdDSA, kid, testURL, "nonce-003", struct{}{}) + + v, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: false}) + if err != nil { + t.Fatalf("VerifyJWS: %v", err) + } + if v.Algorithm != "EdDSA" { + t.Errorf("algorithm = %q, want EdDSA", v.Algorithm) + } +} + +// --- VerifyJWS rejection paths ----------------------------------------- + +func TestVerifyJWS_Reject_AlgNotInAllowList(t *testing.T) { + // HS256 (HMAC-SHA256, symmetric) is forbidden by RFC 8555 §6.2. + key := []byte("supersecretkey32byteslongforhmac") + signer, err := jose.NewSigner( + jose.SigningKey{Algorithm: jose.HS256, Key: key}, + (&jose.SignerOptions{}). + WithHeader(jose.HeaderKey("url"), testURL). + WithHeader("kid", testKID). + WithHeader("nonce", "n"), + ) + if err != nil { + t.Fatalf("hs256 signer: %v", err) + } + jws, _ := signer.Sign([]byte("{}")) + body := jws.FullSerialize() + + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces("n"), + AccountKID: testAccountKID, + } + _, err = VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{}) + if err == nil { + t.Fatal("expected algorithm-rejected error; got nil") + } + // ParseSigned filters the alg before we ever see the JWS, so the + // error wraps ErrJWSMalformed (the verifier can't distinguish + // "wrong format" from "bad alg" at this layer — both manifest as + // malformed). + if !errors.Is(err, ErrJWSMalformed) && !errors.Is(err, ErrJWSAlgorithmRejected) { + t.Errorf("err = %v; want ErrJWSMalformed or ErrJWSAlgorithmRejected", err) + } +} + +func TestVerifyJWS_Reject_BadSignature(t *testing.T) { + signingKey := genRSAKey(t) + // The verifier resolves the account row's stored JWK and uses its + // public component as the verify key. Register an account whose + // stored JWK is a DIFFERENT key — same shape, different material. + // The JWS parses cleanly but Verify returns "verification failed". + storedKey := genRSAKey(t) + storedJWK := &jose.JSONWebKey{Key: &storedKey.PublicKey} + storedPEM, _ := JWKToPEM(storedJWK) + storedThumb, _ := JWKThumbprint(storedJWK) + + cfg := VerifierConfig{ + Accounts: &stubAccounts{byID: map[string]*domain.ACMEAccount{ + "acme-acc-test123": { + AccountID: "acme-acc-test123", JWKPEM: storedPEM, JWKThumbprint: storedThumb, + Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp", + }, + }}, + Nonces: newStubNonces("n1"), + AccountKID: testAccountKID, + } + body := signWithKID(t, signingKey, jose.RS256, testKID, testURL, "n1", map[string]any{"x": 1}) + + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{}) + if err == nil { + t.Fatal("expected signature-invalid error; got nil") + } + if !errors.Is(err, ErrJWSSignatureInvalid) { + t.Errorf("err = %v; want ErrJWSSignatureInvalid wrapper", err) + } +} + +func TestVerifyJWS_Reject_NonceMissingFromHeader(t *testing.T) { + key := genRSAKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces(), + AccountKID: testAccountKID, + } + signer, _ := jose.NewSigner( + jose.SigningKey{Algorithm: jose.RS256, Key: key}, + (&jose.SignerOptions{EmbedJWK: true}). + WithHeader(jose.HeaderKey("url"), testURL), + // nonce omitted intentionally + ) + jws, _ := signer.Sign([]byte("{}")) + body := jws.FullSerialize() + + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: true}) + if !errors.Is(err, ErrJWSMissingNonce) { + t.Errorf("err = %v; want ErrJWSMissingNonce", err) + } +} + +func TestVerifyJWS_Reject_NonceUnknown(t *testing.T) { + key := genRSAKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces(), // no nonces issued + AccountKID: testAccountKID, + } + body := signWithJWK(t, key, jose.RS256, testURL, "ghost-nonce", map[string]any{}) + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: true}) + if !errors.Is(err, ErrJWSBadNonce) { + t.Errorf("err = %v; want ErrJWSBadNonce", err) + } +} + +func TestVerifyJWS_Reject_NonceReplay(t *testing.T) { + key := genRSAKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces("n-replay"), + AccountKID: testAccountKID, + } + body := signWithJWK(t, key, jose.RS256, testURL, "n-replay", map[string]any{}) + if _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: true}); err != nil { + t.Fatalf("first verify: %v", err) + } + // Replay — same JWS, second time. + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: true}) + if !errors.Is(err, ErrJWSBadNonce) { + t.Errorf("err = %v; want ErrJWSBadNonce on replay", err) + } +} + +func TestVerifyJWS_Reject_URLMismatch(t *testing.T) { + key := genRSAKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces("n-url"), + AccountKID: testAccountKID, + } + body := signWithJWK(t, key, jose.RS256, testURL, "n-url", map[string]any{}) + // Hand the verifier a different URL than the one signed. + _, err := VerifyJWS(cfg, []byte(body), "https://server/acme/different", VerifyOptions{ExpectNewAccount: true}) + if !errors.Is(err, ErrJWSURLMismatch) { + t.Errorf("err = %v; want ErrJWSURLMismatch", err) + } +} + +func TestVerifyJWS_Reject_ExpectKidGotJWK(t *testing.T) { + key := genRSAKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces("n-mix1"), + AccountKID: testAccountKID, + } + body := signWithJWK(t, key, jose.RS256, testURL, "n-mix1", map[string]any{}) + // New-account expects jwk; we set ExpectNewAccount=false so this + // flow demands kid. + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: false}) + if !errors.Is(err, ErrJWSExpectKidGotJWK) { + t.Errorf("err = %v; want ErrJWSExpectKidGotJWK", err) + } +} + +func TestVerifyJWS_Reject_ExpectJWKGotKid(t *testing.T) { + key := genRSAKey(t) + jwk := &jose.JSONWebKey{Key: &key.PublicKey} + pem, _ := JWKToPEM(jwk) + thumb, _ := JWKThumbprint(jwk) + cfg := VerifierConfig{ + Accounts: &stubAccounts{byID: map[string]*domain.ACMEAccount{ + "acme-acc-test123": { + AccountID: "acme-acc-test123", JWKPEM: pem, JWKThumbprint: thumb, + Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp", + }, + }}, + Nonces: newStubNonces("n-mix2"), + AccountKID: testAccountKID, + } + body := signWithKID(t, key, jose.RS256, testKID, testURL, "n-mix2", map[string]any{}) + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{ExpectNewAccount: true}) + if !errors.Is(err, ErrJWSExpectJWKGotKid) { + t.Errorf("err = %v; want ErrJWSExpectJWKGotKid", err) + } +} + +func TestVerifyJWS_Reject_AccountUnknown(t *testing.T) { + key := genRSAKey(t) + cfg := VerifierConfig{ + Accounts: &stubAccounts{}, + Nonces: newStubNonces("n-acct"), + AccountKID: testAccountKID, + } + body := signWithKID(t, key, jose.RS256, testKID, testURL, "n-acct", map[string]any{}) + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{}) + if !errors.Is(err, ErrJWSAccountNotFound) { + t.Errorf("err = %v; want ErrJWSAccountNotFound", err) + } +} + +func TestVerifyJWS_Reject_AccountDeactivated(t *testing.T) { + key := genRSAKey(t) + jwk := &jose.JSONWebKey{Key: &key.PublicKey} + pem, _ := JWKToPEM(jwk) + thumb, _ := JWKThumbprint(jwk) + cfg := VerifierConfig{ + Accounts: &stubAccounts{byID: map[string]*domain.ACMEAccount{ + "acme-acc-test123": { + AccountID: "acme-acc-test123", JWKPEM: pem, JWKThumbprint: thumb, + Status: domain.ACMEAccountStatusDeactivated, // ← deactivated + ProfileID: "prof-corp", + }, + }}, + Nonces: newStubNonces("n-deact"), + AccountKID: testAccountKID, + } + body := signWithKID(t, key, jose.RS256, testKID, testURL, "n-deact", map[string]any{}) + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{}) + if !errors.Is(err, ErrJWSAccountInactive) { + t.Errorf("err = %v; want ErrJWSAccountInactive", err) + } +} + +func TestVerifyJWS_Reject_KIDMismatchesProfile(t *testing.T) { + key := genRSAKey(t) + jwk := &jose.JSONWebKey{Key: &key.PublicKey} + pem, _ := JWKToPEM(jwk) + thumb, _ := JWKThumbprint(jwk) + cfg := VerifierConfig{ + Accounts: &stubAccounts{byID: map[string]*domain.ACMEAccount{ + "acme-acc-test123": { + AccountID: "acme-acc-test123", JWKPEM: pem, JWKThumbprint: thumb, + Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp", + }, + }}, + Nonces: newStubNonces("n-cross"), + // AccountKID expects prof-corp; the test JWS uses a kid that + // claims prof-corp BUT we're going to feed an off-canonical + // kid that doesn't match. + AccountKID: testAccountKID, + } + // Sign with a kid that points at a different host. The verifier's + // AccountKID round-trip-check should reject it. + wrongKID := "https://different-host/acme/profile/prof-corp/account/acme-acc-test123" + body := signWithKID(t, key, jose.RS256, wrongKID, testURL, "n-cross", map[string]any{}) + _, err := VerifyJWS(cfg, []byte(body), testURL, VerifyOptions{}) + if err == nil { + t.Fatal("expected error from kid round-trip mismatch") + } + if !errors.Is(err, ErrJWSMalformed) { + t.Errorf("err = %v; want ErrJWSMalformed (round-trip mismatch)", err) + } +} + +// MapJWSErrorToProblem coverage check: every exported sentinel maps +// to a typed Problem (not the default malformed catch-all). +func TestMapJWSErrorToProblem_KnownSentinels(t *testing.T) { + cases := []struct { + err error + wantTyp string + }{ + {ErrJWSBadNonce, "urn:ietf:params:acme:error:badNonce"}, + {ErrJWSMissingNonce, "urn:ietf:params:acme:error:badNonce"}, + {ErrJWSAccountNotFound, "urn:ietf:params:acme:error:accountDoesNotExist"}, + {ErrJWSAccountInactive, "urn:ietf:params:acme:error:unauthorized"}, + {ErrJWSURLMismatch, "urn:ietf:params:acme:error:unauthorized"}, + {ErrJWSSignatureInvalid, "urn:ietf:params:acme:error:unauthorized"}, + {ErrJWSAlgorithmRejected, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSExpectJWKGotKid, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSExpectKidGotJWK, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSBothKidAndJWK, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSNeitherKidNorJWK, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSInvalidJWK, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSWrongType, "urn:ietf:params:acme:error:malformed"}, + {ErrJWSPayloadMismatch, "urn:ietf:params:acme:error:serverInternal"}, + {ErrJWSMalformed, "urn:ietf:params:acme:error:malformed"}, + } + for _, tc := range cases { + p := MapJWSErrorToProblem(tc.err) + if p.Type != tc.wantTyp { + t.Errorf("err=%v: type = %q, want %q", tc.err, p.Type, tc.wantTyp) + } + if p.Status == 0 { + t.Errorf("err=%v: status was 0", tc.err) + } + } +} diff --git a/internal/api/handler/acme.go b/internal/api/handler/acme.go index 26238da..899082f 100644 --- a/internal/api/handler/acme.go +++ b/internal/api/handler/acme.go @@ -7,21 +7,37 @@ import ( "context" "encoding/json" "errors" + "io" "net/http" + jose "github.com/go-jose/go-jose/v4" + "github.com/shankar0123/certctl/internal/api/acme" + "github.com/shankar0123/certctl/internal/domain" "github.com/shankar0123/certctl/internal/service" ) +// MaxJWSBodyBytes caps the per-request JWS payload at 64 KiB. RFC 8555 +// payloads are tiny (a JWK is < 1 KiB; a CSR < 4 KiB), so anything +// larger is either malformed or hostile. The router-level body-limit +// middleware already caps requests at the server-wide +// CERTCTL_MAX_BODY_SIZE (default 1 MiB), but ACME-specifically we +// tighten further. +const MaxJWSBodyBytes = 64 * 1024 + // ACMEService is the handler-facing surface for the ACME server. The // service-layer concrete type is *service.ACMEService; the interface // definition lives here to keep the handler import-direction -// canonical (handler imports service, not the reverse). Phase 1a -// pins two methods; Phase 1b extends with VerifyJWS, NewAccount, -// LookupAccount, UpdateAccount, DeactivateAccount. +// canonical (handler imports service, not the reverse). type ACMEService interface { BuildDirectory(ctx context.Context, profileID, baseURL string) (*acme.Directory, error) IssueNonce(ctx context.Context) (string, error) + // Phase 1b — JWS verification + account resource. + VerifyJWS(ctx context.Context, body []byte, requestURL string, expectNewAccount bool, accountKID func(accountID string) string) (*acme.VerifiedRequest, error) + NewAccount(ctx context.Context, profileID string, jwk *jose.JSONWebKey, contact []string, onlyReturnExisting bool, tosAgreed bool) (*domain.ACMEAccount, bool, error) + LookupAccount(ctx context.Context, accountID string) (*domain.ACMEAccount, error) + UpdateAccount(ctx context.Context, accountID string, contact []string) (*domain.ACMEAccount, error) + DeactivateAccount(ctx context.Context, accountID string) (*domain.ACMEAccount, error) } // ACMEHandler exposes the ACME server's RFC 8555 endpoints under the @@ -147,8 +163,8 @@ func (h ACMEHandler) directoryBaseURL(r *http.Request, profileID string) string // writeServiceError maps service-layer sentinels to RFC 7807 + RFC // 8555 §6.7 problem responses. Centralized so every handler method -// gets identical mapping; future Phase 1b/2/3/4 sentinels extend -// the switch. +// gets identical mapping; new sentinels extend the switch as later +// phases land. func writeServiceError(w http.ResponseWriter, err error) { switch { case errors.Is(err, service.ErrACMEUserActionRequired): @@ -161,6 +177,11 @@ func writeServiceError(w http.ResponseWriter, err error) { Detail: "profile not found", Status: http.StatusNotFound, }) + case errors.Is(err, service.ErrACMEAccountNotFound): + acme.WriteProblem(w, acme.AccountDoesNotExist("account not found")) + case errors.Is(err, service.ErrACMEAccountDoesNotExist): + acme.WriteProblem(w, acme.AccountDoesNotExist( + "no account exists for this JWK; submit a new-account request without onlyReturnExisting")) default: // Avoid leaking internal error text per master-prompt // criterion #10 (operator-actionable errors with no info @@ -168,3 +189,224 @@ func writeServiceError(w http.ResponseWriter, err error) { acme.WriteProblem(w, acme.ServerInternal("ACME server error")) } } + +// NewAccount handles POST /acme/profile/{id}/new-account (RFC 8555 +// §7.3). The request body is a JWS with `jwk` (NOT `kid`) in the +// protected header — the verifier enforces this via +// ExpectNewAccount=true. +// +// Behavior matrix: +// - JWK already registered + payload.OnlyReturnExisting=false → +// 200 + existing account row (idempotent re-registration per +// RFC 8555 §7.3.1). +// - JWK already registered + payload.OnlyReturnExisting=true → +// same 200 + existing row. +// - JWK new + OnlyReturnExisting=false → 201 + newly-created row. +// - JWK new + OnlyReturnExisting=true → 400 + accountDoesNotExist. +func (h ACMEHandler) NewAccount(w http.ResponseWriter, r *http.Request) { + profileID := r.PathValue("id") + requestURL := h.requestURL(r) + + body, err := io.ReadAll(io.LimitReader(r.Body, MaxJWSBodyBytes+1)) + if err != nil { + acme.WriteProblem(w, acme.Malformed("could not read request body")) + return + } + if len(body) > MaxJWSBodyBytes { + acme.WriteProblem(w, acme.Malformed("request body too large")) + return + } + + verified, err := h.svc.VerifyJWS(r.Context(), body, requestURL, true /*expectNewAccount*/, h.accountKID(r, profileID)) + if err != nil { + acme.WriteProblem(w, acme.MapJWSErrorToProblem(err)) + return + } + + var req acme.NewAccountRequest + if err := json.Unmarshal(verified.Payload, &req); err != nil { + acme.WriteProblem(w, acme.Malformed("could not parse new-account payload")) + return + } + + acct, isNew, err := h.svc.NewAccount( + r.Context(), profileID, verified.JWK, req.Contact, + req.OnlyReturnExisting, req.TermsOfServiceAgreed, + ) + if err != nil { + writeServiceError(w, err) + return + } + + if nonce, err := h.svc.IssueNonce(r.Context()); err == nil { + w.Header().Set("Replay-Nonce", nonce) + } + w.Header().Set("Location", h.accountKID(r, profileID)(acct.AccountID)) + w.Header().Set("Content-Type", "application/json") + if isNew { + w.WriteHeader(http.StatusCreated) + } else { + w.WriteHeader(http.StatusOK) + } + _ = json.NewEncoder(w).Encode( + acme.MarshalAccount(acct, h.accountOrdersURL(r, profileID, acct.AccountID)), + ) +} + +// Account handles POST /acme/profile/{id}/account/{acc-id} (RFC 8555 +// §7.3.2 + §7.3.6 + POST-as-GET per §6.3). The verifier requires +// `kid` (NOT `jwk`); the kid path-segment must match the URL +// path-segment. +// +// Payload variants: +// - empty body or empty JSON {}: POST-as-GET; returns the account. +// - {"contact": [...]}: contact update (RFC 8555 §7.3.2). +// - {"status": "deactivated"}: deactivation (RFC 8555 §7.3.6). +// +// Mixing contact + status in one request is permitted; we apply +// status first (deactivation is the more conservative action). +func (h ACMEHandler) Account(w http.ResponseWriter, r *http.Request) { + profileID := r.PathValue("id") + urlAccountID := r.PathValue("acc_id") + requestURL := h.requestURL(r) + + body, err := io.ReadAll(io.LimitReader(r.Body, MaxJWSBodyBytes+1)) + if err != nil { + acme.WriteProblem(w, acme.Malformed("could not read request body")) + return + } + if len(body) > MaxJWSBodyBytes { + acme.WriteProblem(w, acme.Malformed("request body too large")) + return + } + + verified, err := h.svc.VerifyJWS(r.Context(), body, requestURL, false /*expectNewAccount*/, h.accountKID(r, profileID)) + if err != nil { + acme.WriteProblem(w, acme.MapJWSErrorToProblem(err)) + return + } + + // kid path-segment must equal URL path-segment (defense in depth — + // the verifier already round-tripped the kid against the canonical + // URL). + if verified.Account == nil || verified.Account.AccountID != urlAccountID { + acme.WriteProblem(w, acme.Problem{ + Type: "urn:ietf:params:acme:error:unauthorized", + Detail: "kid does not match URL account id", + Status: http.StatusUnauthorized, + }) + return + } + + var ( + updated *domain.ACMEAccount + readOnly bool + ) + // Empty body or empty JSON object → POST-as-GET (§6.3). + trimmed := trimBody(verified.Payload) + if len(trimmed) == 0 || string(trimmed) == "{}" { + readOnly = true + updated = verified.Account + } else { + var req acme.AccountUpdateRequest + if err := json.Unmarshal(verified.Payload, &req); err != nil { + acme.WriteProblem(w, acme.Malformed("could not parse account update payload")) + return + } + // Status transition first (the more conservative action). + switch req.Status { + case "": + // no-op + case "deactivated": + acct, err := h.svc.DeactivateAccount(r.Context(), urlAccountID) + if err != nil { + writeServiceError(w, err) + return + } + updated = acct + default: + acme.WriteProblem(w, acme.Malformed( + "only `deactivated` is a valid status for account update; got "+req.Status)) + return + } + // Contact update. + if req.Contact != nil { + acct, err := h.svc.UpdateAccount(r.Context(), urlAccountID, req.Contact) + if err != nil { + writeServiceError(w, err) + return + } + updated = acct + } + if updated == nil { + // Empty status + nil contact → no-op; treat as POST-as-GET. + updated = verified.Account + readOnly = true + } + } + + if nonce, err := h.svc.IssueNonce(r.Context()); err == nil { + w.Header().Set("Replay-Nonce", nonce) + } + if readOnly { + w.Header().Set("Content-Type", "application/json") + } else { + w.Header().Set("Content-Type", "application/json") + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode( + acme.MarshalAccount(updated, h.accountOrdersURL(r, profileID, updated.AccountID)), + ) +} + +// requestURL composes the full URL the JWS protected-header `url` +// MUST equal. Equivalent to scheme://host + r.URL.Path. +func (h ACMEHandler) requestURL(r *http.Request) string { + scheme := "https" + if r.TLS == nil { + scheme = "http" + } + return scheme + "://" + r.Host + r.URL.Path +} + +// accountKID returns the closure VerifyJWS uses to round-trip-check +// inbound `kid` headers. Centralized so both NewAccount + Account +// build the same URL shape. +func (h ACMEHandler) accountKID(r *http.Request, profileID string) func(accountID string) string { + scheme := "https" + if r.TLS == nil { + scheme = "http" + } + prefix := scheme + "://" + r.Host + if profileID != "" { + prefix += "/acme/profile/" + profileID + } else { + prefix += "/acme" + } + return func(accountID string) string { return prefix + "/account/" + accountID } +} + +// accountOrdersURL is the URL Phase 2 will serve account orders at. +// Phase 1b emits it in the account JSON for RFC 8555 §7.1.2.1 +// compliance even though hitting it returns 404 until Phase 2. +func (h ACMEHandler) accountOrdersURL(r *http.Request, profileID, accountID string) string { + return h.accountKID(r, profileID)(accountID) + "/orders" +} + +// trimBody is a minimal JSON-aware trim that returns a copy with +// outer whitespace removed. We don't need full JSON parsing here — +// just enough to detect empty body / empty object for POST-as-GET +// routing. +func trimBody(b []byte) []byte { + for len(b) > 0 && (b[0] == ' ' || b[0] == '\t' || b[0] == '\n' || b[0] == '\r') { + b = b[1:] + } + for len(b) > 0 { + c := b[len(b)-1] + if c != ' ' && c != '\t' && c != '\n' && c != '\r' { + break + } + b = b[:len(b)-1] + } + return b +} diff --git a/internal/api/handler/acme_handler_test.go b/internal/api/handler/acme_handler_test.go index 67e47e9..7374f0a 100644 --- a/internal/api/handler/acme_handler_test.go +++ b/internal/api/handler/acme_handler_test.go @@ -4,6 +4,7 @@ package handler import ( + "bytes" "context" "encoding/json" "errors" @@ -12,7 +13,10 @@ import ( "strings" "testing" + jose "github.com/go-jose/go-jose/v4" + "github.com/shankar0123/certctl/internal/api/acme" + "github.com/shankar0123/certctl/internal/domain" "github.com/shankar0123/certctl/internal/service" ) @@ -20,8 +24,13 @@ import ( // Mirrors the mockSCEPService pattern at scep_handler_test.go (struct // holding canned responses + an err field per method). type mockACMEService struct { - BuildDirectoryFn func(ctx context.Context, profileID, baseURL string) (*acme.Directory, error) - IssueNonceFn func(ctx context.Context) (string, error) + BuildDirectoryFn func(ctx context.Context, profileID, baseURL string) (*acme.Directory, error) + IssueNonceFn func(ctx context.Context) (string, error) + VerifyJWSFn func(ctx context.Context, body []byte, requestURL string, expectNewAccount bool, accountKID func(string) string) (*acme.VerifiedRequest, error) + NewAccountFn func(ctx context.Context, profileID string, jwk *jose.JSONWebKey, contact []string, onlyReturnExisting bool, tosAgreed bool) (*domain.ACMEAccount, bool, error) + LookupAccountFn func(ctx context.Context, accountID string) (*domain.ACMEAccount, error) + UpdateAccountFn func(ctx context.Context, accountID string, contact []string) (*domain.ACMEAccount, error) + DeactivateAccountFn func(ctx context.Context, accountID string) (*domain.ACMEAccount, error) } func (m *mockACMEService) BuildDirectory(ctx context.Context, profileID, baseURL string) (*acme.Directory, error) { @@ -38,6 +47,41 @@ func (m *mockACMEService) IssueNonce(ctx context.Context) (string, error) { return "test-nonce-12345", nil } +func (m *mockACMEService) VerifyJWS(ctx context.Context, body []byte, requestURL string, expectNewAccount bool, accountKID func(string) string) (*acme.VerifiedRequest, error) { + if m.VerifyJWSFn != nil { + return m.VerifyJWSFn(ctx, body, requestURL, expectNewAccount, accountKID) + } + return nil, errors.New("VerifyJWS not stubbed") +} + +func (m *mockACMEService) NewAccount(ctx context.Context, profileID string, jwk *jose.JSONWebKey, contact []string, onlyReturnExisting bool, tosAgreed bool) (*domain.ACMEAccount, bool, error) { + if m.NewAccountFn != nil { + return m.NewAccountFn(ctx, profileID, jwk, contact, onlyReturnExisting, tosAgreed) + } + return nil, false, errors.New("NewAccount not stubbed") +} + +func (m *mockACMEService) LookupAccount(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + if m.LookupAccountFn != nil { + return m.LookupAccountFn(ctx, accountID) + } + return nil, errors.New("LookupAccount not stubbed") +} + +func (m *mockACMEService) UpdateAccount(ctx context.Context, accountID string, contact []string) (*domain.ACMEAccount, error) { + if m.UpdateAccountFn != nil { + return m.UpdateAccountFn(ctx, accountID, contact) + } + return nil, errors.New("UpdateAccount not stubbed") +} + +func (m *mockACMEService) DeactivateAccount(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + if m.DeactivateAccountFn != nil { + return m.DeactivateAccountFn(ctx, accountID) + } + return nil, errors.New("DeactivateAccount not stubbed") +} + // newACMETestServer wires the ACMEHandler against the mock + a stdlib // ServeMux configured exactly the way internal/api/router/router.go // does it in production. Routes: @@ -55,9 +99,13 @@ func newACMETestServer(t *testing.T, mock *mockACMEService) *httptest.Server { mux.HandleFunc("GET /acme/profile/{id}/directory", h.Directory) mux.HandleFunc("HEAD /acme/profile/{id}/new-nonce", h.NewNonce) mux.HandleFunc("GET /acme/profile/{id}/new-nonce", h.NewNonce) + mux.HandleFunc("POST /acme/profile/{id}/new-account", h.NewAccount) + mux.HandleFunc("POST /acme/profile/{id}/account/{acc_id}", h.Account) mux.HandleFunc("GET /acme/directory", h.Directory) mux.HandleFunc("HEAD /acme/new-nonce", h.NewNonce) mux.HandleFunc("GET /acme/new-nonce", h.NewNonce) + mux.HandleFunc("POST /acme/new-account", h.NewAccount) + mux.HandleFunc("POST /acme/account/{acc_id}", h.Account) return httptest.NewServer(mux) } @@ -240,3 +288,254 @@ func TestACMEHandler_NewNonce_ServiceError(t *testing.T) { t.Errorf("content-type = %q", got) } } + +// --- Phase 1b — new-account + account update --------------------------- + +// stubVerifiedReq returns a VerifiedRequest pre-baked with payload + +// the supplied Account / JWK for handler-level tests that don't go +// through the actual JWS verifier. +func stubVerifiedReq(payload interface{}, account *domain.ACMEAccount, jwk *jose.JSONWebKey) func(ctx context.Context, body []byte, requestURL string, expectNewAccount bool, accountKID func(string) string) (*acme.VerifiedRequest, error) { + return func(ctx context.Context, body []byte, requestURL string, expectNewAccount bool, accountKID func(string) string) (*acme.VerifiedRequest, error) { + raw, _ := json.Marshal(payload) + return &acme.VerifiedRequest{ + Payload: raw, + Algorithm: "RS256", + URL: requestURL, + Nonce: "test-nonce", + Account: account, + JWK: jwk, + }, nil + } +} + +func TestACMEHandler_NewAccount_HappyPath_New(t *testing.T) { + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq( + acme.NewAccountRequest{Contact: []string{"mailto:a@example.com"}, TermsOfServiceAgreed: true}, + nil, // jwk path → no Account + &jose.JSONWebKey{}, + ), + NewAccountFn: func(ctx context.Context, profileID string, jwk *jose.JSONWebKey, contact []string, onlyReturnExisting bool, tosAgreed bool) (*domain.ACMEAccount, bool, error) { + return &domain.ACMEAccount{ + AccountID: "acme-acc-fresh", JWKThumbprint: "thumb-x", + Contact: contact, Status: domain.ACMEAccountStatusValid, ProfileID: profileID, + }, true, nil + }, + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/new-account", "application/jose+json", bytes.NewReader([]byte("ignored-by-mock"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + t.Errorf("status = %d, want 201", resp.StatusCode) + } + if got := resp.Header.Get("Replay-Nonce"); got == "" { + t.Error("Replay-Nonce header missing") + } + if got := resp.Header.Get("Location"); !strings.Contains(got, "/account/acme-acc-fresh") { + t.Errorf("Location = %q (want suffix /account/acme-acc-fresh)", got) + } + var body acme.AccountResponseJSON + if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { + t.Fatalf("decode: %v", err) + } + if body.Status != "valid" { + t.Errorf("status = %q", body.Status) + } + if !strings.HasSuffix(body.Orders, "/account/acme-acc-fresh/orders") { + t.Errorf("orders URL = %q", body.Orders) + } +} + +func TestACMEHandler_NewAccount_Idempotent_ExistingReturns200(t *testing.T) { + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq(acme.NewAccountRequest{}, nil, &jose.JSONWebKey{}), + NewAccountFn: func(ctx context.Context, profileID string, jwk *jose.JSONWebKey, contact []string, onlyReturnExisting bool, tosAgreed bool) (*domain.ACMEAccount, bool, error) { + return &domain.ACMEAccount{ + AccountID: "acme-acc-existing", Status: domain.ACMEAccountStatusValid, ProfileID: profileID, + }, false /*isNew=false*/, nil + }, + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/new-account", "application/jose+json", bytes.NewReader([]byte("x"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200 (idempotent re-registration)", resp.StatusCode) + } +} + +func TestACMEHandler_NewAccount_OnlyReturnExisting_NoMatch(t *testing.T) { + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq(acme.NewAccountRequest{OnlyReturnExisting: true}, nil, &jose.JSONWebKey{}), + NewAccountFn: func(ctx context.Context, profileID string, jwk *jose.JSONWebKey, contact []string, onlyReturnExisting bool, tosAgreed bool) (*domain.ACMEAccount, bool, error) { + return nil, false, service.ErrACMEAccountDoesNotExist + }, + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/new-account", "application/jose+json", bytes.NewReader([]byte("x"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("status = %d, want 400", resp.StatusCode) + } + var p acme.Problem + _ = json.NewDecoder(resp.Body).Decode(&p) + if p.Type != "urn:ietf:params:acme:error:accountDoesNotExist" { + t.Errorf("Problem.Type = %q", p.Type) + } +} + +func TestACMEHandler_NewAccount_JWSMalformed(t *testing.T) { + mock := &mockACMEService{ + VerifyJWSFn: func(ctx context.Context, body []byte, requestURL string, expectNewAccount bool, accountKID func(string) string) (*acme.VerifiedRequest, error) { + return nil, acme.ErrJWSMalformed + }, + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/new-account", "application/jose+json", bytes.NewReader([]byte("garbage"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("status = %d, want 400", resp.StatusCode) + } + var p acme.Problem + _ = json.NewDecoder(resp.Body).Decode(&p) + if p.Type != "urn:ietf:params:acme:error:malformed" { + t.Errorf("Problem.Type = %q", p.Type) + } +} + +func TestACMEHandler_Account_KIDMismatch(t *testing.T) { + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq( + acme.AccountUpdateRequest{}, + &domain.ACMEAccount{ + AccountID: "acme-acc-A", Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp", + }, + nil, + ), + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + // URL claims account B, JWS-verified account is A. + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/account/acme-acc-B", "application/jose+json", bytes.NewReader([]byte("x"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", resp.StatusCode) + } +} + +func TestACMEHandler_Account_Deactivate(t *testing.T) { + called := false + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq( + acme.AccountUpdateRequest{Status: "deactivated"}, + &domain.ACMEAccount{AccountID: "acme-acc-D", Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp"}, + nil, + ), + DeactivateAccountFn: func(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + called = true + return &domain.ACMEAccount{AccountID: accountID, Status: domain.ACMEAccountStatusDeactivated, ProfileID: "prof-corp"}, nil + }, + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/account/acme-acc-D", "application/jose+json", bytes.NewReader([]byte("x"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + if !called { + t.Error("DeactivateAccount was not invoked") + } + var body acme.AccountResponseJSON + _ = json.NewDecoder(resp.Body).Decode(&body) + if body.Status != "deactivated" { + t.Errorf("status = %q", body.Status) + } +} + +func TestACMEHandler_Account_UpdateContact(t *testing.T) { + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq( + acme.AccountUpdateRequest{Contact: []string{"mailto:new@example.com"}}, + &domain.ACMEAccount{AccountID: "acme-acc-U", Status: domain.ACMEAccountStatusValid, ProfileID: "prof-corp"}, + nil, + ), + UpdateAccountFn: func(ctx context.Context, accountID string, contact []string) (*domain.ACMEAccount, error) { + return &domain.ACMEAccount{AccountID: accountID, Status: domain.ACMEAccountStatusValid, Contact: contact, ProfileID: "prof-corp"}, nil + }, + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/account/acme-acc-U", "application/jose+json", bytes.NewReader([]byte("x"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + var body acme.AccountResponseJSON + _ = json.NewDecoder(resp.Body).Decode(&body) + if len(body.Contact) != 1 || body.Contact[0] != "mailto:new@example.com" { + t.Errorf("contact = %v", body.Contact) + } +} + +func TestACMEHandler_Account_PostAsGet(t *testing.T) { + // Empty payload → POST-as-GET (RFC 8555 §6.3): handler returns + // the unmodified account row. + mock := &mockACMEService{ + VerifyJWSFn: stubVerifiedReq( + struct{}{}, // empty payload + &domain.ACMEAccount{AccountID: "acme-acc-G", Status: domain.ACMEAccountStatusValid, Contact: []string{"mailto:o@example.com"}, ProfileID: "prof-corp"}, + nil, + ), + } + srv := newACMETestServer(t, mock) + defer srv.Close() + + resp, err := http.Post(srv.URL+"/acme/profile/prof-corp/account/acme-acc-G", "application/jose+json", bytes.NewReader([]byte("x"))) + if err != nil { + t.Fatalf("Post: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want 200 (POST-as-GET)", resp.StatusCode) + } +} diff --git a/internal/api/router/openapi_parity_test.go b/internal/api/router/openapi_parity_test.go index 974685c..a0ca6df 100644 --- a/internal/api/router/openapi_parity_test.go +++ b/internal/api/router/openapi_parity_test.go @@ -61,12 +61,16 @@ var SpecParityExceptions = map[string]string{ // new-order, finalize, authz, challenge, cert, key-change, // revoke-cert, renewal-info — each gets its own exception entry // in the same commit that lands the route. - "GET /acme/profile/{id}/directory": "RFC 8555 §7.1.1 directory; documented in docs/acme-server.md", - "HEAD /acme/profile/{id}/new-nonce": "RFC 8555 §7.2 new-nonce; documented in docs/acme-server.md", - "GET /acme/profile/{id}/new-nonce": "RFC 8555 §7.2 new-nonce (GET form); documented in docs/acme-server.md", - "GET /acme/directory": "RFC 8555 §7.1.1 directory (default-profile shorthand); documented in docs/acme-server.md", - "HEAD /acme/new-nonce": "RFC 8555 §7.2 new-nonce (default-profile shorthand); documented in docs/acme-server.md", - "GET /acme/new-nonce": "RFC 8555 §7.2 new-nonce GET (default-profile shorthand); documented in docs/acme-server.md", + "GET /acme/profile/{id}/directory": "RFC 8555 §7.1.1 directory; documented in docs/acme-server.md", + "HEAD /acme/profile/{id}/new-nonce": "RFC 8555 §7.2 new-nonce; documented in docs/acme-server.md", + "GET /acme/profile/{id}/new-nonce": "RFC 8555 §7.2 new-nonce (GET form); documented in docs/acme-server.md", + "POST /acme/profile/{id}/new-account": "RFC 8555 §7.3 new-account; documented in docs/acme-server.md", + "POST /acme/profile/{id}/account/{acc_id}": "RFC 8555 §7.3.2 account update + §7.3.6 deactivation; documented in docs/acme-server.md", + "GET /acme/directory": "RFC 8555 §7.1.1 directory (default-profile shorthand); documented in docs/acme-server.md", + "HEAD /acme/new-nonce": "RFC 8555 §7.2 new-nonce (default-profile shorthand); documented in docs/acme-server.md", + "GET /acme/new-nonce": "RFC 8555 §7.2 new-nonce GET (default-profile shorthand); documented in docs/acme-server.md", + "POST /acme/new-account": "RFC 8555 §7.3 new-account (default-profile shorthand); documented in docs/acme-server.md", + "POST /acme/account/{acc_id}": "RFC 8555 §7.3.2 + §7.3.6 (default-profile shorthand); documented in docs/acme-server.md", } func TestRouter_OpenAPIParity(t *testing.T) { diff --git a/internal/api/router/router.go b/internal/api/router/router.go index efc9318..10edcee 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -415,6 +415,8 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { r.Register("GET /acme/profile/{id}/directory", http.HandlerFunc(reg.ACME.Directory)) r.Register("HEAD /acme/profile/{id}/new-nonce", http.HandlerFunc(reg.ACME.NewNonce)) r.Register("GET /acme/profile/{id}/new-nonce", http.HandlerFunc(reg.ACME.NewNonce)) + r.Register("POST /acme/profile/{id}/new-account", http.HandlerFunc(reg.ACME.NewAccount)) + r.Register("POST /acme/profile/{id}/account/{acc_id}", http.HandlerFunc(reg.ACME.Account)) // Default-profile shorthand. The handler's profile-resolution path // returns userActionRequired (RFC 7807 + RFC 8555 §6.7) when // CERTCTL_ACME_SERVER_DEFAULT_PROFILE_ID is unset; when set it @@ -422,6 +424,8 @@ func (r *Router) RegisterHandlers(reg HandlerRegistry) { r.Register("GET /acme/directory", http.HandlerFunc(reg.ACME.Directory)) r.Register("HEAD /acme/new-nonce", http.HandlerFunc(reg.ACME.NewNonce)) r.Register("GET /acme/new-nonce", http.HandlerFunc(reg.ACME.NewNonce)) + r.Register("POST /acme/new-account", http.HandlerFunc(reg.ACME.NewAccount)) + r.Register("POST /acme/account/{acc_id}", http.HandlerFunc(reg.ACME.Account)) } // RegisterESTHandlers sets up EST (RFC 7030) routes under diff --git a/internal/domain/acme.go b/internal/domain/acme.go new file mode 100644 index 0000000..f0c4718 --- /dev/null +++ b/internal/domain/acme.go @@ -0,0 +1,50 @@ +// Copyright (c) certctl +// SPDX-License-Identifier: BSL-1.1 + +package domain + +import "time" + +// ACMEAccount mirrors a row in the acme_accounts table (RFC 8555 §7.1.2). +// The (ProfileID, JWKThumbprint) pair is unique per the migration's +// UNIQUE constraint — RFC 8555 §7.3.1 idempotent semantics — so the +// new-account endpoint maps a re-registration of an existing key onto +// the original account row rather than creating a duplicate. +// +// JWKPEM is the public-only JWK serialized via api/acme.JWKToPEM (a +// PEM-wrapped JSON envelope). Stored as TEXT in the column for diff- +// friendliness; the verifier round-trips through ParseJWKFromPEM at +// request time. +type ACMEAccount struct { + AccountID string `json:"account_id"` + JWKThumbprint string `json:"jwk_thumbprint"` + JWKPEM string `json:"jwk_pem"` + Contact []string `json:"contact,omitempty"` + Status ACMEAccountStatus `json:"status"` + ProfileID string `json:"profile_id"` + OwnerID string `json:"owner_id,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// ACMEAccountStatus is the closed enum for acme_accounts.status. The +// migration's CHECK constraint is implicit (the migration uses TEXT +// without a CHECK; service-layer validation owns the value-set). +type ACMEAccountStatus string + +const ( + // ACMEAccountStatusValid is the default for newly-created accounts. + // JWS-authenticated requests are accepted only when the bound + // account is `valid`. + ACMEAccountStatusValid ACMEAccountStatus = "valid" + // ACMEAccountStatusDeactivated marks an account the client + // voluntarily retired via POST /acme/.../account/ with + // payload {"status": "deactivated"} (RFC 8555 §7.3.6). Future + // JWS-authenticated requests using this account's kid are + // rejected with `unauthorized`. + ACMEAccountStatusDeactivated ACMEAccountStatus = "deactivated" + // ACMEAccountStatusRevoked marks an account the operator + // administratively retired (e.g. after detecting a compromised + // JWK). Same access semantics as deactivated. + ACMEAccountStatusRevoked ACMEAccountStatus = "revoked" +) diff --git a/internal/repository/postgres/acme.go b/internal/repository/postgres/acme.go index 24e8bb1..b80b165 100644 --- a/internal/repository/postgres/acme.go +++ b/internal/repository/postgres/acme.go @@ -6,8 +6,13 @@ package postgres import ( "context" "database/sql" + "errors" "fmt" "time" + + "github.com/lib/pq" + "github.com/shankar0123/certctl/internal/domain" + "github.com/shankar0123/certctl/internal/repository" ) // ACMERepository implements the ACME server's persistence layer @@ -84,3 +89,170 @@ func (r *ACMERepository) ConsumeNonce(ctx context.Context, nonce string) error { } return nil } + +// ErrACMEAccountDuplicateThumbprint is the sentinel returned by +// CreateAccount[WithTx] when the (profile_id, jwk_thumbprint) UNIQUE +// constraint fires. Callers (the new-account flow) translate this +// into "an account already exists for this JWK" — RFC 8555 §7.3.1 +// idempotent-semantics path. +var ErrACMEAccountDuplicateThumbprint = errors.New("acme: account already exists for this profile + JWK thumbprint") + +// CreateAccount inserts a new acme_accounts row. Use CreateAccountWithTx +// when the insert must be atomic with an audit row. +func (r *ACMERepository) CreateAccount(ctx context.Context, acct *domain.ACMEAccount) error { + return r.CreateAccountWithTx(ctx, r.db, acct) +} + +// CreateAccountWithTx inserts using the supplied Querier (typically +// *sql.Tx from postgres.WithinTx). Returns +// ErrACMEAccountDuplicateThumbprint on the (profile_id, jwk_thumbprint) +// UNIQUE collision per migration 000025. +func (r *ACMERepository) CreateAccountWithTx(ctx context.Context, q repository.Querier, acct *domain.ACMEAccount) error { + if acct.AccountID == "" || acct.JWKThumbprint == "" || acct.JWKPEM == "" || acct.ProfileID == "" { + return fmt.Errorf("acme: create account: missing required field") + } + if acct.Status == "" { + acct.Status = domain.ACMEAccountStatusValid + } + now := time.Now().UTC() + if acct.CreatedAt.IsZero() { + acct.CreatedAt = now + } + acct.UpdatedAt = now + + contact := pq.Array(acct.Contact) + var ownerID interface{} + if acct.OwnerID != "" { + ownerID = acct.OwnerID + } + _, err := q.ExecContext(ctx, ` + INSERT INTO acme_accounts ( + account_id, jwk_thumbprint, jwk_pem, contact, status, + profile_id, owner_id, created_at, updated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + `, + acct.AccountID, acct.JWKThumbprint, acct.JWKPEM, contact, + string(acct.Status), acct.ProfileID, ownerID, + acct.CreatedAt, acct.UpdatedAt, + ) + if err != nil { + // Postgres SQLSTATE 23505 = unique_violation. lib/pq wraps the + // raw error in *pq.Error; decode and translate to the + // repository sentinel. + var pqErr *pq.Error + if errors.As(err, &pqErr) && pqErr.Code == "23505" { + return ErrACMEAccountDuplicateThumbprint + } + return fmt.Errorf("acme: insert account: %w", err) + } + return nil +} + +// GetAccountByID returns the account row for an account ID. +// Returns sql.ErrNoRows wrapped via repository.ErrNotFound when +// no row matches (callers branch on errors.Is(err, repository.ErrNotFound)). +func (r *ACMERepository) GetAccountByID(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + row := r.db.QueryRowContext(ctx, ` + SELECT account_id, jwk_thumbprint, jwk_pem, contact, status, + profile_id, COALESCE(owner_id, ''), created_at, updated_at + FROM acme_accounts + WHERE account_id = $1 + `, accountID) + return scanACMEAccount(row) +} + +// GetAccountByThumbprint returns the account row for a (profile_id, +// jwk_thumbprint) pair. Same sentinel semantics as GetAccountByID. +// The new-account idempotency path queries by thumbprint to detect a +// re-registration of an existing JWK (RFC 8555 §7.3.1). +func (r *ACMERepository) GetAccountByThumbprint(ctx context.Context, profileID, thumbprint string) (*domain.ACMEAccount, error) { + row := r.db.QueryRowContext(ctx, ` + SELECT account_id, jwk_thumbprint, jwk_pem, contact, status, + profile_id, COALESCE(owner_id, ''), created_at, updated_at + FROM acme_accounts + WHERE profile_id = $1 AND jwk_thumbprint = $2 + `, profileID, thumbprint) + return scanACMEAccount(row) +} + +// UpdateAccountContact replaces the account's contact list. Use the +// WithTx variant when the update must be atomic with an audit row. +func (r *ACMERepository) UpdateAccountContact(ctx context.Context, accountID string, contact []string) error { + return r.UpdateAccountContactWithTx(ctx, r.db, accountID, contact) +} + +// UpdateAccountContactWithTx writes the new contact list using the +// supplied Querier. Returns sql.ErrNoRows-wrapped repository.ErrNotFound +// on missing account. +func (r *ACMERepository) UpdateAccountContactWithTx(ctx context.Context, q repository.Querier, accountID string, contact []string) error { + res, err := q.ExecContext(ctx, ` + UPDATE acme_accounts + SET contact = $2, updated_at = NOW() + WHERE account_id = $1 + `, accountID, pq.Array(contact)) + if err != nil { + return fmt.Errorf("acme: update account contact: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("acme: update account contact rows affected: %w", err) + } + if n == 0 { + return fmt.Errorf("account not found: %w", repository.ErrNotFound) + } + return nil +} + +// UpdateAccountStatus is the persistence path for account +// deactivation. Phase 1b accepts only the "valid" → "deactivated" +// transition (RFC 8555 §7.3.6); operator-initiated revocation is a +// future phase. +func (r *ACMERepository) UpdateAccountStatus(ctx context.Context, accountID string, status domain.ACMEAccountStatus) error { + return r.UpdateAccountStatusWithTx(ctx, r.db, accountID, status) +} + +// UpdateAccountStatusWithTx writes the status transition using the +// supplied Querier. Same sentinel semantics as UpdateAccountContactWithTx. +func (r *ACMERepository) UpdateAccountStatusWithTx(ctx context.Context, q repository.Querier, accountID string, status domain.ACMEAccountStatus) error { + res, err := q.ExecContext(ctx, ` + UPDATE acme_accounts + SET status = $2, updated_at = NOW() + WHERE account_id = $1 + `, accountID, string(status)) + if err != nil { + return fmt.Errorf("acme: update account status: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("acme: update account status rows affected: %w", err) + } + if n == 0 { + return fmt.Errorf("account not found: %w", repository.ErrNotFound) + } + return nil +} + +// scanACMEAccount is the shared shape for the SELECT-by-X account +// queries above. Returns sql.ErrNoRows-wrapped repository.ErrNotFound +// on miss; any other scan failure surfaces verbatim. +func scanACMEAccount(row interface{ Scan(...interface{}) error }) (*domain.ACMEAccount, error) { + var ( + acct domain.ACMEAccount + contact pq.StringArray + statusStr string + ) + err := row.Scan( + &acct.AccountID, &acct.JWKThumbprint, &acct.JWKPEM, &contact, + &statusStr, &acct.ProfileID, &acct.OwnerID, + &acct.CreatedAt, &acct.UpdatedAt, + ) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("account not found: %w", repository.ErrNotFound) + } + return nil, fmt.Errorf("acme: scan account: %w", err) + } + acct.Contact = []string(contact) + acct.Status = domain.ACMEAccountStatus(statusStr) + return &acct, nil +} diff --git a/internal/service/acme.go b/internal/service/acme.go index e7f7f07..3f0933f 100644 --- a/internal/service/acme.go +++ b/internal/service/acme.go @@ -10,6 +10,8 @@ import ( "sync/atomic" "time" + jose "github.com/go-jose/go-jose/v4" + "github.com/shankar0123/certctl/internal/api/acme" "github.com/shankar0123/certctl/internal/config" "github.com/shankar0123/certctl/internal/domain" @@ -17,20 +19,25 @@ import ( ) // ACMERepo is the persistence-layer surface ACMEService consumes for -// nonce + (later phases) account / order / authz / challenge state. -// Phase 1a wires only the nonce path; the interface is tightened in -// Phase 1b along with the AccountService. +// nonce + account state. Phase 1b extends the Phase 1a interface with +// the account CRUD path; Phases 2-4 will further extend with order / +// authz / challenge state. // // Defining the interface in the service package (rather than // internal/repository/interfaces.go) keeps the cross-phase blast -// radius small: when Phase 1b adds CreateAccountWithTx / -// GetAccountByThumbprint / etc., only this file's interface and the -// concrete postgres ACMERepository move together. Mock implementations -// in tests satisfy this interface without depending on the postgres -// package. +// radius small: only this file and the concrete postgres +// ACMERepository move together. Mock implementations in tests satisfy +// this interface without depending on the postgres package. type ACMERepo interface { + // Phase 1a — nonce. IssueNonce(ctx context.Context, nonce string, ttl time.Duration) error ConsumeNonce(ctx context.Context, nonce string) error + // Phase 1b — account CRUD. + CreateAccountWithTx(ctx context.Context, q repository.Querier, acct *domain.ACMEAccount) error + GetAccountByID(ctx context.Context, accountID string) (*domain.ACMEAccount, error) + GetAccountByThumbprint(ctx context.Context, profileID, thumbprint string) (*domain.ACMEAccount, error) + UpdateAccountContactWithTx(ctx context.Context, q repository.Querier, accountID string, contact []string) error + UpdateAccountStatusWithTx(ctx context.Context, q repository.Querier, accountID string, status domain.ACMEAccountStatus) error } // profileLookup is the minimum surface ACMEService needs to resolve a @@ -41,31 +48,37 @@ type profileLookup interface { Get(ctx context.Context, id string) (*domain.CertificateProfile, error) } -// ACMEService orchestrates the ACME server's RFC 8555 surface. Phase 1a -// implements: +// ACMEService orchestrates the ACME server's RFC 8555 surface. // -// - BuildDirectory: returns the per-profile directory document. -// - IssueNonce: returns a Replay-Nonce, persisted with TTL. -// -// Phase 1b will extend with VerifyJWS, NewAccount, LookupAccount, -// UpdateAccount, DeactivateAccount. +// - Phase 1a (live): BuildDirectory, IssueNonce. +// - Phase 1b (this commit): VerifyJWS, NewAccount, LookupAccount, +// UpdateAccount, DeactivateAccount. +// - Subsequent phases extend with new-order, finalize, challenges, +// key-change, revoke, ARI. // // The struct deliberately holds raw config rather than per-field -// extracted values — the directory builder uses 4 of the 11 fields -// and reading them lazily keeps the constructor signature tight. +// extracted values — readers use 4 of the 11 fields and reading them +// lazily keeps the constructor signature tight. type ACMEService struct { repo ACMERepo profiles profileLookup cfg config.ACMEServerConfig metrics *ACMEMetrics + + // Phase 1b — atomic-audit plumbing for the JWS-authenticated + // POST surface. Both fields are set via SetTransactor + + // SetAuditService (mirrors CertificateService.SetTransactor at + // internal/service/certificate.go:254). When both are nil the + // service falls back to the non-transactional path — kept for + // the legacy directory + new-nonce paths that don't write to + // stateful tables. + tx repository.Transactor + auditService *AuditService } -// NewACMEService constructs an ACMEService. The constructor matches -// certctl's per-service convention: required dependencies in the -// argument list (repo, profile lookup, config), optional wiring via -// post-construction setters (metrics is wired now to keep the -// Phase-1a-only footprint clean; Phase 1b adds SetTransactor + -// SetAuditService for the JWS-authenticated POST path). +// NewACMEService constructs an ACMEService with the directory + nonce +// surface wired. Account-creating endpoints additionally need the +// transactor + audit service — see SetTransactor / SetAuditService. func NewACMEService(repo ACMERepo, profiles profileLookup, cfg config.ACMEServerConfig) *ACMEService { return &ACMEService{ repo: repo, @@ -75,6 +88,17 @@ func NewACMEService(repo ACMERepo, profiles profileLookup, cfg config.ACMEServer } } +// SetTransactor wires the atomic-audit transactor. Mirrors +// CertificateService.SetTransactor; cmd/server/main.go calls this +// at startup with the same *postgres.transactor instance shared +// across CertificateService / RevocationSvc / RenewalService. +func (s *ACMEService) SetTransactor(tx repository.Transactor) { s.tx = tx } + +// SetAuditService wires the audit service. cmd/server/main.go +// constructs auditService once and passes the same instance into +// every service that emits audit rows. +func (s *ACMEService) SetAuditService(a *AuditService) { s.auditService = a } + // Metrics returns the per-op counter snapshotter. cmd/server/main.go // passes this into MetricsHandler so the Prometheus exposer picks up // the per-op signals. @@ -92,6 +116,17 @@ var ErrACMEUserActionRequired = errors.New("acme: default profile not configured // says "something is wrong server-side"). var ErrACMEProfileNotFound = errors.New("acme: profile not found") +// ErrACMEAccountNotFound is returned by LookupAccount when the +// account ID in the URL doesn't match any row. Handler maps to +// 404 + RFC 8555 §6.7 accountDoesNotExist. +var ErrACMEAccountNotFound = errors.New("acme: account not found") + +// ErrACMEAccountDoesNotExist is returned by NewAccount when +// onlyReturnExisting=true and no account exists for the supplied +// JWK. RFC 8555 §7.3.1 requires returning 400 + +// urn:ietf:params:acme:error:accountDoesNotExist (NOT 404). +var ErrACMEAccountDoesNotExist = errors.New("acme: account does not exist for this JWK") + // BuildDirectory constructs the per-profile directory document. // // profileID resolution: @@ -176,14 +211,22 @@ func (s *ACMEService) resolveProfile(ctx context.Context, profileID string) (str // ACMEMetrics is the per-op counter table for the ACME server. Mirrors // the IssuanceMetrics / DeployCounters pattern (atomic.Uint64 + a -// Snapshot method that emits stable tuples). Phase 1a tracks just -// directory + new-nonce; subsequent phases add new-account / new-order -// / etc. +// Snapshot method that emits stable tuples). Phases 2-4 will extend +// with new-order / finalize / challenge counters. type ACMEMetrics struct { + // Phase 1a — directory + new-nonce. DirectoryTotal atomic.Uint64 DirectoryFailureTotal atomic.Uint64 NewNonceTotal atomic.Uint64 NewNonceFailureTotal atomic.Uint64 + + // Phase 1b — account resource. + NewAccountTotal atomic.Uint64 + NewAccountFailureTotal atomic.Uint64 + NewAccountIdempotentTotal atomic.Uint64 // re-registration of existing JWK (RFC 8555 §7.3.1) + UpdateAccountTotal atomic.Uint64 + UpdateAccountFailureTotal atomic.Uint64 + DeactivateAccountTotal atomic.Uint64 } // NewACMEMetrics returns a zeroed counter table. Concurrent callers @@ -192,7 +235,7 @@ type ACMEMetrics struct { func NewACMEMetrics() *ACMEMetrics { return &ACMEMetrics{} } // bump increments a single atomic counter. Centralized so the call -// sites in BuildDirectory + IssueNonce are uniform. +// sites in BuildDirectory + IssueNonce + NewAccount + etc. are uniform. func (m *ACMEMetrics) bump(c *atomic.Uint64) { c.Add(1) } // Snapshot emits the current counter values as a map (op → count). @@ -201,9 +244,263 @@ func (m *ACMEMetrics) bump(c *atomic.Uint64) { c.Add(1) } // directly without per-op stringly-typed branching. func (m *ACMEMetrics) Snapshot() map[string]uint64 { return map[string]uint64{ - "certctl_acme_directory_total": m.DirectoryTotal.Load(), - "certctl_acme_directory_failures_total": m.DirectoryFailureTotal.Load(), - "certctl_acme_new_nonce_total": m.NewNonceTotal.Load(), - "certctl_acme_new_nonce_failures_total": m.NewNonceFailureTotal.Load(), + "certctl_acme_directory_total": m.DirectoryTotal.Load(), + "certctl_acme_directory_failures_total": m.DirectoryFailureTotal.Load(), + "certctl_acme_new_nonce_total": m.NewNonceTotal.Load(), + "certctl_acme_new_nonce_failures_total": m.NewNonceFailureTotal.Load(), + "certctl_acme_new_account_total": m.NewAccountTotal.Load(), + "certctl_acme_new_account_failures_total": m.NewAccountFailureTotal.Load(), + "certctl_acme_new_account_idempotent_total": m.NewAccountIdempotentTotal.Load(), + "certctl_acme_update_account_total": m.UpdateAccountTotal.Load(), + "certctl_acme_update_account_failures_total": m.UpdateAccountFailureTotal.Load(), + "certctl_acme_deactivate_account_total": m.DeactivateAccountTotal.Load(), } } + +// VerifyJWS adapts the api/acme verifier to the service-layer +// dependency surface. It builds the VerifierConfig from the service's +// repo + the supplied AccountKID-builder closure, then delegates to +// acme.VerifyJWS. +// +// accountKID is the handler-supplied closure that returns the +// canonical kid URL for an account ID (scheme + host + per-profile +// path). VerifyJWS uses it to round-trip-check the inbound `kid` +// against what the server would have emitted on new-account. +func (s *ACMEService) VerifyJWS( + ctx context.Context, + body []byte, + requestURL string, + expectNewAccount bool, + accountKID func(accountID string) string, +) (*acme.VerifiedRequest, error) { + cfg := acme.VerifierConfig{ + Accounts: &accountAdapter{ctx: ctx, repo: s.repo}, + Nonces: &nonceAdapter{ctx: ctx, repo: s.repo}, + AccountKID: accountKID, + } + return acme.VerifyJWS(cfg, body, requestURL, acme.VerifyOptions{ + ExpectNewAccount: expectNewAccount, + }) +} + +// accountAdapter bridges the service-layer ACMERepo to the verifier's +// AccountLookup interface. The verifier doesn't take a context (its +// surface is sync-pure for testability), so the adapter captures the +// per-request context at construction time. +type accountAdapter struct { + ctx context.Context + repo ACMERepo +} + +func (a *accountAdapter) LookupAccount(accountID string) (*domain.ACMEAccount, error) { + acct, err := a.repo.GetAccountByID(a.ctx, accountID) + if err != nil { + if errors.Is(err, repository.ErrNotFound) { + return nil, acme.ErrJWSAccountNotFound + } + return nil, fmt.Errorf("acme: lookup account: %w", err) + } + return acct, nil +} + +// nonceAdapter bridges the service-layer ACMERepo's ConsumeNonce +// to the verifier's NonceConsumer interface (no-context signature). +type nonceAdapter struct { + ctx context.Context + repo ACMERepo +} + +func (n *nonceAdapter) ConsumeNonce(nonce string) error { + return n.repo.ConsumeNonce(n.ctx, nonce) +} + +// NewAccount creates (or, on RFC 8555 §7.3.1 idempotent re-registration, +// re-returns the existing) account row for the supplied JWK. Returns +// the persisted ACMEAccount + a bool indicating whether the row was +// newly created (true) or already existed (false). +// +// onlyReturnExisting=true makes the call read-only: when no account +// exists for the JWK, the service returns ErrACMEAccountDoesNotExist +// instead of creating one. +// +// State writes (cert insert + audit row) are atomic via WithinTx + +// RecordEventWithTx — same pattern as CertificateService.Create. +func (s *ACMEService) NewAccount( + ctx context.Context, + profileID string, + jwk *jose.JSONWebKey, + contact []string, + onlyReturnExisting bool, + tosAgreed bool, +) (*domain.ACMEAccount, bool, error) { + if s.tx == nil || s.auditService == nil { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, fmt.Errorf("acme: new-account requires SetTransactor + SetAuditService") + } + resolvedProfileID, err := s.resolveProfile(ctx, profileID) + if err != nil { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, err + } + + thumb, err := acme.JWKThumbprint(jwk) + if err != nil { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, fmt.Errorf("acme: thumbprint: %w", err) + } + + // RFC 8555 §7.3.1 idempotency: a new-account request for an + // already-registered JWK returns the existing row unmodified. + if existing, err := s.repo.GetAccountByThumbprint(ctx, resolvedProfileID, thumb); err == nil { + s.metrics.bump(&s.metrics.NewAccountIdempotentTotal) + return existing, false, nil + } else if !errors.Is(err, repository.ErrNotFound) { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, fmt.Errorf("acme: lookup-by-thumbprint: %w", err) + } + + if onlyReturnExisting { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, ErrACMEAccountDoesNotExist + } + + jwkPEM, err := acme.JWKToPEM(jwk) + if err != nil { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, fmt.Errorf("acme: serialize jwk: %w", err) + } + + acct := &domain.ACMEAccount{ + AccountID: acme.AccountID(thumb), + JWKThumbprint: thumb, + JWKPEM: jwkPEM, + Contact: contact, + Status: domain.ACMEAccountStatusValid, + ProfileID: resolvedProfileID, + } + + auditDetails := map[string]interface{}{ + "profile_id": resolvedProfileID, + "jwk_thumbprint": thumb, + "contact_count": len(contact), + "tos_agreed": tosAgreed, + } + + err = s.tx.WithinTx(ctx, func(q repository.Querier) error { + if err := s.repo.CreateAccountWithTx(ctx, q, acct); err != nil { + return fmt.Errorf("acme: create account: %w", err) + } + return s.auditService.RecordEventWithTx( + ctx, q, + fmt.Sprintf("acme:%s", acct.AccountID), + domain.ActorTypeUser, + "acme_account_created", + "acme_account", + acct.AccountID, + auditDetails, + ) + }) + if err != nil { + s.metrics.bump(&s.metrics.NewAccountFailureTotal) + return nil, false, err + } + s.metrics.bump(&s.metrics.NewAccountTotal) + return acct, true, nil +} + +// LookupAccount returns the account by ID. Returns +// ErrACMEAccountNotFound when the row doesn't exist (handler maps to +// 404 with RFC 7807 + RFC 8555 §6.7 accountDoesNotExist Problem). +func (s *ACMEService) LookupAccount(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + acct, err := s.repo.GetAccountByID(ctx, accountID) + if err != nil { + if errors.Is(err, repository.ErrNotFound) { + return nil, ErrACMEAccountNotFound + } + return nil, fmt.Errorf("acme: lookup account: %w", err) + } + return acct, nil +} + +// UpdateAccount replaces the account's contact list. Atomic: the +// repo update + audit row run in one WithinTx. +func (s *ACMEService) UpdateAccount( + ctx context.Context, + accountID string, + contact []string, +) (*domain.ACMEAccount, error) { + if s.tx == nil || s.auditService == nil { + s.metrics.bump(&s.metrics.UpdateAccountFailureTotal) + return nil, fmt.Errorf("acme: update-account requires SetTransactor + SetAuditService") + } + auditDetails := map[string]interface{}{ + "account_id": accountID, + "contact_count": len(contact), + } + err := s.tx.WithinTx(ctx, func(q repository.Querier) error { + if err := s.repo.UpdateAccountContactWithTx(ctx, q, accountID, contact); err != nil { + return err + } + return s.auditService.RecordEventWithTx( + ctx, q, + fmt.Sprintf("acme:%s", accountID), + domain.ActorTypeUser, + "acme_account_updated", + "acme_account", + accountID, + auditDetails, + ) + }) + if err != nil { + s.metrics.bump(&s.metrics.UpdateAccountFailureTotal) + return nil, err + } + // Re-read the row so the response carries the persisted state. + acct, err := s.LookupAccount(ctx, accountID) + if err != nil { + s.metrics.bump(&s.metrics.UpdateAccountFailureTotal) + return nil, err + } + s.metrics.bump(&s.metrics.UpdateAccountTotal) + return acct, nil +} + +// DeactivateAccount transitions the account from `valid` to +// `deactivated` (RFC 8555 §7.3.6). Subsequent JWS-authenticated +// requests using this account's kid are rejected by the verifier +// (status check at acme/jws.go). +func (s *ACMEService) DeactivateAccount(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + if s.tx == nil || s.auditService == nil { + s.metrics.bump(&s.metrics.UpdateAccountFailureTotal) + return nil, fmt.Errorf("acme: deactivate-account requires SetTransactor + SetAuditService") + } + auditDetails := map[string]interface{}{ + "account_id": accountID, + "new_status": string(domain.ACMEAccountStatusDeactivated), + } + err := s.tx.WithinTx(ctx, func(q repository.Querier) error { + if err := s.repo.UpdateAccountStatusWithTx(ctx, q, accountID, domain.ACMEAccountStatusDeactivated); err != nil { + return err + } + return s.auditService.RecordEventWithTx( + ctx, q, + fmt.Sprintf("acme:%s", accountID), + domain.ActorTypeUser, + "acme_account_deactivated", + "acme_account", + accountID, + auditDetails, + ) + }) + if err != nil { + s.metrics.bump(&s.metrics.UpdateAccountFailureTotal) + return nil, err + } + acct, err := s.LookupAccount(ctx, accountID) + if err != nil { + s.metrics.bump(&s.metrics.UpdateAccountFailureTotal) + return nil, err + } + s.metrics.bump(&s.metrics.DeactivateAccountTotal) + return acct, nil +} diff --git a/internal/service/acme_test.go b/internal/service/acme_test.go index abd9592..8617471 100644 --- a/internal/service/acme_test.go +++ b/internal/service/acme_test.go @@ -5,10 +5,14 @@ package service import ( "context" + "crypto/rand" + "crypto/rsa" "errors" "testing" "time" + jose "github.com/go-jose/go-jose/v4" + "github.com/shankar0123/certctl/internal/config" "github.com/shankar0123/certctl/internal/domain" "github.com/shankar0123/certctl/internal/repository" @@ -16,13 +20,23 @@ import ( // fakeACMERepo is an in-memory ACMERepo for tests. It tracks issued // nonces in a map; Consume removes the entry to model one-shot use. +// Phase 1b extends with account state. type fakeACMERepo struct { issued map[string]time.Time // nonce → expires_at issueErr error + + // Phase 1b — account state. + accounts map[string]*domain.ACMEAccount // account_id → row + thumbToAccount map[string]string // (profile|thumbprint) → account_id + createAccountErr error } func newFakeACMERepo() *fakeACMERepo { - return &fakeACMERepo{issued: make(map[string]time.Time)} + return &fakeACMERepo{ + issued: make(map[string]time.Time), + accounts: make(map[string]*domain.ACMEAccount), + thumbToAccount: make(map[string]string), + } } func (f *fakeACMERepo) IssueNonce(ctx context.Context, nonce string, ttl time.Duration) error { @@ -45,6 +59,88 @@ func (f *fakeACMERepo) ConsumeNonce(ctx context.Context, nonce string) error { return nil } +func (f *fakeACMERepo) CreateAccountWithTx(ctx context.Context, q repository.Querier, acct *domain.ACMEAccount) error { + if f.createAccountErr != nil { + return f.createAccountErr + } + key := acct.ProfileID + "|" + acct.JWKThumbprint + if _, exists := f.thumbToAccount[key]; exists { + return errors.New("duplicate") + } + cp := *acct + cp.CreatedAt = time.Now() + cp.UpdatedAt = cp.CreatedAt + f.accounts[acct.AccountID] = &cp + f.thumbToAccount[key] = acct.AccountID + return nil +} + +func (f *fakeACMERepo) GetAccountByID(ctx context.Context, accountID string) (*domain.ACMEAccount, error) { + acct, ok := f.accounts[accountID] + if !ok { + return nil, repository.ErrNotFound + } + cp := *acct + return &cp, nil +} + +func (f *fakeACMERepo) GetAccountByThumbprint(ctx context.Context, profileID, thumbprint string) (*domain.ACMEAccount, error) { + id, ok := f.thumbToAccount[profileID+"|"+thumbprint] + if !ok { + return nil, repository.ErrNotFound + } + return f.GetAccountByID(ctx, id) +} + +func (f *fakeACMERepo) UpdateAccountContactWithTx(ctx context.Context, q repository.Querier, accountID string, contact []string) error { + acct, ok := f.accounts[accountID] + if !ok { + return repository.ErrNotFound + } + acct.Contact = contact + acct.UpdatedAt = time.Now() + return nil +} + +func (f *fakeACMERepo) UpdateAccountStatusWithTx(ctx context.Context, q repository.Querier, accountID string, status domain.ACMEAccountStatus) error { + acct, ok := f.accounts[accountID] + if !ok { + return repository.ErrNotFound + } + acct.Status = status + acct.UpdatedAt = time.Now() + return nil +} + +// fakeTransactor is the repository.Transactor stand-in: runs fn +// against the supplied querier (we just pass nil — fakes ignore it). +// Mirrors how production transactor works without an actual DB. +type fakeTransactor struct{} + +func (f *fakeTransactor) WithinTx(ctx context.Context, fn func(q repository.Querier) error) error { + return fn(nil) +} + +// fakeAuditRepo records the audit events fakeAuditService emits so +// tests can assert on the audit row count + shape. +type fakeAuditRepo struct { + events []*domain.AuditEvent +} + +func (f *fakeAuditRepo) Create(ctx context.Context, event *domain.AuditEvent) error { + return f.CreateWithTx(ctx, nil, event) +} + +func (f *fakeAuditRepo) CreateWithTx(ctx context.Context, q repository.Querier, event *domain.AuditEvent) error { + cp := *event + f.events = append(f.events, &cp) + return nil +} + +func (f *fakeAuditRepo) List(ctx context.Context, filter *repository.AuditFilter) ([]*domain.AuditEvent, error) { + return f.events, nil +} + // fakeProfileLookup is an in-memory profileLookup that returns the // profile by ID. Unknown IDs return repository.ErrNotFound (the // canonical sentinel ACMEService maps to ErrACMEProfileNotFound). @@ -67,6 +163,20 @@ func newSvc(t *testing.T, cfg config.ACMEServerConfig, profiles map[string]*doma return NewACMEService(repo, pl, cfg), repo } +// newSvcWithAudit returns a service wired with the transactor + audit +// service required by the JWS-authenticated POST endpoints. +func newSvcWithAudit(t *testing.T, cfg config.ACMEServerConfig, profiles map[string]*domain.CertificateProfile) (*ACMEService, *fakeACMERepo, *fakeAuditRepo) { + t.Helper() + repo := newFakeACMERepo() + pl := &fakeProfileLookup{profiles: profiles} + auditRepo := &fakeAuditRepo{} + auditSvc := NewAuditService(auditRepo) + svc := NewACMEService(repo, pl, cfg) + svc.SetTransactor(&fakeTransactor{}) + svc.SetAuditService(auditSvc) + return svc, repo, auditRepo +} + func TestBuildDirectory_HappyPath(t *testing.T) { cfg := config.ACMEServerConfig{ NonceTTL: 5 * time.Minute, @@ -168,6 +278,8 @@ func TestACMEMetrics_Snapshot(t *testing.T) { m.DirectoryTotal.Store(7) m.NewNonceTotal.Store(11) m.NewNonceFailureTotal.Store(2) + m.NewAccountTotal.Store(3) + m.NewAccountIdempotentTotal.Store(1) snap := m.Snapshot() if snap["certctl_acme_directory_total"] != 7 { t.Errorf("directory_total = %d", snap["certctl_acme_directory_total"]) @@ -178,4 +290,195 @@ func TestACMEMetrics_Snapshot(t *testing.T) { if snap["certctl_acme_new_nonce_failures_total"] != 2 { t.Errorf("new_nonce_failures_total = %d", snap["certctl_acme_new_nonce_failures_total"]) } + if snap["certctl_acme_new_account_total"] != 3 { + t.Errorf("new_account_total = %d", snap["certctl_acme_new_account_total"]) + } + if snap["certctl_acme_new_account_idempotent_total"] != 1 { + t.Errorf("new_account_idempotent_total = %d", snap["certctl_acme_new_account_idempotent_total"]) + } +} + +// --- Phase 1b — account management ------------------------------------- + +func mustGenJWK(t *testing.T) *jose.JSONWebKey { + t.Helper() + k, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("rsa keygen: %v", err) + } + return &jose.JSONWebKey{Key: &k.PublicKey} +} + +func TestNewAccount_HappyPath(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, repo, auditRepo := newSvcWithAudit(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + + acct, isNew, err := svc.NewAccount(context.Background(), "prof-corp", jwk, []string{"mailto:a@example.com"}, false, true) + if err != nil { + t.Fatalf("NewAccount: %v", err) + } + if !isNew { + t.Errorf("isNew = false; want true") + } + if acct == nil || acct.AccountID == "" || acct.JWKThumbprint == "" { + t.Fatalf("account row is malformed: %+v", acct) + } + if got := svc.Metrics().NewAccountTotal.Load(); got != 1 { + t.Errorf("NewAccountTotal = %d, want 1", got) + } + if got := len(auditRepo.events); got != 1 { + t.Errorf("audit events = %d, want 1", got) + } + if got := auditRepo.events[0].Action; got != "acme_account_created" { + t.Errorf("audit action = %q", got) + } + if _, ok := repo.accounts[acct.AccountID]; !ok { + t.Errorf("account row not in repo") + } +} + +func TestNewAccount_Idempotent_ExistingJWKReturnsExistingRow(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, _, auditRepo := newSvcWithAudit(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + + first, _, err := svc.NewAccount(context.Background(), "prof-corp", jwk, []string{"mailto:a@example.com"}, false, true) + if err != nil { + t.Fatalf("first NewAccount: %v", err) + } + second, isNew, err := svc.NewAccount(context.Background(), "prof-corp", jwk, []string{"mailto:b@example.com"}, false, true) + if err != nil { + t.Fatalf("second NewAccount: %v", err) + } + if isNew { + t.Errorf("isNew = true on idempotent re-registration; want false") + } + if second.AccountID != first.AccountID { + t.Errorf("second account ID = %q; want first %q", second.AccountID, first.AccountID) + } + // Idempotent re-registration MUST NOT update contact / write a + // second audit row (RFC 8555 §7.3.1 says return the existing row + // unmodified). + if got := len(auditRepo.events); got != 1 { + t.Errorf("audit events = %d after idempotent call; want 1", got) + } + if got := svc.Metrics().NewAccountIdempotentTotal.Load(); got != 1 { + t.Errorf("NewAccountIdempotentTotal = %d, want 1", got) + } +} + +func TestNewAccount_OnlyReturnExisting_NoMatch(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, _, _ := newSvcWithAudit(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + + _, _, err := svc.NewAccount(context.Background(), "prof-corp", jwk, nil, true /*onlyReturnExisting*/, false) + if !errors.Is(err, ErrACMEAccountDoesNotExist) { + t.Errorf("err = %v; want ErrACMEAccountDoesNotExist", err) + } +} + +func TestNewAccount_OnlyReturnExisting_Match(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, _, _ := newSvcWithAudit(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + + first, _, err := svc.NewAccount(context.Background(), "prof-corp", jwk, nil, false, false) + if err != nil { + t.Fatalf("first: %v", err) + } + second, isNew, err := svc.NewAccount(context.Background(), "prof-corp", jwk, nil, true, false) + if err != nil { + t.Fatalf("second: %v", err) + } + if isNew { + t.Errorf("isNew = true; want false") + } + if second.AccountID != first.AccountID { + t.Errorf("ids differ") + } +} + +func TestUpdateAccount_HappyPath(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, _, auditRepo := newSvcWithAudit(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + acct, _, err := svc.NewAccount(context.Background(), "prof-corp", jwk, []string{"mailto:old@example.com"}, false, false) + if err != nil { + t.Fatalf("seed: %v", err) + } + + updated, err := svc.UpdateAccount(context.Background(), acct.AccountID, []string{"mailto:new@example.com"}) + if err != nil { + t.Fatalf("UpdateAccount: %v", err) + } + if len(updated.Contact) != 1 || updated.Contact[0] != "mailto:new@example.com" { + t.Errorf("contact = %v", updated.Contact) + } + // Two audit rows: the create + the update. + if got := len(auditRepo.events); got != 2 { + t.Errorf("audit events = %d, want 2", got) + } + if got := auditRepo.events[1].Action; got != "acme_account_updated" { + t.Errorf("update audit action = %q", got) + } +} + +func TestDeactivateAccount_HappyPath(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, _, auditRepo := newSvcWithAudit(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + acct, _, err := svc.NewAccount(context.Background(), "prof-corp", jwk, nil, false, false) + if err != nil { + t.Fatalf("seed: %v", err) + } + + deactivated, err := svc.DeactivateAccount(context.Background(), acct.AccountID) + if err != nil { + t.Fatalf("DeactivateAccount: %v", err) + } + if deactivated.Status != domain.ACMEAccountStatusDeactivated { + t.Errorf("status = %q, want %q", deactivated.Status, domain.ACMEAccountStatusDeactivated) + } + if got := svc.Metrics().DeactivateAccountTotal.Load(); got != 1 { + t.Errorf("DeactivateAccountTotal = %d, want 1", got) + } + if got := auditRepo.events[len(auditRepo.events)-1].Action; got != "acme_account_deactivated" { + t.Errorf("last audit action = %q", got) + } +} + +func TestLookupAccount_NotFound(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + svc, _ := newSvc(t, cfg, nil) + _, err := svc.LookupAccount(context.Background(), "acme-acc-missing") + if !errors.Is(err, ErrACMEAccountNotFound) { + t.Errorf("err = %v; want ErrACMEAccountNotFound", err) + } +} + +func TestNewAccount_RequiresTransactor(t *testing.T) { + cfg := config.ACMEServerConfig{NonceTTL: 5 * time.Minute} + // Use newSvc (no transactor wired) — NewAccount should refuse. + svc, _ := newSvc(t, cfg, map[string]*domain.CertificateProfile{ + "prof-corp": {ID: "prof-corp", Name: "corp"}, + }) + jwk := mustGenJWK(t) + _, _, err := svc.NewAccount(context.Background(), "prof-corp", jwk, nil, false, false) + if err == nil { + t.Fatal("expected error when transactor is unset") + } }