ldap:AttributeAddFromLDAP cannot be used to add pictures

[GoogleCodeExporter]

What steps will reproduce the problem?
1. On the IdP, set up ldap:AttributeAddFromLDAP to add "jpegPhoto" from LDAP 
(in our case, AD)
2. Attempt to log in on an SP

What is the expected output? What do you see instead?
The SP gives me the error "Exception: Error parsing XML string.". Debugging 
shows the following (formatted for readability):
<saml:Attribute Name="jpegPhoto" 
NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
    <saml:AttributeValue xsi:type="xs:string">ÿØÿà</saml:AttributeValue>
</saml:Attribute> 
The ÿØÿà part is FF D8 FF E0 in HEX.
Expected is of course valid XML in the SAML assertion

What version of the product are you using? On what operating system?
simpleSamlPhp 1.11.0 on both SP and IdP

Please provide any additional information below.
When not using AttributeAddFromLDAP, but directly including "jpegPhoto" in the 
LDAP authsource, this problem does not occur.

Original issue reported on code.google.com by yorndej...@gmail.com on 11 Feb 2014 at 12:20

[GoogleCodeExporter]

Hi Yorn!

The problem here is that AttributeAddFromLDAP uses to fetch the attributes a 
different method from the LDAP base class than the standard authentication 
procedure. While the latter uses SimpleSAML_Auth_LDAP::getAttributes(), the 
former uses SimpleSAML_Auth_LDAP::searchfrommultiple(), and this one doesn't 
perform any kind of base64 encoding on the values of the attributes 
(specifically for jpeg photos).

Could you please apply the following patch and check if it works for you?

Original comment by jaim...@gmail.com on 18 Feb 2014 at 2:35

• Changed state: Started

Attachments:

• AttributeAddFromLDAP_620.diff

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Original comment by jaim...@gmail.com on 19 Feb 2014 at 12:18

• Added labels: Milestone-Release1.12
• Removed labels: Milestone-Release2.0

[GoogleCodeExporter]

It works! It also works when using the Authsource directly (so I found no 
regression problems).

But is hardcoding "jpegPhoto" as attribute name really the best solution?

Original comment by yorndej...@gmail.com on 20 Feb 2014 at 12:13

[GoogleCodeExporter]

Good!

You are right, hardcoding the attribute name is far from optimal or good 
practice, but it's just the way it was in getAttributes(). It's a difficult 
problem, because the proper way to solve this would be to take the LDAP schema 
into account and use it to determine if you need to base64-encode the values or 
not. But that's really difficult, so I've tried a different approach: detect if 
the bytes in the string are "printable", and if not, encode it in base64.

Can you tell me if the patch attached works equally good for you, and for any 
type of attribute, not just jpeg pictures?

Original comment by jaim...@gmail.com on 20 Feb 2014 at 3:41

Attachments:

• AttributeAddFromLDAP_620_2.diff

[GoogleCodeExporter]

Ok, I've verified myself that it works, so I've applied the same simple fix to 
the code in getAttributes(). Available in r3368.

Original comment by jaim...@gmail.com on 20 Feb 2014 at 7:11

• Changed state: Fixed

[GoogleCodeExporter]

Actually... no, it doesn't. It works for pictures, but it also "works" for 
norgewian characters, so attributes that are pure plain text get 
base64-encoded. Apparently, ctype_print() thinks an 'ø' is not printable.

Anyway, I've reverted the change, and applied it again with the 'jpegPhoto' 
hardcoded in r3370.

Original comment by jaim...@gmail.com on 20 Feb 2014 at 8:38

[GoogleCodeExporter]

You could still look at http://php.net/mb_detect_encoding, but I don't think 
this is the way to go: If you assume that a non-printable string is a picture, 
what about for example public keys?

Nevertheless, hardcoding attribute names works for me. Maybe 2.0 could look at 
the schema?

Original comment by yorndej...@gmail.com on 21 Feb 2014 at 11:10