search

italia / spid-sp-test

SAML2 SPID/CIE Service Provider validation tool
European Union Public License 1.2
38 stars 17 forks source link

CIE private metadata contact information check fails #121

Closed ewedlund closed 8 months ago

ewedlund commented 2 years ago

running a test on CIE metadata 

spid_sp_test --profile cie-sp-private --metadata-url https://mycietest.somedomain.it/metadata/cie/ --debug ERROR

returns

ERROR:spid_sp_test.metadata:SpidSpMetadataCheck.test_extensions_public_private: Missing ContactPerson/Extensions/Private, this element MUST be present
ERROR:spid_sp_test.metadata:Missing ContactPerson/Extensions/Private, this element MUST be present
ERROR:spid_sp_test.metadata:SpidSpMetadataCheck.test_Contacts_PubPriv: ContactPerson MUST be present
ERROR:spid_sp_test.metadata:ContactPerson MUST be present
ERROR:spid_sp_test.metadata:SpidSpMetadataCheck.test_Contacts_PubPriv: Only one ContactPerson element of contactType "technical" MUST be present
ERROR:spid_sp_test.metadata:Only one ContactPerson element of contactType "technical" MUST be present
Spid QA: executed 85 tests, 3 failed. 0 warnings.

Even though the contact information follows the guidelines in https://docs.italia.it/italia/cie/cie-manuale-tecnico-docs/it/master/federazione.html#informazioni-di-censimento-e-contatto, i.e.

[...]
  <md:ContactPerson contactType="administrative">
    <md:Extensions>
      <cie:Private/>
      <cie:VATNumber>[...]</cie:VATNumber>
      <cie:FiscalCode>[...]</cie:FiscalCode>
      <cie:NACE2Code>[...]</cie:NACE2Code>
      <cie:Municipality>H501</cie:Municipality>
    </md:Extensions>
    <md:Company>[...]</md:Company>
    <md:EmailAddress>[...]</md:EmailAddress>
    <md:TelephoneNumber>[...]</md:TelephoneNumber>
  </md:ContactPerson>
[...]

I have also tested with the metadata example in paragraph "2.3.6. Esempio di metadata" (adding a signature and certificate), and it gives the same result as above:

