URL: https://github.com/freeipa/freeipa/pull/298
Title: #298: ipaldap: handle binary encoding option transparently

jcholast commented:
>  If `ipaldap` is a generic LDAP client, it should obey the RFCs and always 
> transfer the relevant attributes (`userCertificate`, `cACertificate`, etc) 
> with the `;binary` encoding option, and it should expect to see it when 
> reading the relevant attributes from the server.

No, it should respect whatever is defined on the server, otherwise it's not a 
generic LDAP client. If the server does something wrong, it has to be fixed 
there, on the server. The goal of `ipaldap` is not to make buggy or non-LDAPv3 
(e.g. AD) servers look like they are LDAPv3-compliant, the goal is to interpret 
attributes according to the server-defined schema.

> IMO `ipaldap` should handle this transparently because it is part of the LDAP 
> protocol.

Nowhere in the RFCs is it mandated that a compliant client cannot request the 
attributes without the option, nor that it must not accept the attributes 
without the option in server responses. If this was true, it would have to be 
fixed in OpenLDAP libs anyway, not in `ipaldap`.

> There is no 389DS-specific hack in my proposed change (but I'm curious about 
> what part of it you feel is).

The part where you implicitly add the binary transfer option to attribute names 
(although not mandated on clients by any RFC) without knowing how the attribute 
types are defined on the server (although mandated only on attribute types with 
the certificate syntax by RFC 4523) .

> This would also avoid inconsistent handling of relevant attributes between 
> different plugins, which is the situation we currently have.

This is because of historical reasons (the original implementation of `host` 
and `service` plugins used `userCertificate` instead of 
`userCertificate;binary`) and will have to stay this way at least until all of 
the buggy 389 DS / IPA releases go out of support.

> But apart from the inconsisency (which is a nusiance) we have a bigger 
> problem - in several plugins we specifically try to read `userCertificate`, 
> but a RFC 4522 compliant server (which 389DS is not now, but hopefully one 
> day will be) will always return `userCertificate;binary`. So, our current 
> code breaks if/when that happens. Furthermore, other RFC 4522-compliant 
> programs that correctly use the ;binary transfer encoding option to, e.g. 
> write certificates to user entries, will cause those certificates to be 
> unreadable by current IPA plugin code. This is not good enough.

We can easily fix the plugins to read from `userCertificate;binary` in addition 
to `userCertificate`. We have to continue to write to `userCertificate` only 
though, because of backward compatibility with older servers.

> 389DS does not behave correctly; it's treatment of `;binary` is wrong in 
> several ways, apart from the incorrect attribute syntax for relevant 
> attributes.

Not enforcing `;binary` on attribute types with octet string syntax *is* 
correct. I was not trying to imply anything else.

See the full comment at 
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to