Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: SAML2 Issuer format SPID test 30, issuer MAY be omitted #128

Merged
merged 5 commits into from
Feb 21, 2024

Conversation

peppelinux
Copy link
Member

It turns out that CIE SAML2 Response fails with SPID test number 30

at the same time, according to SAML2 Core and SPID tests, the response.issuer.format MUST be omitted or if present MUST be equal to ... and CIE SAML2 IDP returns a Saml2 Respons ewithout self.response.issuer.format

This PR fixes the Spid Validator

@MdreW
Copy link
Collaborator

MdreW commented Feb 21, 2024

Hi @peppelinux,
If I try to authenticate with your commit I receive these errors:

satosa-saml2spid-1  | [2024-02-21 12:05:35] [ERROR]: returncode=1                                                                                                                              
satosa-saml2spid-1  | error=Verification status: FAILED                                                                                                                                        
satosa-saml2spid-1  | Failure reason: SIGNATURE                                                                                                                                                
satosa-saml2spid-1  | Error: failed to verify file "/tmp/tmp0e3bnuwn.xml"                                                                                                                      
satosa-saml2spid-1  |                                                                                                                                                                          
satosa-saml2spid-1  | output= [saml2.sigver._run_xmlsec:869]                                                                                                                                   
satosa-saml2spid-1  | [2024-02-21 12:05:35] [ERROR]: check_sig: ['/usr/bin/xmlsec1', '--verify', '--enabled-reference-uris', 'empty,same-doc', '--enabled-key-data', 'raw-x509-cert', '--pubkey
-cert-pem', '/tmp/tmpc8ezm4os.pem', '--id-attr:ID', 'urn:oasis:names:tc:SAML:2.0:protocol:Response', '--node-id', '_e33b0ca1-6f0c-4ed7-81f2-a8972d116ee0', '--output', '/tmp/tmp2hkzgsh2.xml', 
'--lax-key-search', '/tmp/tmp0e3bnuwn.xml'] [saml2.sigver._check_signature:1516]                                                                                                               
satosa-saml2spid-1  | [2024-02-21 12:05:35] [ERROR]: returncode=1                                                                                                                              
satosa-saml2spid-1  | error=Verification status: FAILED                                                                                                                                        
satosa-saml2spid-1  | Failure reason: SIGNATURE                                                                                                                                                
satosa-saml2spid-1  | Error: failed to verify file "/tmp/tmp025v_xiz.xml"                                                                                                                      
satosa-saml2spid-1  |                                                                                                                                                                          
satosa-saml2spid-1  | output= [saml2.sigver._run_xmlsec:869]
satosa-saml2spid-1  | [2024-02-21 12:05:35] [ERROR]: check_sig: ['/usr/bin/xmlsec1', '--verify', '--enabled-reference-uris', 'empty,same-doc', '--enabled-key-data', 'raw-x509-cert', '--pubkey
-cert-pem', '/tmp/tmpjo1r41mg.pem', '--id-attr:ID', 'urn:oasis:names:tc:SAML:2.0:assertion:Assertion', '--node-id', '_c7559190-db4d-4ceb-adcc-1e69262ff20d', '--output', '/tmp/tmpze3ovdgv.xml'
, '--lax-key-search', '/tmp/tmp025v_xiz.xml'] [saml2.sigver._check_signature:1516]                                                                                                             
satosa-saml2spid-1  | ERROR:backends.spidsaml2_validator:Issuer NameFormat is invalid: None != "urn:oasis:names:tc:SAML:2.0:nameid-format:entity"
satosa-saml2spid-1  | [2024-02-21 12:05:35] [ERROR]: Issuer NameFormat is invalid: None != "urn:oasis:names:tc:SAML:2.0:nameid-format:entity" [backends.spidsaml2.authn_response:604]          
satosa-saml2spid-1  | [2024-02-21 12:05:35] [ERROR]: Failed to parse authn request: Issuer NameFormat is invalid: None != "urn:oasis:names:tc:SAML:2.0:nameid-format:entity"  [backends.spidsam
l2.handle_error:464]                                                                                                                                                                           
satosa-saml2spid-1  | [pid: 17|app: 0|req: 3/4] 172.24.0.1 () {74 vars in 4111 bytes} [Wed Feb 21 12:05:35 2024] POST /spidSaml2/acs/post => generated 10036 bytes in 96 msecs (HTTP/1.1 403) 3
 headers in 3077 bytes (1 switches on core 0)

