[Pki-devel] [PATCH] Added fix for pki-server for db-update

[Fraser Tweedale]

On Thu, Jul 14, 2016 at 06:01:51PM +1000, Fraser Tweedale wrote:
> On Thu, Jul 14, 2016 at 01:05:18PM +0530, Geetika Kapoor wrote:
> > 
> > 
> > On 07/14/2016 11:38 AM, Geetika Kapoor wrote:
> > >
> > >
> > > On 07/14/2016 10:06 AM, Fraser Tweedale wrote:
> > >> On Wed, Jul 13, 2016 at 04:36:26PM +0530, Geetika Kapoor wrote:
> > >>> Hi,
> > >>>
> > >>> Please review this patch.Below is a small summary about this fix and
> > >>> what we are trying to achieve.
> > >>>
> > >>> CLI :  pki-server db-upgrade
> > >>>
> > >>> what it should be doing is if it sees that issuerName doesn't exist,NULL
> > >>> it will add it itself.
> > >>>
> > >>> Operation 1 : Search for the empty cn value for issuerName
> > >>> -------------------------------------------------------------------------------
> > >>>
> > >>> Current :   '(&(objectclass=certificateRecord)(issuerName=*))  -- I
> > >>> tried this it didn't show data even if i have record with empty issuerName
> > >>>
> > >> Hi Geetika,
> > >>
> > >> The current filter is actually:
> > >>
> > >>   '(&(objectclass=certificateRecord)(!(issuerName=*)))',
> > >>
> > >> This should match entries missing the issuerName attribute.  You
> > >> talk about an entry with "empty issuerName" but empty strings are
> > >> not allowed for the Directory String attribute type.  Could you
> > >> please clarify exactly what data is in the offending entry/entries
> > >> and how it got there?
> > > Hi Fraser,
> > >
> > > If we disable syntax check in ldap dse.ldif , it will accept empty
> > > data as well.So if a end user disable syntax check,issuerName can be
> > > empty in that case.(a test case that i tried)
> > > So in that case db-update will never happen because that condition is
> > > not considered.This scenario can be reproduced using below ldif file.
> > >
> > > <file>
> > >
> > > dn: cn=106,ou=certificateRepository,ou=ca,o=pkitest-CA
> > > objectClass: certificateRecord
> > > objectClass: top
> > > cn: 106
> > > algorithmId: 1.2.840.113549.1.1.1
> > > autoRenew: ENABLED
> > > certStatus: VALID
> > > dateOfCreate: 20160712084443Z
> > > dateOfModify: 20160712084443Z
> > > duration: 1131536000000
> > > issuedBy:   geetika20
> > > *issuerName:     *  
> > > metaInfo: requestId:100
> > > notAfter: 20170712084205Z
> > > notBefore: 20160712084205Z
> > > publicKeyData::
> > > MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAu0Hlk6SdMnyr0Igq
> > > serialno: 100
> > > signingAlgorithmId: 1.2.840.113549.1.1.11
> > > subjectName: CN=CS Administrator,C=US
> > > userCertificate;binary::
> > > MIIC6DCCAdCgAwIBAgIBBzANBgkqhkiG9w0BAQsFADBHMSQwIgY
> > > version: 2
> > >
> > > </file>
> > >
> > > So in such a case using
> > > '(&(objectclass=certificateRecord)(!(issuerName=*)))',will not able to
> > > search for such entries.I tried and it gives me empty data .I believe
> > > using (&(objectclass=certificateRecord)
> > > (!(issuerName=*))(!(issuerName=cn*))) can solve that purpose.
> > >
> > > Thanks
> > > Geetika
> > Hi Frazer,
> > 
> > I just did one quick round of testing .If we have
> > '(&(objectclass=certificateRecord)(!(issuerName=cn*)))', it will work in
> > both cases :
> > 
> > 1. When issuerName doesn't exist.
> > 2. When issuserName field exist but has empty value.
> > 
> > Thanks
> > Geetika
> >
> I still disagree that it is the right approach, because it may do
> unnecessary work for records that already have an issuerName that
> does not start with "cn".
> 
> Is it even necessary to support cases where customer has disabled
> syntax checking?  Nevertheless, let me disable syntax checking on
> one of my instances and see if I can find a better filter.
> 
Please try this filter:

    (&(objectclass=certificaterecord)(|(!(issuername=*))(issuername=)))

It will find only certificates with missing or empty issuername
attribute.  Does it work as expected for you, Geetika?

> 
> > >>> Modified :  (&(objectclass=certificateRecord)(!(issuerName=cn*)))'   --
> > >>> This solves the purpose as it shows all the certs without issuerName
> > >>>
> > >> This filter is wrong - it does match entries without issuerName (as
> > >> intended), but also matches entries with issuerName set but not
> > >> starting with "cn".
> > >>
> > >>> Operation 2 : If we see a empty cn value , we are replacing it with
> > >>> value we get from code
> > >>> ------------------------------------------------------------------------------------------------------------------
> > >>> < code>
> > >>>
> > >>> cert = nss.Certificate(bytearray(attr_cert[0]))
> > >>>         issuer_name = str(cert.issuer)
> > >>>
> > >>> </code>
> > >>>
> > >>> Current : we are updating the list it the format as mentioned 
> > >>> 'issuerName': ['', 'CN=CA Signing Certificate,O=example.com Security
> > >>> Domain']
> > >>>
> > >>> Do we want to keep this behavior or we want to overwrite it in first
> > >>> place? I believe in place of we do it MOD_REPLACE.
> > >>>
> > >>> <try:
> > >>>             conn.ldap.modify_s(dn, [(ldap.MOD_ADD, 'issuerName',
> > >>> issuer_name)])
> > >>> Modified : onn.ldap.modify_s(dn, [(ldap.MOD_REPLACE, 'issuerName',
> > >>> issuer_name)])
> > >>>
> > >> This change is OK.
> > >
> >