[Patch] Fix the ";binary" suffix behavior

[Jeremy Allison]

On Thu, Aug 11, 2016 at 12:26:34AM +0200, Niklas Abel wrote:
> 
> 
> On 08/10/2016 11:01 AM, Simo Sorce wrote:
> > On Tue, Aug 09, 2016 at 11:32:31AM -0700, Jeremy Allison wrote:
> >> On Tue, Aug 09, 2016 at 06:08:04AM +0200, Volker Lendecke wrote:
> >>> On Mon, Aug 08, 2016 at 05:13:59PM -0700, Jeremy Allison wrote:
> >>>> On Mon, Aug 08, 2016 at 06:00:00PM +0200, Niklas Abel wrote:
> >>>>
> >>>>> No problem, thanks for the review and the fast response.
> >>>> OK - LGTM. Can I get a second Team reviewer ?
> >>> Attached find a set of cosmetic patches on top. Feel free to comment &
> >>> squash.
> >>>
> >>> On the semantics -- RFC4522 describes what this is supposed to do. Can
> >>> we really just ignore this?
> >> Here is the squashed patchset. Team - please don't push until
> >> we get clarification on if this is correct w.r.t AD and RFC4522.
> > RFC4522 requires that attributes that carry the ;binary tag be returned in
> > binary form (whatever that means for the specific syntax).
> > Also the comment of the commit is utterly void of useful information.
> > The author should explain why the commit is needed (normative rferences, test
> > cases, uses cases, or whatever else) and why the specific implementation path
> > was chosen if that may be controversial.
> >
> > This commit looks incomplete in that it does not assure the values are returned
> > in the appropriate form, but it is possible that no transformation is needed if
> > we always store them unmodified. A test that insures usercertificate is stored
> > and returned in the correct format is needed at the very minimum.
> >
> > Simo.
> >  
> The patch is a result of the following conversation:
> 
> https://lists.samba.org/archive/samba/2015-November/196049.html
> 
> It will be used to share S/MIME certificates.
> 
> My test setup is a Debian based VM as a server.
> The test call is:
> ldapsearch -H ldap://127.0.0.1 -U Administrator at X.LOCAL -b
> "OU=testou,DC=X,DC=LOCAL" -LLL "userCertificate;binary"
> 
> which should result with the certificate as same as:
> ldapsearch -H ldap://127.0.0.1 -U Administrator at X.LOCAL -b
> "OU=testou,DC=X,DC=LOCAL" -LLL "userCertificate"
> 
> It works fine with the written patch.

Yeah, I think Simo is right though. From RFC4522:

"Where the binary option is present in an attribute description, the
   associated attribute values or assertion values MUST be BER encoded
   (otherwise the values are encoded according to the LDAP-specific
   encoding [RFC4517] for the attribute's syntax).  Note that it is
   possible for a syntax to be defined such that its LDAP-specific
   encoding is exactly the same as its BER encoding.

   In terms of the protocol [RFC4511], the binary option specifies that
   the contents octets of the associated AttributeValue or
   AssertionValue OCTET STRING are a complete BER encoding of the
   relevant value."

So we do need to ensure that LDAP returns when ;binary
is detected are BER encoded, and we're not currently
doing that in this patch.