fix(saml): bump crystal-saml — namespaces + Shibboleth signature verification - #34
Merged
Merged
Conversation
camreeves
force-pushed
the
fix/saml-xmlns-shard-bump
branch
2 times, most recently
from
August 17, 2026 00:10
9e09b7b to
ab22630
Compare
camreeves
force-pushed
the
fix/saml-xmlns-shard-bump
branch
from
August 17, 2026 00:34
ab22630 to
1cd39ed
Compare
…gnature verification Two rounds of crystal-saml fixes found while chasing the failed UCLA Shibboleth test, now pinned at spider-gazelle/crystal-saml@d16bbb0: 1. Generated messages carried literal xmlns_samlp/xmlns_saml attributes (Crystal named args cannot contain a colon), leaving the samlp:/saml: prefixes unbound. OpenSAML rejects the AuthnRequest before resolving the SP, which is exactly the UCLA error page observed. 2. Signature verification could not verify Apache Santuario (Shibboleth) shapes: C14N stripped whitespace-only text nodes that are part of the signed bytes, every to_xml in the verify path pretty-printed (injecting whitespace the signer never saw), and the reference transform's InclusiveNamespaces PrefixList was discarded with the enveloped Signature before digest computation. 3. NotBefore/NotOnOrAfter validation ran with zero clock-skew tolerance; the settings-level default is now 10s, matching the legacy Ruby service's allowed_clock_drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Steve's real-client test against UCLA Shibboleth failed with UCLA's "The login request you submitted is invalid" page on every attempt. Decoding the SAMLRequest from the redirect URL showed the AuthnRequest carried literal attributes named
xmlns_samlp/xmlns_samlinstead of real namespace declarations (Crystal named args cannot contain a colon), so thesamlp:/saml:prefixes were unbound and OpenSAML refused the message before resolving the SP. That matches the blank "application name" on the error page. libxml2 parses the malformed XML leniently, which is why our own specs and round trips never caught it.An audit for whatever else a real Shibboleth exchange would hit found that the response leg would have failed next, so this bump also picks up:
to_xml, which pretty-prints and injects whitespace the signer never saw. The two defects had been cancelling each other, which is why self-generated round trips always passed.InclusiveNamespaces PrefixList(Shibboleth usesPrefixList="xsd") was removed along with the enveloped Signature before the digest was computed, guaranteeing a digest mismatch.allowed_clock_drift.All fixed on crystal-saml main (2644bd0 + 4b1bdc3 + d16bbb0) with regression specs whose expected canonical bytes are hand-derived rather than computed through the code under test. crystal-saml suite: 215 examples, 0 failures. Wire format verified end to end locally: generated a redirect URL with UCLA-shaped settings and decoded it exactly as the IdP does.
This PR is the shard.lock bump to d16bbb0. The lock entry is hand-written because
shards updatecannot complete on a host Crystal that fails ameba's postinstall; the entry matches what shards would write.Before retesting on UCLA nonprod, two things to confirm on their side:
adfs_stratrow must haveidp_certoridp_cert_fingerprintset. auth.cr fails closed without them, and the Ruby service working proves nothing here since its fallback validator accepted whatever certificate the response embedded.🤖 Generated with Claude Code