<?xml version="1.0" encoding="UTF-8" standalone="no"?><md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:cie="https://www.cartaidentita.interno.gov.it/saml-extensions" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" entityID="https://entityidsp"><ds:Signature>
<ds:SignedInfo>
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
<ds:Reference URI="">
<ds:Transforms>
<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
</ds:Transforms>
<ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
<ds:DigestValue>sHAeKoRBgaL0zgAtlkzFQZ14QTu87d0R9WiwRQpRdFo=</ds:DigestValue>
</ds:Reference>
</ds:SignedInfo>
<ds:SignatureValue>
O4tRAd4e9F9Cfcl8bltM7D2JzvlTjtDfL+NU5655OqiqeT94Qg5qJFgwXSwzgiszZMKZ2iOjx0RY
cuAtntxFSW+BWROLPK4rtgF9aszktNdDeFHwR8VicV0lWjRmtLZx1+SILFhX0TUu3OXn2nKRF40g
Y4F0P/uG+0gSNNny/xnLNZwrrXFNtFwwKohBLKFwe2G//gNjMe9hThlnisZU9VuI08S65TTTo8Pz
Cz9vVCHAZwbKbu3qqCFHDotSISNI+hsXG9nPNd/pcTGR73u0s2EA2vAGryoq1gE05QxrPl0g360V
4/VYR/5PYnQrOM7nDJBrxYd5Ic7gCL1qAtlaXQ==
</ds:SignatureValue>
<ds:KeyInfo>
<ds:KeyValue>
<ds:RSAKeyValue>
<ds:Modulus>
nOdMyHFulodWTy3tLp27CSQNhDZONDMjubcSaIePcavkGh5R42MuJljpAQGxQHgXCAIANC1GyKaM
nQAg0fqtSXWNJGx/2M+HRoYB7EMbcRUgFWPsUfTu9D+7KosrXtgsWwg9gQFQ/QYkbjicg9AlcIXW
LuH4s1LnlFpiglb28lu9q+PMz2qz7lKIoKjufd3X7k3Mxo5nMn/fq0B69cmaVrJKKR9Vjja7zUZP
BU4hkqSzHAvsvpcD86ugMEE6LeIbsf2Y43uurK+Ex92j8T3kw1KdZv46OW12oNtuxj5+09MhKlu6
2Tmq4Rj0I49Aulslj43t+6wocxqdkpGUFsqiYQ==
</ds:Modulus>
<ds:Exponent>AQAB</ds:Exponent>
</ds:RSAKeyValue>
</ds:KeyValue>
<ds:X509Data>
<ds:X509Certificate>
MIIDhzCCAm+gAwIBAgIUWTIdFlVzvuJENEI9pSsFPb/iFKEwDQYJKoZIhvcNAQELBQAwUzELMAkG
A1UEBhMCSVQxDTALBgNVBAgMBFJvbWExDTALBgNVBAcMBFJvbWExDDAKBgNVBAoMA0ZVQjEYMBYG
A1UEAwwPc3BpZHRlc3QuZnViLml0MB4XDTIyMDIxODA4MTUwN1oXDTMyMDIxNjA4MTUwN1owUzEL
MAkGA1UEBhMCSVQxDTALBgNVBAgMBFJvbWExDTALBgNVBAcMBFJvbWExDDAKBgNVBAoMA0ZVQjEY
MBYGA1UEAwwPc3BpZHRlc3QuZnViLml0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA
nOdMyHFulodWTy3tLp27CSQNhDZONDMjubcSaIePcavkGh5R42MuJljpAQGxQHgXCAIANC1GyKaM
nQAg0fqtSXWNJGx/2M+HRoYB7EMbcRUgFWPsUfTu9D+7KosrXtgsWwg9gQFQ/QYkbjicg9AlcIXW
LuH4s1LnlFpiglb28lu9q+PMz2qz7lKIoKjufd3X7k3Mxo5nMn/fq0B69cmaVrJKKR9Vjja7zUZP
BU4hkqSzHAvsvpcD86ugMEE6LeIbsf2Y43uurK+Ex92j8T3kw1KdZv46OW12oNtuxj5+09MhKlu6
2Tmq4Rj0I49Aulslj43t+6wocxqdkpGUFsqiYQIDAQABo1MwUTAdBgNVHQ4EFgQUsYZoOsU+Hcs5
f8id5LMA3lZJyPkwHwYDVR0jBBgwFoAUsYZoOsU+Hcs5f8id5LMA3lZJyPkwDwYDVR0TAQH/BAUw
AwEB/zANBgkqhkiG9w0BAQsFAAOCAQEACxhOkIFjoLKAnwakk/SL89F7XNdymuWr9CIFzcUd7lEI
U5p5wqX6RYjNn1FOnwXIJlmEcN8wyr6DsAwU6nGdU6/Wi/Ky6uYHLvjjF7ZNVJxoYvSz1PH0Evtf
UjR1BzQhWB+QwMrq/sWzoZxIEp8BNk6ZWLdDjXCqjcgvcYtkI/xnX0lhM9Gp2yZpl61g93pS0jrr
pJygy0WGFEuc7oRMR4nIVmYzwlsLw9ESIeqEoJuSYAqjoLmgAXZ0qmSqrfr7/jzvxF/gZlYdfPld
Cy3dTUPPBLBN9BuhdTyqxgRmIFo+KUs9LzEcLGwpY6S//BFZsrYfStKOw09L0wizM3/flg==
</ds:X509Certificate>
</ds:X509Data>
</ds:KeyInfo>
</ds:Signature>
  <md:SPSSODescriptor AuthnRequestsSigned="true" WantAssertionsSigned="true" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
    <md:KeyDescriptor use="signing">
      <ds:KeyInfo>
        <ds:X509Data>
          <ds:X509Certificate>MIIDhzCCAm+gAwIBAgIUJmEv/+Lan9lS+/EkbZEQRUBPSEYwDQYJKoZIhvcNAQELBQAwUzELMAkG
