Bug 44966 - Improve error handling if setting "umc/saml/idp-server" fails
Improve error handling if setting "umc/saml/idp-server" fails
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: SAML
UCS 4.2
Other Linux
: P5 normal (vote)
: UCS 4.2-1-errata
Assigned To: Jürn Brodersen
Florian Best
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-11 13:54 CEST by Jürn Brodersen
Modified: 2017-07-28 14:28 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments
proposed patch (3.82 KB, patch)
2017-07-12 15:41 CEST, Jürn Brodersen
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jürn Brodersen univentionstaff 2017-07-11 13:54:52 CEST
If downloading of the metadata from the idp fails, the 404 message is saved into:
/usr/share/univention-management-console/saml/idp/ucs-sso.univention.intranet.xml

The saml login now fails with a parsing error. If possible we should catch that error somehow and tell the admin what to do: check the value from umc/saml/idp-server and set again if the connection is fixed.
Comment 1 Florian Best univentionstaff 2017-07-11 14:23:30 CEST
Yes, I thought the same some times already.
Could you write a short patch which checks the HTTP error status to be 200 and validate the downloaded file to be valid XML syntax?
Comment 2 Jürn Brodersen univentionstaff 2017-07-12 15:41:48 CEST
Created attachment 9023 [details]
proposed patch

I added some logging to saml/sp.py to make debugging easier.

I also added logging for SamlErrors inside univention-management-console-web-server because these errors there often only visible on the saml iframe which is not shown if any errors appear. Or is that to much information?
Comment 3 Florian Best univentionstaff 2017-07-20 14:35:22 CEST
As discussed, please apply parts of the patch.
Comment 4 Jürn Brodersen univentionstaff 2017-07-20 16:29:21 CEST
r81297: YAML
r81296: Improve error handling if setting "umc/saml/idp-server" fails
Package: univention-management-console
Version: 9.0.80-56A~4.2.0.201707201612
Branch: ucs_4.2-0
Scope: errata4.2-1
Comment 5 Jürn Brodersen univentionstaff 2017-07-20 17:38:21 CEST
r81299: Changed logging for samlErrors
r81300: YAML
Package: univention-management-console
Version: 9.0.80-57A~4.2.0.201707201731
Branch: ucs_4.2-0
Scope: errata4.2-1
Comment 6 Florian Best univentionstaff 2017-07-21 15:53:02 CEST
OK: error handling in the UCR module
OK: syntax validation
OK: Now we are hitting Bug #39268.
Module: setup_saml_sp
Try to download idp metadata (1/60)
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
E: your request could not be fulfilled
try `univention-config-registry --help` for more information

REOPEN: This introduces a vulnerability, please use defusedxml for the syntax validation.
Comment 7 Jürn Brodersen univentionstaff 2017-07-24 10:27:47 CEST
r81323: Use defusedxml instead of xml.etree
Package: univention-management-console
Version: 9.0.80-58A~4.2.0.201707241022
Branch: ucs_4.2-0
Scope: errata4.2-1


The dependencies should be ok:
univention-management-console-web-server depends on
python-pysaml2 depends on
python-defusedxml
Comment 8 Jürn Brodersen univentionstaff 2017-07-24 11:12:56 CEST
r81330: Added python-defusedxml dependency
Package: univention-management-console
Version: 9.0.80-59A~4.2.0.201707241109
Branch: ucs_4.2-0
Scope: errata4.2-1
Comment 9 Florian Best univentionstaff 2017-07-24 13:58:10 CEST
OK: defusedxml
OK: dependency
Comment 10 Erik Damrose univentionstaff 2017-07-26 14:39:40 CEST
<http://errata.software-univention.de/ucs/4.2/102.html>
Comment 11 Philipp Hahn univentionstaff 2017-07-28 14:26:20 CEST
# cat /usr/share/univention-management-console/saml/idp/ucs-sso.phahn.dev.xml 
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /saml-bin/php-cgi/simplesamlphp/saml2/idp/metadata.php was not found on this server.</p>
<hr>
<address>Apache/2.4.10 (Debian) Server at ucs-sso.phahn.dev Port 443</address>
</body></html>

# less /var/log/daemon.log
Jul 28 14:06:23 dc0 systemd[1]: Starting LSB: OpenLDAP standalone server (Lightweight Directory Access Protocol)...
Jul 28 14:06:23 dc0 slapd[24809]: Starting ldap server(s): slapd ...
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-CRITICAL **: libxml2: Space required after the Public Identifier\n
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-CRITICAL **: libxml2: SystemLiteral \" or ' expected\n
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-CRITICAL **: libxml2: SYSTEM or PUBLIC, the URI is missing\n
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-CRITICAL **: libxml2: Opening and ending tag mismatch: hr line 7 and body\n
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-CRITICAL **: libxml2: Opening and ending tag mismatch: body line 4 and html\n
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-CRITICAL **: libxml2: Premature end of data in tag html line 2\n
Jul 28 14:06:23 dc0 slapd[24809]: (process:24821): Lasso-WARNING **: 2017-07-28 14:06:23#011Cannot load metadata from /usr/share/univention-management-console/saml/idp/ucs-sso.phahn.dev.xml
Jul 28 14:06:23 dc0 slapd[24809]: done.
Jul 28 14:06:23 dc0 s44966lapd[24809]: Checking Schema ID: ...done.


Don't let `curl` (or whatever) put an error message in that file!

Fixed it by `ucr set "umc/saml/idp-server=$(ucr get umc/saml/idp-server)"`
Comment 12 Florian Best univentionstaff 2017-07-28 14:28:13 CEST
(In reply to Philipp Hahn from comment #11)
Yes, that is what we fixed now!?!