fails to parse Google IdP SAML metadata XML

vmpjdc commented 3 months ago

Bug Description

The charm fails as follows when parsing the Google IdP metadata:

unit-saml-integrator-0: 22:07:51 ERROR unit.saml-integrator/0.juju-log saml:10: Uncaught exception while in charm code:
Traceback (most recent call last):
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/./src/charm.py", line 90, in <module>
    main(SamlIntegratorOperatorCharm)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/main.py", line 544, in main
    manager.run()
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/main.py", line 520, in run
    self._emit()
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/main.py", line 509, in _emit
    _emit_charm_event(self.charm, self.dispatcher.event_name)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/main.py", line 143, in _emit_charm_event
    event_to_emit.emit(*args, **kwargs)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/framework.py", line 352, in emit
    framework._emit(event)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/framework.py", line 851, in _emit
    self._reemit(event_path)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/venv/ops/framework.py", line 941, in _reemit
    custom_handler(event)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/./src/charm.py", line 54, in _on_relation_created
    self._update_relations()
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/./src/charm.py", line 73, in _update_relations
    self.saml.update_relation_data(relation, self.get_saml_data())
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/./src/charm.py", line 84, in get_saml_data
    certificates=self._saml_integrator.certificates,
  File "/usr/lib/python3.10/functools.py", line 981, in __get__
    val = self.func(instance)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/src/saml.py", line 144, in certificates
    tree = self.tree
  File "/usr/lib/python3.10/functools.py", line 981, in __get__
    val = self.func(instance)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/src/saml.py", line 87, in tree
    not self.signing_certificate
  File "/usr/lib/python3.10/functools.py", line 981, in __get__
    val = self.func(instance)
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/src/saml.py", line 120, in signing_certificate
    tree = self._read_tree()
  File "/var/lib/juju/agents/unit-saml-integrator-0/charm/src/saml.py", line 62, in _read_tree
    tree = etree.fromstring(raw_data)  # nosec
  File "src/lxml/etree.pyx", line 3264, in lxml.etree.fromstring
  File "src/lxml/parser.pxi", line 1911, in lxml.etree._parseMemoryDocument
ValueError: Unicode strings with encoding declaration are not supported. Please use bytes input or XML fragments without declaration.
unit-saml-integrator-0: 22:07:52 ERROR juju.worker.uniter.operation hook "saml-relation-created" (via hook dispatching script: dispatch) failed: exit status 1

This appears to be by design.

It looks like skipping the .decode(...) (implied below, explicit in the charm code) might fix the problem:

Python 3.12.3 (main, Jul 31 2024, 17:43:48) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from lxml import etree
>>> good = etree.fromstring(open('GoogleIDPMetadata.xml', 'rb').read())
>>> good
<Element {urn:oasis:names:tc:SAML:2.0:metadata}EntityDescriptor at 0x7bba7d5b7780>
>>> bad = etree.fromstring(open('GoogleIDPMetadata.xml').read())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "src/lxml/etree.pyx", line 3287, in lxml.etree.fromstring
  File "src/lxml/parser.pxi", line 1984, in lxml.etree._parseMemoryDocument
ValueError: Unicode strings with encoding declaration are not supported. Please use bytes input or XML fragments without declaration.
>>> _

To Reproduce

Deploy saml-integrator, and integrate with a suitable consumer:

stg-netbox@is-bastion-ps6:~/google-saml$ cat config.yaml 
saml-integrator:
  entity_id: "https://accounts.google.com/o/saml2?idpid=C03oypjtr"
  fingerprint: "A3:B0:BA:05:B7:5F:DF:1B:34:BD:68:C6:5A:3A:A8:73:EC:EC:58:89:C1:06:86:60:59:9D:E7:F8:57:F8:3D:7D"
  metadata_url: https://people.canonical.com/~pjdc/GoogleIDPMetadata.xml
stg-netbox@is-bastion-ps6:~/google-saml$ juju deploy -m admin/stg-netbox-k8s saml-integrator --config config.yaml
stg-netbox@is-bastion-ps6:~/google-saml$ juju integrate -m admin/stg-netbox-k8s netbox saml-integrator
stg-netbox@is-bastion-ps6:~/google-saml$ _

Most likely the consumer can be anything, even a stub; the bug triggers pretty early during integration.

Environment

Juju 3.4.5 on Canonical K8s, itself deployed on OpenStack:

stg-netbox@is-bastion-ps6:~/google-saml$ juju status -m admin/stg-netbox-k8s
Model           Controller                      Cloud/Region            Version  SLA          Timestamp
stg-netbox-k8s  juju-controller-34-staging-ps6  stg-netbox-k8s/default  3.4.5    unsupported  22:35:14Z

SAAS        Status  Store  URL
postgresql  active  local  admin/stg-netbox.postgresql

App                   Version  Status   Scale  Charm                 Channel        Rev  Address         Exposed  Message
httprequest-lego-k8s           active       1  httprequest-lego-k8s  latest/stable   91  10.152.183.102  no       3/3 certificate requests are fulfilled
netbox                         active       1  netbox                latest/stable   18  10.152.183.87   no       
redis-k8s             7.2.5    active       1  redis-k8s             latest/edge     34  10.152.183.53   no       
s3-integrator                  waiting      1  s3-integrator         latest/edge     28  10.152.183.210  no       waiting for units to settle down
saml-integrator                waiting      1  saml-integrator       latest/stable   48  10.152.183.44   no       installing agent
traefik               2.11.0   active       3  traefik-k8s           latest/beta    199  10.152.183.237  yes      Serving at netbox.staging.admin.canonical.com

Unit                     Workload     Agent  Address     Ports  Message
httprequest-lego-k8s/0*  active       idle   10.1.4.28          3/3 certificate requests are fulfilled
netbox/0*                active       idle   10.1.4.3           
redis-k8s/0*             active       idle   10.1.2.85          
s3-integrator/0*         maintenance  idle   10.1.4.140         
saml-integrator/0*       error        idle   10.1.3.145         hook failed: "saml-relation-created"
traefik/0*               active       idle   10.1.4.7           Serving at netbox.staging.admin.canonical.com
traefik/1                active       idle   10.1.3.40          Serving at netbox.staging.admin.canonical.com
traefik/2                active       idle   10.1.2.202         Serving at netbox.staging.admin.canonical.com
stg-netbox@is-bastion-ps6:~/google-saml$ _

Relevant log output

See description.

Additional context

No response

vmpjdc commented 2 months ago

I thought I might be able to work around this by removing the encoding attribute from my hosted copy of the metadata file, but that breaks the metadata signature.

vmpjdc commented 2 months ago

The previous was in fact due to writing the fingerprint in uppercase: https://github.com/canonical/saml-integrator-operator/issues/91

Using my modified copy of GoogleIDPMetadata.xml seems to work as well.

canonical / saml-integrator-operator