✨ Feature / Metadata validation — certificate and ds:Signature structure rules#31
Open
docJerem wants to merge 7 commits into
Open
✨ Feature / Metadata validation — certificate and ds:Signature structure rules#31docJerem wants to merge 7 commits into
docJerem wants to merge 7 commits into
Conversation
Introduces an X509-backed test helper that generates self-signed certificates (signing, encryption, CA, expired) for upcoming metadata certificate validation rules. Wires test/support into elixirc_paths and adds x509 as a test-only dependency.
Introduces ExSaml.Metadata.Certificate, plugged into the run_rules
pipeline, which emits three always-error violations against every
<md:KeyDescriptor> declared under SPSSO/IDPSSO descriptors:
* :missing_x509_certificate — KeyDescriptor without a non-empty
<ds:X509Certificate> text node.
* :invalid_x509_certificate — content that is not parseable as
base64 DER or whose ASN.1 structure pkix_decode_cert rejects.
* :certificate_expired — parseable cert whose notAfter is in the
past as of DateTime.utc_now/0. Silence-able via the existing
:ignore option for legacy scenarios.
The module relies only on :public_key, already in extra_applications
— no new runtime dependency. Cert paths are reported in XPath form,
indexed only when more than one KeyDescriptor or X509Certificate
share a parent.
Best-practice cert linting (CA detection, KeyUsage, shared
signing/encryption cert) is intentionally deferred to the next
batch, alongside the :strict-mode plumbing it depends on.
Refs #17
… allow-list
Introduces ExSaml.Metadata.Signature, plugged into the run_rules
pipeline, which inspects every <ds:Signature> declared either at
document level (child of <md:EntityDescriptor>) or nested inside a
SPSSO / IDPSSO descriptor.
Three always-error violations:
* :invalid_signature_structure — a <ds:Signature> is missing one of
its mandatory XML-DSig children (SignedInfo, SignatureValue,
SignedInfo/SignatureMethod, SignedInfo/Reference[N]/DigestMethod,
SignedInfo/Reference[N]/DigestValue) or one of those children is
missing the required @Algorithm attribute.
* :unknown_signature_algorithm — <ds:SignatureMethod Algorithm>
URI is not part of the spec-defined XML-DSig / xmldsig-more set
declared in @known_signature_algorithms.
* :unknown_digest_algorithm — same idea for <ds:DigestMethod>.
The "known" allow-list intentionally still recognises legacy URIs
(RSA-SHA1, MD5, …) because PR 2 only validates that a value is part
of the published spec. The dedicated :deprecated_signature_algorithm
rule, which flags the legacy subset, ships in PR 3 together with
strict-mode promotion.
Cryptographic verification of the signature is explicitly out of
scope (issue #17 Non-goals).
Refs #17
* Extract nested closures in Certificate so every function body stays within the credo max-nesting threshold (max depth 2). * Alias X509.Certificate / Certificate.Extension / Certificate.Validity / PrivateKey at the top of CertFactory rather than referencing them by fully-qualified module path inside function bodies. No behavioural change — pure refactor. mix audit (which runs credo --strict) now reports no findings on the touched files.
Document the six new codes emitted by validate/2 — three certificate rules and three ds:Signature structure rules — and add an Unreleased section to CHANGELOG.md. Refs #17
@moduledoc Replace the leading `# ...` comment blocks (paired with @moduledoc false) with real triple-quoted @moduledoc strings, so the rationale and list of emitted codes show up in ExDoc instead of staying buried in plain source comments. No behavioural change.
4 tasks
…/2 and Signature.violations/2 Dialyzer cannot narrow the root element through the `if Record.is_record(root, :xmlElement)` guard in `parse/1` across function boundaries, so the call site in `check_entity_descriptor/1` passes the broader xmerl record union type. That widened type violated the `:xmerl.xmlElement()` contract on `violations/2`, producing `:no_return` on `check_entity_descriptor/1` and breaking `mix dialyzer`. The existing convention in `ExSaml.Metadata` is to leave internal helpers untyped (`parse/1`, `run_rules/1`, `classify_root/1`, …), so the two new helpers now follow the same convention. No new `.dialyzer_ignore.exs` entry is added. Refs #17
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second slice of the metadata-validation effort tracked in #17 (PR 2 of the PLAN).
ExSaml.Metadata.Certificate, plugged into therun_rules/1pipeline, which inspects every<md:KeyDescriptor>declared under SPSSO/IDPSSO descriptors and emits three always-error violations::missing_x509_certificate,:invalid_x509_certificate,:certificate_expired(silence-able via:ignorefor legacy scenarios).ExSaml.Metadata.Signature, also plugged intorun_rules/1, which inspects every<ds:Signature>declared at document level or nested in a descriptor. Three always-error violations::invalid_signature_structure,:unknown_signature_algorithm,:unknown_digest_algorithm.:deprecated_signature_algorithmrule that flags legacy URIs ships in PR 3 alongside strict-mode promotion.:public_keyis already inextra_applications.:x509is added as a test-only dependency to back the newExSaml.Test.CertFactoryhelper, which generates self-signed signing / encryption / CA / expired certificates for the validation tests.Cryptographic verification of
<ds:Signature>against a trust anchor remains explicitly out of scope (see #17 "Non-goals").Decisions made during implementation
:ca_certificate_used_for_signing,:invalid_signing_key_usage, and:shared_signing_and_encryption_certificateopen between PR 2 and PR 3. They live more naturally in PR 3 alongside the:strictplumbing and the warning→error promotion mechanism, so PR 2 stays focused on always-error rules.lib/ex_saml/metadata.exorchestrates only;CertificateandSignaturelive as sibling modules and expose a singleviolations(root, namespaces) :: [violation()]entry point each. PR 3 will reuseCertificatehelpers for the CA / KeyUsage / shared-cert rules and the@known_signature_algorithmsset for the deprecated-algorithm rule.:path. Nodes that may repeat under a single parent (KeyDescriptor,X509Certificate,Signature,Reference) are indexed with 1-based[N]only when more than one is present — consistent with theAssertionConsumerService[N]convention introduced in PR 1.Out of scope for this PR
:deprecated_signature_algorithmrule — PR 3.:allow_http_endpoints,:cert_min_remaining_days— PR 3.:entity_id_domain_checkopt-in — PR 4.ExSaml.Metadata.verify_signature/2).Test plan
mix test— 220/220 passing (16 new tests: 7 cert + 9 signature intest/ex_saml/metadata_test.exs).mix format --check-formatted— clean.mix credo --strict— no issues.mix deps.unlock --check-unused— clean.Refs #17 (PR 2 of 5).