Bug 39355 - pam_saml/umc segfault if IDP metadata file does not exists
pam_saml/umc segfault if IDP metadata file does not exists
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: SAML
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.2-1-errata
Assigned To: Jürn Brodersen
Florian Best
https://github.com/univention/crudesa...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-09-16 15:16 CEST by Florian Best
Modified: 2017-08-09 16:57 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 7: Crash: Bug causes crash or data loss
Who will be affected by this bug?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 5: Blocking further progress on the daily work
User Pain: 0.200
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2015-09-16 15:16:02 CEST
The pam stack of univention-management-console contains pam_saml entry, which looks like the following:

auth sufficient pam_saml.so grace=600 userid=urn:oid:0.9.2342.19200300.100.1.1 idp=/usr/share/univention-management-console/saml/idp/backup502.deadlock50.intranet.xml idp=/usr/share/univention-management-console/saml/idp/master501.deadlock50.intranet.xml trusted_sp=https://master501.deadlock50.intranet/umcp/saml/metadata

If one of the referenced files in the idp= parameter does not exists (or no read permissions exists) the pam module (python-pam?) segfaults during authentication.

As the configuration is generated by a UCR template which checks the existence of every file this should not occur (often). A manual removal is required. Nevertheless it causes a segfault in the UMC-Server.

pam_saml.c
141 »   »   if (gctx->uid_attr != NULL) {
142 »   »   »   free((void *)gctx->uid_attr);

#0  0x00007ffff700f769 in free () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff6290e07 in gctx_cleanup (pamh=<optimized out>,
data=0x7fffffffd9b8, error=<optimized out>) at pam_saml.c:142
#2  0x00007ffff62915b4 in pam_global_context_init (av=0x999aa0,
ac=10069542, pamh=0x95a660) at pam_saml.c:335
#3  pam_sm_authenticate (pamh=0x95a660, flags=<optimized out>,
ac=10069542, av=0x999aa0) at pam_saml.c:457
#4  0x00007ffff669a224 in ?? () from /lib/x86_64-linux-gnu/libpam.so.0
#5  0x00007ffff6699a5f in pam_authenticate () from
/lib/x86_64-linux-gnu/libpam.so.0

