OpenConext / OpenConext-engineblock

OpenConext SAML 2.0 IdP/SP Gateway
14 stars 22 forks source link

Error on omitting Stepup-certificate (v5.13) #852

Closed tvdijen closed 4 years ago

tvdijen commented 4 years ago

This may already been covered by the migration from /etc/openconext/engineblock.ini to YAML config file, but in that case this issue may be kept for future reference to other people bumping into this issue. It probably exists as of 5.13 where the new stepup-functionality was added.

Case:

Composer install went fine, but on accessing EB from the browser in an authflow the following error occured:

May 14 20:16:25 sv1811042 EBLOG[11331]: [2020-05-14 20:16:25] app.DEBUG: Caught Exception "OpenConext\EngineBlock\Exception\InvalidArgumentException":"Keyfile '/etc/openconext/engineblock.crt' should be a valid file" {"session_id":"h7c9ljfn4q0k13erctj8ohsnt7","request_id":"5ebd8af9728c2"} []
May 14 20:16:25 sv1811042 EBLOG[11331]: [2020-05-14 20:16:25] engineblock.ERROR: Keyfile '/etc/openconext/engineblock.crt' should be a valid file | Caught Unhandled generic exception [..]

I'd argue that EB should be perfectly usable without any of the Stepup-stuff configured.. I eventually solved it by setting stepup.gateway.sfo.keyFile to an actual certificate.. The other stepup-settings are still left out and it's working fine now.

Perhaps changing the default for this setting to the value of encryption.keys.default.publicFile would be a solid fix?

pablothedude commented 4 years ago

Hi Tim, thanks for the report. I've created a PR with a fix for the described issue. https://github.com/OpenConext/OpenConext-engineblock/pull/853

Are you able to verify if this will work for you? I'll try to do my best to let this land in the next patch version of 6.2.

tvdijen commented 4 years ago

No bueno.. If left out from the configuration it will try and load /etc/openconext/engineblock.crt. It's still the same assertion failing, so that means it must be fed with some default value elsewhere.

pablothedude commented 4 years ago

Hi Tim, the validation is moved from the constructor completely. The validation now only happens JIT when an entity is configured to use the Stepup callout functionality. So if you don't configure it it won't get validated. This should help you and would suit the majority of the Openconext users.

https://github.com/OpenConext/OpenConext-engineblock/pull/853

I wanted to let you know this just landed in the latest release: https://github.com/OpenConext/OpenConext-engineblock/releases/tag/6.2.1

tvdijen commented 4 years ago

Great, thanks @pablothedude !

thijskh commented 4 years ago

Fixed in EB 6.2.1.