A1UEBhMCSVQxDTALBgNVBAgMBFJvbWExDTALBgNVBAcMBFJvbWExDDAKBgNVBAoMA0ZVQjEYMBYG
A1UEAwwPc3BpZHRlc3QuZnViLml0MB4XDTIxMTAyNTExMTkyM1oXDTMxMTAyMzExMTkyM1owUzEL
MAkGA1UEBhMCSVQxDTALBgNVBAgMBFJvbWExDTALBgNVBAcMBFJvbWExDDAKBgNVBAoMA0ZVQjEY
MBYGA1UEAwwPc3BpZHRlc3QuZnViLml0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA
4T8JyuEGnwD0PqUkdTvZVacYJ/Tn8odcBTs9eSP2XwD3ImprF1yH7DD4LAcHrC7pyxvURtRUu1dL
1aV/DyEAzK37vtmF0iPQ9LnKkkqe1RxrNqtEA8qhKw4XYn6vKuml4kTTxz51NhzZHchbwtkSoyTB
9carnms9VHz82mgxdyE6HqjVqhIZ6FPecA4kdM/UCuHnFbV9KDrLJP7mOGJ3ARRxBvA/riRUuv0u
9ZVXjIs8TMBN6UeSZ7lZGQseOPFVUvjJdOhxjiMbR3Lap+egPqXGt7HLkRBI3qOj9PTdKrEZWNOI
UMgyFsTXBRKz3EzlWKAS7iNALngZuf0EKAz5IwIDAQABo1MwUTAdBgNVHQ4EFgQUK5TtiQCa2+6u
2CSMZIZuYG/woiQwHwYDVR0jBBgwFoAUK5TtiQCa2+6u2CSMZIZuYG/woiQwDwYDVR0TAQH/BAUw
AwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAo2rfMbsVg7tNyw4uyyGWrBKyLVhjhQ6peAJZJni8U2Rz
s6+vTtY+Okzzj9qv0Mxh42ksJ6eEkxy132Me4M2B/AGbc7GtTKiSzBmzG089pXlo2bI8S+Z5QKxm
6V8lrPYbvQcU4y1LPb5kT0Znlk6pbr0CpgHiwhoGPj/5sxQAeTdx2/tR/Wvh5p0hsLKZW+GjDT0R
vll1/xIaKd7l0bbko5aZW5n5m7AM2RMuf7rsfgk1NejFnuvw4U2NuL4gNj6CUK+oFKH27/bQ1BYm
L4WFoxqt0xU/4S7Og4ANq9QSHrvA/Mzi9zN2Os0M5nJtmgV7WpAxiUEXIRNfWvtYv8QIRQ==
          </ds:X509Certificate>
        </ds:X509Data>
      </ds:KeyInfo>
    </md:KeyDescriptor>
    <md:SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://url_esempio_SLO_Redirect"/>
    <md:AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://url_esempio_POST" index="0" isDefault="true"/>
    <md:AttributeConsumingService index="0">
      <md:ServiceName xml:lang="">urn:uuid:86eabbc2-6023-4f8d-a7dc-22401f5ac4fe</md:ServiceName>
      <md:RequestedAttribute Name="name"/>
      <md:RequestedAttribute Name="familyName"/>
      <md:RequestedAttribute Name="dateOfBirth"/>
      <md:RequestedAttribute Name="fiscalNumber"/>
    </md:AttributeConsumingService>
  </md:SPSSODescriptor>
  <md:Organization>
    <md:OrganizationName xml:lang="it">Service Provider Privato s.r.l.</md:OrganizationName>
    <md:OrganizationDisplayName xml:lang="it">SPP</md:OrganizationDisplayName>
    <md:OrganizationURL xml:lang="it">https://www.esempio_sp_privato.it</md:OrganizationURL>
  </md:Organization>
  <md:ContactPerson contactType="administrative">
    <md:Extensions>
      <cie:Private/>
      <cie:VATNumber>IT01234567890</cie:VATNumber>
      <cie:FiscalCode>9876543210</cie:FiscalCode>
      <cie:NACE2Code>CODICE_ATECO</cie:NACE2Code>
      <cie:Municipality>CODICE_ISTAT</cie:Municipality>
    </md:Extensions>
    <md:Company>Service Provider Privato s.r.l.</md:Company>
    <md:EmailAddress>esempio_sp_privato@spp.it</md:EmailAddress>
    <md:TelephoneNumber>+39061234567</md:TelephoneNumber>
  </md:ContactPerson>