@MdreW
Copy link
Collaborator

MdreW commented Feb 21, 2024

For information, before version 2.0.1 CIE was work correctly.
I had over 100 daly CIE auth at january

@peppelinux
Copy link
Member Author

@MdreW I see two problems in your output:

  1. signature validation failure
  2. issuer name format

Please check that you have this change in your docker backend:
https://github.com/italia/Satosa-Saml2Spid/pull/128/files#diff-184556c7075814dc05546801301e9b16cf0d0728884ef56a461f60e1f013c7c7R81

I'm asking since it turns out that I have relaxed this check when the format attribute is not present, while in your output it seems to me that the check still happen

@peppelinux
Copy link
Member Author

peppelinux commented Feb 21, 2024

@MdreW we didn't have changes (see: 9340187#diff-184556c7075814dc05546801301e9b16cf0d0728884ef56a461f60e1f013c7c7) then it seems to me that the CIE id IDP Saml Respose has changed making the spid validator fails

@MdreW
Copy link
Collaborator

MdreW commented Feb 21, 2024

Now work fine with SPID (no errors) but not with CIE:

satosa-saml2spid-1  | [2024-02-21 13:18:08] [INFO ]: {'msg': 'decided target backend by target issuer', 'target_issuer': 'https://idserver.servizicie.interno.gov.it/idp/profile/SAML2/POST/SSO
', 'target_backend': 'cieSaml2'} [satosa.micro_services.custom_routing.process:55]                                                                                                             
satosa-saml2spid-1  | [2024-02-21 13:18:08] [INFO ]: [urn:uuid:b7c7473d-2f4c-4856-927d-6789ce82fc39] {'message': 'Selected IdP', 'only_one': False, 'target_entity_id': 'https://idserver.servi
zicie.interno.gov.it/idp/profile/SAML2/POST/SSO', 'force_authn': None, 'memorized_idp': None, 'entity_id': 'https://idserver.servizicie.interno.gov.it/idp/profile/SAML2/POST/SSO'} [satosa.bac
kends.saml2.get_idp_entity_id:176]                                                                                                                                                             
satosa-saml2spid-1  | [pid: 18|app: 0|req: 6/11] 172.24.0.1 () {68 vars in 4160 bytes} [Wed Feb 21 13:18:08 2024] GET /Saml2/disco?entityID=https://idserver.servizicie.interno.gov.it/idp/prof
ile/SAML2/POST/SSO&areturn=https://sso.isprambiente.it/Saml2/disco => generated 6076 bytes in 26 msecs (HTTP/1.1 200) 3 headers in 3060 bytes (1 switches on core 0)                                                         
satosa-saml2spid-1  | [2024-02-21 13:18:11] [DEBUG]: [urn:uuid:5603443a-3b09-4cc7-9a59-c4eb4c27b9eb] Sending metadata response for entityId = https://sso.isprambiente.it/Saml2IDP/metadata [sa
tosa.frontends.saml2._metadata_endpoint:528]                                                                                                                                                   
satosa-saml2spid-1  | [pid: 18|app: 0|req: 7/12] 172.24.0.3 () {50 vars in 611 bytes} [Wed Feb 21 13:18:11 2024] GET /Saml2IDP/metadata => generated 6879 bytes in 99 msecs (HTTP/1.1 200) 3 he
aders in 847 bytes (1 switches on core 0)                                                                                                                                                                                                                                                       
satosa-saml2spid-1  | ERROR:backends.spidsaml2_validator:Issuer format is not valid: None.  Contattare il supporto tecnico per eventuali chiarimenti                                           
satosa-saml2spid-1  | ERROR:backends.ciesaml2:Issuer format is not valid: None.  Contattare il supporto tecnico per eventuali chiarimenti
satosa-saml2spid-1  | ERROR:backends.ciesaml2:Failed to parse authn request: Issuer format is not valid: None.  Contattare il supporto tecnico per eventuali chiarimenti                       
satosa-saml2spid-1  | [pid: 18|app: 0|req: 8/13] 172.24.0.1 () {74 vars in 4149 bytes} [Wed Feb 21 13:18:30 2024] POST /cieSaml2/acs/post => generated 10031 bytes in 53 msecs (HTTP/1.1 403) 3
 headers in 3077 bytes (1 switches on core 0)

@peppelinux peppelinux requested a review from MdreW February 21, 2024 13:40
@peppelinux peppelinux merged commit dd3f479 into master Feb 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants