🔒 Security / Validate assertion Issuer against IdP entity_id#46
Open
Actalab wants to merge 2 commits into
Open
🔒 Security / Validate assertion Issuer against IdP entity_id#46Actalab wants to merge 2 commits into
Actalab wants to merge 2 commits into
Conversation
… #165 finding 13)
The assertion Issuer was extracted but never compared to the IdP's
entity_id, so IdP/identity confusion was possible — and critical whenever
assertion signature verification is relaxed.
- SpConfig: add :idp_entity_id, populated from IdpData.entity_id (the
entityID parsed from the IdP metadata) in build_sp_config/2.
- Saml.validate_assertion/4: new optional expected_issuer arg + a
validate_issuer/2 step that rejects a mismatch with {:error, :bad_issuer}.
Backward compatible: when the expected issuer is nil/empty the check is
skipped, so existing 3-arity callers are unaffected.
- SP.validate_assertion passes sp.idp_entity_id.
Refs shark-up/cryptr-gateway#165
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.
Contexte
Audit de sécurité du gateway — shark-up/cryptr-gateway#165, finding #13 (🟠) :
Issuerde l'assertion jamais validé (IdP confusion).L'
Issuerde l'assertion était extrait mais jamais comparé à l'entity_idde l'IdP. La séparation entre IdP ne reposait que sur le fingerprint du certificat de signature — correcte quand la signature est exigée, critique dès qu'elle est relâchée.Changement
SpConfig: nouveau champ:idp_entity_id, alimenté depuisIdpData.entity_id(l'entityIDparsé du metadata IdP) dansbuild_sp_config/2.Saml.validate_assertion/4: nouvel argument optionnelexpected_issuer+ étapevalidate_issuer/2qui rejette un mismatch avec{:error, :bad_issuer}.SP.validate_assertiontransmetsp.idp_entity_id.Rétro-compatibilité
Quand l'issuer attendu est
nil/vide, la vérification est ignorée → les appelants 3-arity existants et les tests ne sont pas affectés. Pour un IdP configuré via metadata,idp_entity_idest l'entityIDdu metadata et l'Issuerde l'assertion est validé contre lui (comportement SAML attendu).Refs shark-up/cryptr-gateway#165