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

Both X509Data and KeyValue found #7

Closed
freddy34 opened this issue Feb 26, 2021 · 5 comments
Closed

Both X509Data and KeyValue found #7

freddy34 opened this issue Feb 26, 2021 · 5 comments
Labels
good first issue Good for newcomers wontfix This will not be worked on

Comments

@freddy34
Copy link

Ciao Luca,

sia nella validazione "remota" che nella validazione "locale" (by spid-testenv2) ottengo il seguente errore Python "signxml.exceptions.InvalidInput: Both X509Data and KeyValue found. Use verify(ignore_ambiguous_key_info=True) to ignore KeyValue and validate using X509Data only." Ho seguito la tua Wiki (complimenti per la precisione) stando attento a mettere NONE alla proprietà "SAML Signature Key Name" e ho rimosso tutti i KeyName durante la creazione del file sp_metadata.xml prima di registrare il file xml nei rispettivi database IdP di test.

@lscorcia
Copy link
Collaborator

Ciao! A quanto pare lato AGID hanno aggiornato il tool spid-testenv2 una decina di giorni fa, e nel farlo hanno aggiornato anche la libreria signxml che esegue la validazione della firma XML, perché prima questo errore non c'era. Sono riuscito a riprodurre il problema, ed è tracciato anche da questa issue della libreria di validazione: XML-Security/signxml#143 .

Sostanzialmente, la libreria signxml si lamenta del fatto che il certificato fornito già contiene una chiave pubblica, e la stessa chiave pubblica è poi ripetuta all'interno dell'elemento KeyValue nella firma:

<dsig:KeyInfo>
	<dsig:X509Data>
		<dsig:X509Certificate>MIII...A==</dsig:X509Certificate>
	</dsig:X509Data>
	<dsig:KeyValue>
		<dsig:RSAKeyValue>
			<dsig:Modulus>3mD...Aqw==</dsig:Modulus>
			<dsig:Exponent>AQAB</dsig:Exponent>
		</dsig:RSAKeyValue>
	</dsig:KeyValue>
</dsig:KeyInfo>

Come indicato nella issue linkata sopra, la specifica XML Signature non vieta questa possibilità. Naturalmente è un comportamento che ha senso solo se le due chiavi pubbliche coincidono, ma l'autore della libreria non ha preso una decisione chiara e per il momento ha aggiunto una proprietà in modo che il chiamante possa decidere cosa fare. Temo che il problema quindi sia più a livello spid-testenv2 che non di questo plugin.

Proverò a vedere se è qualcosa che posso pilotare facilmente in modo da soddisfare anche lo spid-testenv2, ma tieni presente che comunque è uno strumento di scarsa utilità, è pieno di errori e non conformità. In produzione, con gli IdP reali, questo problema non si verifica. Sarebbe da verificare se il problema si verifica anche con il componente spid-saml-check, che è decisamente più importante dello spid-testenv2.

@lscorcia
Copy link
Collaborator

Ho verificato il comportamento con spid-saml-check (https://github.com/italia/spid-saml-check) e non ci sono regressioni. Probabilmente è davvero il caso di aprire una issue sul repo di spid-testenv2 per fargli ignorare il controllo sul KeyValue.

@freddy34
Copy link
Author

freddy34 commented Feb 27, 2021

Grazie infinite per la precisione delle risposte. Approfitto, in sovrapposizione con la issue/question #2 per chiederti le seguenti cose:

  • disponendo di un descrittore SAML compliant con SPID Italia all'url https://<keycloak-domain>/auth/realms/<nome realm>/spid-sp-metadata si eviterebbe lo step 3 indicato nella tua Wiki (https://github.com/lscorcia/keycloak-spid-provider/wiki/Generating-SP-metadata) oltre a disporre di un url "pubblico" da "dare in pasto" celermente al validatore spid-saml-check, corretto?
  • attualmente, soprattutto al fine di completare la registrazione formale come Service Provider, la pubblicazione del descrittore SAML generato al punto 3 della tua Wiki è a carico dello sviluppatore/fornitore (es: https://<keycloak-domain>/metadata), corretto?
    Mi scuso per domande "banali" ma servono soprattutto a sensibilizzare chi commissiona l'adozione dello SPID e magari non comprende tutte le difficoltà che si intrecciano tra problematiche applicative che logiche.

@freddy34
Copy link
Author

stavo aprendo la issue sul repo di spid-testenv2 e qualcuno questa mattina mi ha preceduto: italia/spid-testenv2#325 (comment). Dalla risposta credo che il team di sviluppo percorra la strada del NON volere la KeyValue (anche se non sono assolutamente d'accordo in quanto da come da te riportato è un'opzione plausibile e più un limite della libreria Python usata per la validazione... che in ogni caso offre la possibilità di "risolvere" la problematica con una più pulita opzione)

@lscorcia
Copy link
Collaborator

lscorcia commented Mar 8, 2021

Ciao @freddy34, nessun problema, scusa il ritardo ma ho dovuto completare alcuni interventi prima di risponderti.

Ho aggiornato il wiki per indicare come accedere al metadato SPID pubblicato direttamente da Keycloak, quindi, a meno che non siano necessari altri interventi manuali per qualche motivo particolare, il link generato da Keycloak può essere fornito direttamente ad AGID per la validazione.

Ho anche inviato la pull request per la correzione dell'anomalia sul KeyValue sia sulla libreria python sia su spid-testenv2 (italia/spid-testenv2#327). Credo che sarà integrata a breve in spid-testenv2, se hai fretta puoi comunque compilare il tool manualmente, includendo la pull request.

A presto, ciao

@lscorcia lscorcia added good first issue Good for newcomers wontfix This will not be worked on labels Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants