Don't store entries with a usercertificate in the LDAP cache

[rcritten]

Don't store entries with a usercertificate in the LDAP cache

usercertificate often has a subclass and both the plain and
subclassed (binary) values are queried. I'm concerned that
they are used more or less interchangably in places so not
caching these entries is the safest path forward for now until
we can dedicate the time to find all usages, determine their
safety and/or perhaps handle this gracefully within the cache
now.

What we see in this bug is that usercertificate;binary holds the
first certificate value but a user-mod is done with
setattr usercertificate=<new_cert>. Since there is no
usercertificate value (remember, it's usercertificate;binary)
a replace is done and 389-ds wipes the existing value as we've
asked it to.

I'm not comfortable with simply treating them the same because
in LDAP they are not.

https://pagure.io/freeipa/issue/8986

Signed-off-by: Rob Crittenden rcritten@redhat.com

[rcritten]

Fixed linter issue.

[fcami]

LGTM. Not Acking formally yet.

[frasertweedale]

Well... actually how 389DS treats the ;binary option is not correct. It treats it as a subtype, when
in fact it should not. See https://datatracker.ietf.org/doc/html/rfc4522. I vaguely recall other FreeIPA
and/or Dogtag issues due to this quirk.

The workaround in this PR looks fine to me.

[tbordaz]

Well... actually how 389DS treats the ;binary option is not correct. It treats it as a subtype, when
in fact it should not. See https://datatracker.ietf.org/doc/html/rfc4522. I vaguely recall other FreeIPA
and/or Dogtag issues due to this quirk.

The workaround in this PR looks fine to me.

Hi @frasertweedale, 389-ds transfers all returned attributes following BER encoding form. Do you mean 389ds should transfert certificates attributes (https://datatracker.ietf.org/doc/html/rfc4523) in DER encoding from ?

[frasertweedale]

G'day @tbordaz. What I mean is that 389 treats ;binary as an attribute subtype, rather than a transport option. That is, it happily accepts and stores a userCertificate attribute (without the ;binary transfer option), and returns it in the same way. This violates https://datatracker.ietf.org/doc/html/rfc4523 which says that attributes having the various certificate-related syntaxes MUST only be transfered using the ;binary transfer option.

Furthermore, RFC 4522 states:

 The binary option is not a tagging option [RFC4512], so the presence
 of the binary option does not specify an attribute subtype.  An
 attribute description containing the binary option references exactly
 the same attribute as the attribute description without the binary
 option.  The supertype/subtype relationships of attributes with
 tagging options are not altered in any way by the presence or absence
 of the binary option.

But 389DS treats ;binary as an attribute subtype tag. It leads to
bugs like this one this PR addresses.

[tbordaz]

G'day @tbordaz. What I mean is that 389 treats ;binary as an attribute subtype, rather than a transport option. That is, it happily accepts and stores a userCertificate attribute (without the ;binary transfer option), and returns it in the same way. This violates https://datatracker.ietf.org/doc/html/rfc4523 which says that attributes having the various certificate-related syntaxes MUST only be transfered using the ;binary transfer option.

Furthermore, RFC 4522 states:

 The binary option is not a tagging option [RFC4512], so the presence
 of the binary option does not specify an attribute subtype.  An
 attribute description containing the binary option references exactly
 the same attribute as the attribute description without the binary
 option.  The supertype/subtype relationships of attributes with
 tagging options are not altered in any way by the presence or absence
 of the binary option.

But 389DS treats ;binary as an attribute subtype tag. It leads to
bugs like this one this PR addresses.

I agree, the handling ';binary' is not conform. it is a transfer syntax not a subtype.

[rcritten]

@frasertweedale so is that an ack? :-)

[frasertweedale]

@fcami what do you think? I am not familiar with this new LDAP cache feature, but the patch makes sense to me.

[fcami]

ACK

[rcritten]

temp commit removed

[rcritten]

master:

• ba526c5 Don't store entries with a usercertificate in the LDAP cache
• 540b01b ipatests: Test that a user can be issued multiple certificates