I checked that it's not a double free.
linking pam-saml with efence (http://linux.die.net/man/3/efence) seems not to help much in debugging: the segfault is gone then but authentication fails ignoring the correct credentials.
I checked in a patch which at least ignores the IDP if there are multiple IDP's set up to reduce the risk.
Comment 1 Florian Best univentionstaff 2015-12-11 19:26:56 CET
The same applies to cy2saml if one changes the metadata.xml file without restarting slapd.
Changing the file is necessary if one e.g. renews the SSL certificates.

In that case slapd segfaulted:
Dec 11 19:21:20 backup31 kernel: [221738.417586] slapd[19552]: segfault at f ip 00007fc82d063d82 sp 00007fc7a9aad220 error 4 in libxmlsec1.so.1.2.18[7fc82d021000+5e000]
Comment 3 Jürn Brodersen univentionstaff 2017-07-31 11:26:24 CEST
Comment on attachment 9066 [details]
proposed patch

The updated patch can be found on:
https://github.com/univention/crudesaml/pull/3

r81546: add test 82_saml/25_broken_idp_metadata
Comment 4 Jürn Brodersen univentionstaff 2017-07-31 13:28:24 CEST
r81560: Fix SEGV if no idp config was found
r81562: YAML
Package: crudesaml
Version: 1.8.0-2A~4.2.0.201707311310
Branch: ucs_4.2-0
Scope: errata4.2-1
Comment 5 Jürn Brodersen univentionstaff 2017-07-31 17:40:13 CEST
(In reply to Florian Best from comment #1)
> The same applies to cy2saml if one changes the metadata.xml file without
> restarting slapd.
> Changing the file is necessary if one e.g. renews the SSL certificates.
> 
> In that case slapd segfaulted:
> Dec 11 19:21:20 backup31 kernel: [221738.417586] slapd[19552]: segfault at f
> ip 00007fc82d063d82 sp 00007fc7a9aad220 error 4 in
> libxmlsec1.so.1.2.18[7fc82d021000+5e000]

I think that was fixed in bug 45042.

Reopened until the pull request is merged and the patch can be modified accordingly.
Comment 6 Florian Best univentionstaff 2017-08-01 18:23:20 CEST
(In reply to Jürn Brodersen from comment #5)
> (In reply to Florian Best from comment #1)
> > The same applies to cy2saml if one changes the metadata.xml file without
> > restarting slapd.
> > Changing the file is necessary if one e.g. renews the SSL certificates.
> > 
> > In that case slapd segfaulted:
> > Dec 11 19:21:20 backup31 kernel: [221738.417586] slapd[19552]: segfault at f
> > ip 00007fc82d063d82 sp 00007fc7a9aad220 error 4 in
> > libxmlsec1.so.1.2.18[7fc82d021000+5e000]
> 
> I think that was fixed in bug 45042.
> 
> Reopened until the pull request is merged and the patch can be modified
> accordingly.
No, the pull request is independent of this.
So you would say this Bug is a duplicate of Bug #45042?
Comment 7 Jürn Brodersen univentionstaff 2017-08-02 10:13:44 CEST
(In reply to Florian Best from comment #6)
> (In reply to Jürn Brodersen from comment #5)
> > (In reply to Florian Best from comment #1)
> > > The same applies to cy2saml if one changes the metadata.xml file without
> > > restarting slapd.
> > > Changing the file is necessary if one e.g. renews the SSL certificates.
> > > 
> > > In that case slapd segfaulted:
> > > Dec 11 19:21:20 backup31 kernel: [221738.417586] slapd[19552]: segfault at f
> > > ip 00007fc82d063d82 sp 00007fc7a9aad220 error 4 in
> > > libxmlsec1.so.1.2.18[7fc82d021000+5e000]
> > 
> > I think that was fixed in bug 45042.
> > 
> > Reopened until the pull request is merged and the patch can be modified
> > accordingly.
> No, the pull request is independent of this.
Ok set to resolved again :)

> So you would say this Bug is a duplicate of Bug #45042?
The part about the slapd segfault. As far as I can tell the umc and slapd segfaults were caused by different bugs. The umc is using pam and slapd is using sasl. At least the fix I committed is only relevant for the pam part.
umc segfault -> due to this bug 39355
slapd segfault (might have caused umc segfaults as well) -> due to bug 45042
Comment 8 Florian Best univentionstaff 2017-08-02 19:25:42 CEST
REOPEN as discussed.
Comment 9 Jürn Brodersen univentionstaff 2017-08-04 12:27:32 CEST
I changed the patch to only include the relevant change that fixes the segfault.

A test can be found under 82_saml/25_broken_idp_metadata.

r81786: Only patch what fixed the segfault
Package: crudesaml
Version: 1.8.0-3A~4.2.0.201708041146
Branch: ucs_4.2-0-errata4.2-1
Scope: errata4.2-1
Comment 10 Florian Best univentionstaff 2017-08-08 18:45:16 CEST
The test script reproduces the issue :-)

# univention-management-console-server -n -d4 -L /dev/stdout
…
08.08.17 18:43:08.801  AUTH        ( INFO    ) : Canonicalized username: 'Administrator'
*** Error in `/usr/bin/python2.7': free(): invalid pointer: 0x00007f5a94c7e094 ***
Abgebrochen
Comment 11 Florian Best univentionstaff 2017-08-08 18:58:12 CEST
OK: fix works, nice!
OK: YAML (adjusted in r81910)
Comment 12 Arvid Requate univentionstaff 2017-08-09 16:57:16 CEST
<http://errata.software-univention.de/ucs/4.2/124.html>