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

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.