Fix OCSP GET base64url decoding to handle missing padding#33
Fix OCSP GET base64url decoding to handle missing padding#33
Conversation
…ncrete class for storing CA profiles.
…/response handling.
Agent-Logs-Url: https://github.com/jjrdk/opencertserver/sessions/b6f87071-d185-46ee-9ea1-32fac3b8ef73 Co-authored-by: jjrdk <149390+jjrdk@users.noreply.github.com>
…nce, GET, step definitions Agent-Logs-Url: https://github.com/jjrdk/opencertserver/sessions/b6f87071-d185-46ee-9ea1-32fac3b8ef73 Co-authored-by: jjrdk <149390+jjrdk@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the EST/ACME/OCSP components to better align with relevant RFC requirements and adds a new conformance-style test harness to drive/verify protocol behavior across the certificate server stack.
Changes:
- EST: adjust
/cacertspublication (including rollover bundles), update/csrattrshandling, and add manual-authorization + POP (tls-unique) checks. - ACME: tighten request validation (content-type, envelope rules, alg support, POST-as-GET semantics), add revocation support, and improve order/challenge error shaping.
- OCSP/CA: add strict OCSP HTTP binding option + OCSP GET support, correct issuer hashing behavior, add freshness window configuration, and introduce CA profile rollover + publication chain.
Reviewed changes
Copilot reviewed 112 out of 118 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/opencertserver.est.server.tests/TestCsrAttributesLoader.cs | Updates test CSR attrs loader to return CsrAttributesResponse. |
| tests/opencertserver.est.server.tests/Steps/EstServer.cs | Updates EST client construction to pass options: null after signature change. |
| tests/opencertserver.cli.tests/StepDefinitions/TestCsrAttributesHandler.cs | Updates CLI test CSR attrs loader to return CsrAttributesResponse. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestManualAuthorizationStrategy.cs | Adds test manual-authorization strategy for EST enroll deferral behavior. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestCsrAttributesLoader.cs | Adds configurable CSR attrs loader behavior for certserver tests. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestAcmeIssuer.cs | Adds test issuer wrapper to simulate ACME issuance failures. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestAcmeHttp01ChallengeValidator.cs | Adds controllable http-01 validator for ACME conformance testing. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestAcmeDns01ChallengeValidator.cs | Adds controllable dns-01 validator for ACME conformance testing. |
| tests/opencertserver.certserver.tests/StepDefinitions/TestAcmeChallengeValidationState.cs | Adds shared state for test ACME challenge validation outcomes. |
| tests/opencertserver.certserver.tests/StepDefinitions/Ocsp.cs | Updates OCSP tests to use correct issuer cert for CertID creation. |
| tests/opencertserver.certserver.tests/StepDefinitions/EstEnrollment.cs | Updates EST client creation and makes enrollment step more robust to null client setup. |
| tests/opencertserver.certserver.tests/StepDefinitions/CertificateServerFeatures.cs | Expands test host wiring (OCSP strict mode, freshness window, ACME test doubles, manual auth strategy). |
| tests/opencertserver.certserver.tests/README.md | Adds documentation for the new certserver conformance test project. |
| tests/opencertserver.certserver.tests/Features/OcspConformance.feature | Adds OCSP RFC 6960 conformance inventory scenarios. |
| tests/opencertserver.certserver.tests/Features/AcmeConformance.feature | Adds ACME RFC 8555 conformance inventory scenarios. |
| tests/opencertserver.ca.tests/CaProfileTests.cs | Adds unit tests for CA profile rollover window closure behavior. |
| src/opencertserver.lambda/DefaultIssuer.cs | Plumbs ACME notBefore/notAfter into CA signing. |
| src/opencertserver.est.server/README.md | Documents EST /cacerts rollover publication behavior and host options. |
| src/opencertserver.est.server/Handlers/TlsUniqueProofOfPossessionVerifier.cs | Adds tls-unique POP marker detection helper. |
| src/opencertserver.est.server/Handlers/SimpleReEnrollHandler.cs | Reworks reenroll request parsing/validation and CMS encoding of responses. |
| src/opencertserver.est.server/Handlers/SimpleEnrollHandler.cs | Adds base64 normalization, manual-authorization deferral, POP check, and CMS encoding of responses. |
| src/opencertserver.est.server/Handlers/ServerKeyGenHandler.cs | Refactors server-side keygen to support RSA/ECDSA and encrypted key-delivery negotiation. |
| src/opencertserver.est.server/Handlers/RetryAfterResult.cs | Adds an IResult returning 202 + Retry-After for manual authorization deferrals. |
| src/opencertserver.est.server/Handlers/IManualAuthorizationStrategy.cs | Introduces pluggable manual authorization strategy (default no-op). |
| src/opencertserver.est.server/Handlers/ICsrTemplateLoader.cs | Changes contract to return CsrAttributesResponse for RFC 7030/9908 behavior. |
| src/opencertserver.est.server/Handlers/EstMultipartBase64Content.cs | Adds multipart helper that emits base64 transfer encoding. |
| src/opencertserver.est.server/Handlers/CsrTemplateLoader.cs | Default loader now returns “unavailable” csrattrs response. |
| src/opencertserver.est.server/Handlers/CsrAttributesResponse.cs | Introduces a status-code + payload wrapper for csrattrs responses. |
| src/opencertserver.est.server/Handlers/CsrAttributesHandler.cs | Updates handler to emit CsrAttributesResponse via result type. |
| src/opencertserver.est.server/Handlers/CertificateSigningRequestTemplateResult.cs | Updates csrattrs response encoding to DER->base64 string for the response body. |
| src/opencertserver.est.server/Handlers/CaCertHandler.cs | Updates /cacerts handler to publish CMS-wrapped cert bundles (and renames handler). |
| src/opencertserver.est.server/EstServerExtensions.cs | Registers new services and updates EST endpoint mapping (/cacerts, csrattrs behavior). |
| src/opencertserver.est.server/EstPublishedCertificatesResolver.cs | Adds resolver delegate for EST-published CA bundle selection. |
| src/opencertserver.est.client/EstClientOptions.cs | Adds EST client options (trust anchor modes, explicit anchors, revocation knobs). |
| src/opencertserver.est.client/EstBootstrapTrust.cs | Adds bootstrap trust object + certificate fingerprints for out-of-band verification. |
| src/opencertserver.cli/Program_EstServerCertificates.cs | Adds CLI options for explicit trust anchors and trust-anchor mode selection. |
| src/opencertserver.cli/Program_EstReEnroll.cs | Adds CLI options for explicit trust anchors and trust-anchor mode selection. |
| src/opencertserver.cli/Program_EstEnroll.cs | Adds CLI options for explicit trust anchors and trust-anchor mode selection; fixes output path handling. |
| src/opencertserver.certserver/Program.cs | Adds profile creation helper with published bundle support and config-driven TLS 1.2/1.3. |
| src/opencertserver.certserver/DefaultIssuer.cs | Plumbs ACME notBefore/notAfter into CA signing. |
| src/opencertserver.certserver/DefaultCsrValidator.cs | Tightens ACME CSR validation (SAN presence + exact identifier match) with better errors. |
| src/opencertserver.ca/CertificateAuthority.cs | Adds notBefore/notAfter support, published certificates retrieval, OCSP freshness config, and rollover publication cert generation. |
| src/opencertserver.ca/CaConfiguration.cs | Adds strict OCSP HTTP binding option to configuration. |
| src/opencertserver.ca.utils/X509Extensions/X509CertificatesExtensions.cs | Adds typed SAN DNS/IP extraction helpers using ASN.1 parsing. |
| src/opencertserver.ca.utils/X509/Templates/SingleAttributeTemplate.cs | Fixes ASN.1 reading/writing to preserve encoded values correctly. |
| src/opencertserver.ca.utils/X509/Templates/CsrAttributes.cs | Adds CSR attributes structure supporting legacy + template-based csrattrs responses. |
| src/opencertserver.ca.utils/X509/Templates/CsrAttribute.cs | Adds legacy CSR attribute encoding/decoding support. |
| src/opencertserver.ca.utils/X509/Templates/CertificateSigningRequestTemplate.cs | Adds CRI attribute support and correct encoding for template attributes. |
| src/opencertserver.ca.utils/X509/Templates/AsnEncodedValue.cs | Adds helper to carry pre-encoded ASN.1 values. |
| src/opencertserver.ca.utils/X509/GeneralName.cs | Fixes ipAddress parsing/representation and generalizes value type. |
| src/opencertserver.ca.utils/X509/AsnOctetString.cs | Adds ASN.1 octet-string value type for raw bytes (e.g., IP SANs). |
| src/opencertserver.ca.utils/Pkcs7/ContentInfo.cs | Fixes content emission logic for empty content. |
| src/opencertserver.ca.utils/Pkcs7/CmsContentInfo.cs | Adds CMS ContentInfo variant used by EST responses. |
| src/opencertserver.ca.utils/Oids.cs | Adds missing OIDs used for CSR attributes/templates. |
| src/opencertserver.ca.utils/Ocsp/TbsRequest.cs | Fixes signature algorithm identifiers for OCSP request signing. |
| src/opencertserver.ca.utils/Ocsp/SingleResponse.cs | Fixes certStatus tagging/decoding and adds per-response extensions support. |
| src/opencertserver.ca.utils/Ocsp/RevokedInfo.cs | Uses GeneralizedTime for revocationTime (RFC 6960). |
| src/opencertserver.ca.utils/Ocsp/ResponseData.cs | Adds responseExtensions support and encoding/decoding. |
| src/opencertserver.ca.utils/Ocsp/IValidateOcspRequest.cs | Changes validator contract to return OcspResponseStatus? instead of string. |
| src/opencertserver.ca.utils/Ocsp/CertId.cs | Adds overload to compute issuer hashes from issuer certificate (RFC 6960 correctness). |
| src/opencertserver.ca.utils/EncodingExtensions.cs | Adds base64 normalization + CertID hash selection helper + byte[] Base64Encode + padding rules. |
| src/opencertserver.ca.utils/CertificateExtensions.cs | Moves Runtime.InteropServices using into the namespace block (style/ordering). |
| src/opencertserver.ca.utils/Ca/ICertificateAuthority.cs | Adds notBefore/notAfter support + published certificates method. |
| src/opencertserver.ca.utils/Ca/CaProfile.cs | Adds published chain + rollover/close-window support + OCSP settings + safer disposal. |
| src/opencertserver.ca.server/OcspRequestSignatureValidator.cs | Adds signed OCSP request signature validation hook. |
| src/opencertserver.ca.server/Handlers/OcspHandler.cs | Adds strict binding checks, OCSP GET support, nonce echo, issuer validation, and configurable freshness. |
| src/opencertserver.ca.server/Extensions.cs | Registers OCSP request validator and maps OCSP GET endpoint; adds strict binding/freshness options to CA setup. |
| src/opencertserver.acme.server/Workers/ValidationWorker.cs | Improves ACME error types/details and state transitions during challenge validation. |
| src/opencertserver.acme.server/Stores/OrderStore.cs | Adds per-account order ID listing for orderList support. |
| src/opencertserver.acme.server/Stores/InMemoryOrderStore.cs | Adds per-account order ID listing for in-memory store. |
| src/opencertserver.acme.server/Stores/InMemoryAccountStore.cs | Adds JWK-based account lookup for onlyReturnExisting semantics. |
| src/opencertserver.acme.server/Stores/AccountStore.cs | Adds JWK-based account lookup by scanning stored account.json files. |
| src/opencertserver.acme.server/Services/TokenChallengeValidator.cs | Aligns error types/messages with RFC semantics and fixes typo. |
| src/opencertserver.acme.server/Services/Http01ChallengeValidator.cs | Uses standardized rejectedIdentifier error type for prohibited address ranges. |
| src/opencertserver.acme.server/Services/DefaultRevocationService.cs | Adds ACME revokeCert implementation (authorization + CA revocation). |
| src/opencertserver.acme.server/Services/DefaultOrderService.cs | Enforces notBefore/notAfter window validity, sets order expiry from authz, supports order list, deactivates authz, and passes issuance window through issuer. |
| src/opencertserver.acme.server/Services/DefaultAccountService.cs | Implements account update/deactivate/keyChange behaviors and uses store-based FindAccount. |
| src/opencertserver.acme.server/RequestServices/DefaultRequestValidationService.cs | Adds request envelope/content-type validation, endpoint-specific jwk/kid rules, POST-as-GET empty-payload enforcement, and broader alg support. |
| src/opencertserver.acme.server/JsonDefaults.cs | Adds source-gen JSON types for new request DTOs. |
| src/opencertserver.acme.server/Filters/ValidateAcmeRequestFilter.cs | Passes endpoint name + content-type into request validation; uses protocol exception types. |
| src/opencertserver.acme.server/Filters/AcmeProtocolResponseFilter.cs | Centralizes nonce emission + problem+json ACME error shaping for exceptions. |
| src/opencertserver.acme.server/Extensions/ServiceCollectionExtensions.cs | Registers revocation service in DI. |
| src/opencertserver.acme.server/Endpoints/RevocationEndpoints.cs | Adds /revoke-cert endpoint mapping. |
| src/opencertserver.acme.server/Endpoints/OrderEndpoints.cs | Tightens identifier validation, adds authz deactivation handling, and supports POST-as-GET challenge retrieval. |
| src/opencertserver.acme.server/Endpoints/NonceEndpoints.cs | Renames helper to ensure replay-nonce and reuses it consistently. |
| src/opencertserver.acme.server/Endpoints/DirectoryEndpoints.cs | Advertises revokeCert + externalAccountRequired + emits absolute HTTPS URLs via LinkGenerator. |
| src/opencertserver.acme.server/Endpoints/AccountEndpoints.cs | Implements newAccount semantics (TOS/EAB), keyChange nested JWS support, account update/deactivate, and order list responses. |
| src/opencertserver.acme.server/Configuration/ACMEServerOptions.cs | Adds ExternalAccountRequired option. |
| src/opencertserver.acme.server/AcmeRegistration.cs | Adds protocol response filter and revocation endpoints to routing group. |
| src/opencertserver.acme.server/AcmeProblemSerializerContext.cs | Adds placeholder context file for problem serialization strategy documentation. |
| src/opencertserver.acme.abstractions/Storage/IStoreOrders.cs | Adds GetOrderIds API. |
| src/opencertserver.acme.abstractions/Storage/IStoreAccounts.cs | Adds JWK-based FindAccount API. |
| src/opencertserver.acme.abstractions/Services/IRevocationService.cs | Adds revocation service contract. |
| src/opencertserver.acme.abstractions/Services/IOrderService.cs | Adds order list + authorization deactivation APIs. |
| src/opencertserver.acme.abstractions/Services/IAccountService.cs | Adds account update/deactivate/keyChange APIs. |
| src/opencertserver.acme.abstractions/RequestServices/IRequestValidationService.cs | Extends validation API to include content-type + endpoint name. |
| src/opencertserver.acme.abstractions/Model/Authorization.cs | Allows Deactivated as a valid transition state. |
| src/opencertserver.acme.abstractions/Model/Account.cs | Allows key replacement + contact/TOS updates + deactivation behavior. |
| src/opencertserver.acme.abstractions/IssuanceServices/IIssueCertificates.cs | Adds notBefore/notAfter parameters for issuance. |
| src/opencertserver.acme.abstractions/HttpModel/Requests/UpdateAuthorizationRequest.cs | Adds DTO for authorization update/deactivate requests. |
| src/opencertserver.acme.abstractions/HttpModel/Requests/UpdateAccountRequest.cs | Adds DTO for account update/deactivate requests. |
| src/opencertserver.acme.abstractions/HttpModel/Requests/RevokeCertificateRequest.cs | Adds DTO for revokeCert payload. |
| src/opencertserver.acme.abstractions/HttpModel/Requests/KeyChangeRequest.cs | Adds DTO for keyChange inner payload. |
| src/opencertserver.acme.abstractions/HttpModel/Requests/CreateOrGetAccount.cs | Makes TOS agreed nullable and adds externalAccountBinding field. |
| src/opencertserver.acme.abstractions/HttpModel/Order.cs | Always sets finalize URL and only sets certificate URL when valid. |
| src/opencertserver.acme.abstractions/HttpModel/AcmeError.cs | Adds status field and minor formatting cleanup. |
| src/opencertserver.acme.abstractions/Exceptions/BadCsrException.cs | Adds explicit badCSR exception type for finalize handling. |
| src/opencertserver.acme.abstractions/Exceptions/AccountDoesNotExistException.cs | Adds explicit accountDoesNotExist exception type. |
| src/CertesSlim/AcmeContext.cs | Namespace/type alias cleanup for directory handling. |
Files not reviewed (2)
- tests/opencertserver.certserver.tests/Features/OcspConformance.feature.cs: Language not supported
- web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/opencertserver.est.server/Handlers/SimpleReEnrollHandler.cs
Outdated
Show resolved
Hide resolved
| return true; | ||
| } | ||
| } | ||
|
|
||
| return hasIssuedOrder; |
There was a problem hiding this comment.
This returns hasIssuedOrder when no certificate match is found. That means any account with at least one issued order could revoke an unrelated certificate (authorization bypass). This should return false when no match is found, and only return true when the submitted certificate can be tied back to an order belonging to the account.
…padding safety Agent-Logs-Url: https://github.com/jjrdk/opencertserver/sessions/af6bdaaf-f872-41f6-a82f-a0129838c55d Co-authored-by: jjrdk <149390+jjrdk@users.noreply.github.com>
RFC 6960 Appendix A GET requests encode the OCSP request as base64url, which routinely omits
=padding. The previous implementation swapped-/_to standard base64 alphabet characters but never added padding, causingConvert.FromBase64Stringto throw on most real-world GET requests.Change
OcspHandler.HandleGet— replace manual base64url substitution withSystem.Buffers.Text.Base64Url.DecodeFromChars:Base64Urlis available from .NET 9+; the project targetsnet10.0.