Insufficient logging when xmlsec binary is missing

[Guang Yee]

Keystone log is also unhelpful. All we got is

"ERROR idp _sign_assertion Error when signing assertion, reason: [Errno 2] No such file or directory"

When the xmlsec1 package is absent.

We may need to add a check here

https://github.com/openstack/keystone/blob/master/keystone/federation/idp.py#L421

to see if CONF.saml.xmlsec1_binary exist. If absent, we just to provide a more helpful log entry.

Steps to reproduce:

1. Install devstack and enable federation.
2. Uninstall the xmlsec1 package
3. Try to authenticate via federation and you'll get a HTTP 500 error and the corresponding log entry in keystone.log

[wangxiyuan (wangxiyuan)]

Not sure what error code is better.

For the error message, it describes as 'Unable to sign SAML assertion. It is likely that this server does not have xmlsec1 installed or this is the result of misconfiguration. Reason %(reason)s.' It's clear enough. So maybe we should not override the SAMLSigningError error message for the OSError.

[Guang Yee (guang-yee)]

I agree we should not be changing the API return code. It should be 500 for server misconfiguration. However, from support perspective, we need something that is actionable from the log file.

"[Errno 2] No such file or directory"

is unhelpful because it does not indicate which file or direction is missing. As suggested, perhaps we can add a check prior to line 421 in idp.py? i.e.

if not (os.path.isfile(CONF.saml.xmlsec1_binary) and os.access(CONF.saml.xmlsec1_binary, os.X_OK)):
    msg = ('Misconfiguration detected. %s is either missing or not an executable. Please check to make sure xlmsec1_binary in the [saml] section is properly configured.', CONF.saml.xmlsec1_binary)
    LOG.error(msg)
    raise exception.SAMLSigningError(reason=msg)

[Colleen Murphy (krinkle)]

Sounds good to me, either of you want to propose that patch?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/553592

[Lance Bragstad (lbragstad)]

Guang,

I posted a quick fix [0] but it needs a test and or verification. Does the patch give you better logging when you recreate the issue?

[0] https://review.openstack.org/#/c/553592

[Lance Bragstad (lbragstad)]

Guang,

Just for completeness, did you have a specific set of steps that you used for enabling federation with devstack? I thought we had a guide somewhere but I'm failing to find it. If we do any steps somewhere, I was going to take a shot at incorporating them.

[Guang Yee (guang-yee)]

Lance, I have not tested federation on devstack. We have our own ansible playbook to setup Federation.

Looking at the Keystone code, I presume we'll just need to add these lines to local.conf?

# local.conf
enable_plugin keystone git://git.openstack.org/openstack/keystone.git
enable_service keystone-saml2-federation

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/553592
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ccdf2d976f4d26df4f6a2a915da6ff0f643757ac
Submitter: Zuul
Branch: master

commit ccdf2d976f4d26df4f6a2a915da6ff0f643757ac
Author: Lance Bragstad <email address hidden>
Date: Thu Mar 15 19:39:43 2018 +0000

    Add logging for xmlsec1 installation

    Keystone uses a library called xmlsec1 to create SAML assertions when
    acting as an identity provider. If this library isn't present and
    someone attempts to authenticate, keystone will throw an HTTP 500.
    The only thing the error says is that a file or directory doesn't
    exist.

    This patch uses subprocess to check if the provided binary actually
    exists on the system and handles cases when it isn't and logs a
    useful message for operators.

    Change-Id: I41cf87702df5389c1424d35f0abcef9c16301450
    Closes-Bug: 1750917

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 14.0.0.0b1 development milestone.