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

[Geetika Kapoor]

On 07/14/2016 01:53 PM, Fraser Tweedale wrote:
> 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?

Let me try Frazer..

Thanks
>
>>>>>> 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.