</md:EntityDescriptor>
peppelinux commented 2 years ago

Hi @ewedlund

we could move forward with an example metadata in the unit tests to analyze where the problem is and fix it asap

peppelinux commented 2 years ago

As I can see we need a cie private metadata, we actually have only the public one https://github.com/italia/spid-sp-test/blob/main/tests/metadata/public-sp-cie.xml

so we need first of an unsigned private-sp-cie.xml that would be signed and named private-sp-cie_signed.xml

Once get this done, we should take a look here: https://github.com/italia/spid-sp-test/blob/834c9f2cbafb821c0bc7f4b46088bd4578aff537/src/spid_sp_test/metadata.py#L1061

we have to check where code must be improved/fixed

ewedlund commented 2 years ago

So I have have done some testing and digging, and I think the problem is in https://github.com/italia/spid-sp-test/blob/834c9f2cbafb821c0bc7f4b46088bd4578aff537/src/spid_sp_test/metadata.py

The checks for public metadata: https://github.com/italia/spid-sp-test/blob/834c9f2cbafb821c0bc7f4b46088bd4578aff537/src/spid_sp_test/metadata.py#L1053-L1059

And for private: https://github.com/italia/spid-sp-test/blob/834c9f2cbafb821c0bc7f4b46088bd4578aff537/src/spid_sp_test/metadata.py#L1061-L1068

In https://docs.italia.it/italia/cie/cie-manuale-tecnico-docs/it/master/federazione.html#informazioni-di-censimento-e-contatto I don't see any difference in administrative/technical contacts. The technical contact is not mandatory for any of the two profiles, it should be present only if the SP uses an external technical entity.

Changing the method test_profile_cie_sp_private to:

    def test_profile_cie_sp_private(self):
        self.test_profile_cie_sp()
        self.test_extensions_public_private(
            ext_type="Private", contact_type="administrative"
        )
        self.test_Contacts_PubPriv(contact_type="administrative")
        self.test_extensions_cie(ext_type="Private")

Makes the metadata pass the tests, and looks more reasonable to me, or am I missing something?

peppelinux commented 2 years ago

Ciao @ewedlund, sorry for the late!

We may leave self.test_Contacts_PubPriv(contact_type="technical") as it is and makes the entire check not mandatory if the contact_type == technical

ewedlund commented 2 years ago

Ciao @ewedlund, sorry for the late!

We may leave self.test_Contacts_PubPriv(contact_type="technical") as it is and makes the entire check not mandatory if the contact_type == technical

Sounds OK to me.

peppelinux commented 1 year ago

Ciao, i nuovi files XSD condivisi dai colleghi di IPZS sono stati aggiornati in questa release https://github.com/italia/spid-sp-test/releases/tag/v1.2.11 Questo problema è ancora presente a seguito di questo aggiornamento?

ewedlund commented 8 months ago

I'm really sorry about the extreme delay in response, I have just now got back to working on CIE after a long "break".

Unfortunately the problem seems to remain, my test continues to fail and I checked the release you refer to, and the only change seems to be to the cie.xsd whereas the specification of the contact types is in the enumeration here: https://github.com/italia/spid-sp-test/blob/c008edfac47892987c971e6ff27958d93dea2cf2/src/spid_sp_test/xsd/cie/saml-schema-metadata-sp-cie.xsd#L153.

What I do not understand is why the tests test_profile_cie_sp_public and test_profile_cie_sp_private are different, since I cannot find any documentation stating that there is any difference in the type of contacts, the technical contact is never mandatory (https://docs.italia.it/italia/cie/cie-eid-saml-docs/it/versione-corrente/federazione.html#informazioni-di-censimento-e-contatto).

I repeat my suggestion in https://github.com/italia/spid-sp-test/issues/121#issuecomment-1047895719 to modify test_profile_cie_sp_private so that it is the same as test_profile_cie_sp_public

(I also saw that it is not just me getting this error: https://github.com/italia/spid-sp-test/issues/162#issue-1745594948)

peppelinux commented 8 months ago

could you please propose a PR?