Re: [Pki-devel] [PATCH]pki-cfu-0149-Ticket-2246-MAN-Man-Page-AuditVerify.patch

2016-07-14 Thread Endi Sukma Dewata

On 7/12/2016 8:27 PM, Christina Fu wrote:

man page for AuditVerify

https://fedorahosted.org/pki/ticket/2246


Some comments/questions:

1. I think the -P option would unlikely be used. Can we remove this 
option in the future?


2. In the description for the -a option, there's a missing space before 
the left parenthesis:


  ... paths(in chronological order) ...

3. Do we assume the auditor to have an access to the machine running the 
PKI server? Does the auditor have a read access to the files in the 
instance folder?


4. Normally the server does not export the system certificate into 
files, so the admin has to do that before the auditor can import the 
file with this command:


  certutil -d ~jsmith/auditVerifyDir/ -A -n "CA Certificate" -t
  "CT,CT,CT" -a -i /var/lib/instance_ID/alias/cacert.txt

I think we should replace the path with "-i cacert.txt". Here we're 
assuming the auditor already has the certificate file.


5. Similarly, the path to the audit certificate file should be replaced 
with "-i logsigncert.txt":


  certutil -d ~jsmith/auditVerifyDir -A -n "Log Signing Certificate"-t
  ",,P" -a -i /var/lib/instance_ID/alias/logsigncert.txt

6. There should be a space before the -t in #5.

7. The following phrase assumes the auditor has a write access to 
/etc/audit, is that the case? Or do we expect someone else to prepare 
the file for the auditor?


  ... this file could be logListFile in the /etc/audit directory ...

8. The database path in the description does not match the command:

  ... in the user home directory, such as /home/smith/.mozilla, ...

  AuditVerify -d ~jsmith/auitVerifyDir ...

9. The "auditVerifyDir" is misspelled in #8.

10. When viewed using the man tool, the quotes surrounding 
"auditsigningcert" disappear causing an extra space before the comma:


  ... and the signing certificate nickname is auditsigningcert , ...

11. The "auditsigningcert" nickname is inconsistent with the "Log 
Signing Certificate" used in #5.


12. The explanation for the verification failure in the following ticket 
is not included yet:

https://fedorahosted.org/pki/ticket/2217

Is it going to be added in a separate patch?

--
Endi S. Dewata

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


Re: [Pki-devel] [pki-devel][PATCH] 0076-MAN-Apply-generateCRMFRequest-removed-from-Firefox-w.patch

2016-07-14 Thread John Magne
Conditionally ACKED by cfu.
She wanted me to test the new ECC signing cert only profile I added:

Test was a success.

Pushed to master
Closing ticket #1285


Also release note bug on how to use the new profiles here:
https://bugzilla.redhat.com/show_bug.cgi?id=1355849

- Original Message -
From: "John Magne" 
To: "pki-devel" , pki-devel@redhat.com
Cc: c...@redhat.com
Sent: Thursday, July 14, 2016 11:42:36 AM
Subject: [pki-devel][PATCH] 
0076-MAN-Apply-generateCRMFRequest-removed-from-Firefox-w.patch

 [MAN] Apply 'generateCRMFRequest() removed from Firefox'
 workarounds to appropriate 'pki' man page

Ticket #1285 

This fix will involve the following changes to the source tree.

1. Fixes to the CS.cfg to add two new cert profiles.
2. Make the caDualCert.cfg profile invisible since it has little chance of
working any more in Firefox.
3. Create caSigningUserCert.cfg and caSigningECUserCert.cfg to allow the CLI
to have convenient profiles from which to enroll signing ONLY certificates.

To go along with this I have filed a downstream release note bug that shows 
exactly how to
deploy the new  profile to separately create one signing cert and one 
encryption cert (with archival),
which allows one to accomplish what the formater caDualCert profile used to do 
when Firefox supported it.

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


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

2016-07-14 Thread Endi Sukma Dewata

On 7/14/2016 6:51 AM, Fraser Tweedale wrote:

You're welcome.  ACK, and pushed to master:

c3ff087bd07cde4cd272defad499fd4d8367e5c1


I added this commit into this ticket to ensure it's included in the next 
build:

https://fedorahosted.org/pki/ticket/2399

--
Endi S. Dewata

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


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

2016-07-14 Thread Fraser Tweedale
On Thu, Jul 14, 2016 at 03:51:18PM +0530, Geetika Kapoor wrote:
> 
> 
> On 07/14/2016 03:02 PM, Geetika Kapoor wrote:
> >
> > 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.
> >
> > 
> >
> > 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: 113153600
> > 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
> >
> > 
> >
> > 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
> 
> Thanks Frazer for helping in giving a better solution .
>
You're welcome.  ACK, and pushed to master:

c3ff087bd07cde4cd272defad499fd4d8367e5c1

___
Pki-devel mailing list
Pki-devel@redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel


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

2016-07-14 Thread Geetika Kapoor


On 07/14/2016 03:02 PM, Geetika Kapoor wrote:
>
> 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.
>
> 
>
> 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: 113153600
> 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
>
> 
>
> 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

Thanks Frazer for helping in giving a better solution .
>>> 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)
>>>
>>> 
>>>
>>> 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 

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

2016-07-14 Thread 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.

 

 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: 113153600
 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

 

 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)
>>
>> 
>>
>> 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.
>>
>> > conn.ldap.modify_s(dn, [(ldap.MOD_ADD, 'issuerName',
>> issuer_name)])
>> Modified :