Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-28 Thread Jan Cholasta 

On 13.6.2016 08:59, Jan Cholasta wrote:

On 13.6.2016 08:38, Fraser Tweedale wrote:

On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:

On 9.6.2016 11:10, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:

On 9.6.2016 08:44, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:

On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and
managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying
target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.
Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority
id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created
twice - once in
41-subca.update, once using ensure_entry() call? It should be
done only
once.


I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.


2) In ca_del, every CA specified in args[0] should be deleted,
not just the
first one.

3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid?
I guess not,
so you should be able to remove the check altogether and don't
bother with
the warning.


ipacaid is mandatory now, so I'll remove the check.



Patch 0060-0062: LGTM


Yippee \o/



Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key
params, which
unfortunately we cannot leverage in cert_request currently.


No problemo.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show -
show commands
are supposed to retrieve an object given primary key(s), and
the primary key
of CA objects is just their cn.


The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.


Correct, but in IPA the issuer is represented by the CA object, so
in IPA
the primary key for a certificate is actually (CA name, serial).

Certificate lookup by issuer name and serial is actually a search
operation,
analogical to how CA lookup by subject name is also a search
operation, so
it should be done by cert-find.


OK, I will remove the --issuer option for cert-find.




3) In find commands, the options form a filter, so instead of
raising
MutuallyExclusiveError in cert-find, return an empty result, as
with any
other unmatched filter.


Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).


That's one way to look at it, but it's true only if you assume that
cert-find can only search certificates in Dogtag. This will very
soon became
untrue, as we will allow cert-find to also search certificates
anywhere in
LDAP (the server part of ticket #5381). There, the difference
between the
options would be that with --ca you search for certificates

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-15 Thread Fraser Tweedale 
On Wed, Jun 15, 2016 at 07:30:26AM +0200, Jan Cholasta wrote:
> On 15.6.2016 04:02, Fraser Tweedale wrote:
> > On Tue, Jun 14, 2016 at 03:21:24PM +0200, Martin Babinsky wrote:
> > > On 06/14/2016 04:55 AM, Fraser Tweedale wrote:
> > > > On Tue, Jun 14, 2016 at 02:19:27AM +1000, Fraser Tweedale wrote:
> > > > > On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:
> > > > > > > > > 
> > > > > > > > > Hi Fraser,
> > > > > > > > > 
> > > > > > > > > during functional review I found the following issues:
> > > > > > > > > 
> > > > > > > > > 1.)
> > > > > > > > > 
> > > > > > > > > If I create a CAACL rule tied to a specific sub-CA let's say 
> > > > > > > > > for user
> > > > > > > > > certificate issuance:
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > ipa caacl-show user_cert_issuance
> > > > > > > > >   Enabled: TRUE
> > > > > > > > >   User category: all
> > > > > > > > >   CAs: user_sub_ca
> > > > > > > > >   Profiles: caIPAuserCert
> > > > > > > > >   ACL name: user_cert_issuance
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > I can still happily request certificate for a user using 
> > > > > > > > > root-CA:
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > >  ipa cert-request cert.csr --principal jdoe --ca ipa
> > > > > > > > >   Certificate: MIID9j.../Ov8mkjFA==
> > > > > > > > >   Subject: CN=jdoe,O=IPA.TEST
> > > > > > > > >   Issuer: CN=Certificate Authority,O=IPA.TEST
> > > > > > > > >   ...
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > should not this be denied by CA-ACL rule?
> > > > > > > > > 
> > > > > > > > > The default IPA CAACL rule is like this:
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > ipa caacl-show hosts_services_caIPAserviceCert
> > > > > > > > >   Enabled: TRUE
> > > > > > > > >   Host category: all
> > > > > > > > >   Service category: all
> > > > > > > > >   Profiles: caIPAserviceCert
> > > > > > > > >   ACL name: hosts_services_caIPAserviceCert
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > so the default rule should not allow users to request certs 
> > > > > > > > > at all.
> > > > > > > > > 
> > > > > > > > Yes, these should be denied.  Looking into it.
> > > > > > > > 
> > > > > > > Were you using 'admin' account to request the cert?  admin has
> > > > > > > permission 'Request Certificate ignoring CA ACLs' via the
> > > > > > > 'Certificate Manager' privilege.
> > > > > > > 
> > > > > > > If so, please try again with less privileges (e.g. self-service as
> > > > > > > jdoe).
> > > > > > > 
> > > > > > 
> > > > > > You were right when I was requesting certs as user principals 
> > > > > > themselves
> > > > > > everything worked as expected.
> > > > > > 
> > > > > > Regarding usb-CAs not present on replica, both machines run the 
> > > > > > following
> > > > > > version of dogtag CA:
> > > > > > 
> > > > > > pki-ca-10.3.2-3.fc24.noarch
> > > > > > 
> > > > > > Here are the last 256 lines of the debug log:
> > > > > > 
> > > > > > http://paste.fedoraproject.org/378512/65828332
> > > > > > 
> > > > > Thanks for that.  It seems there are two issues.  The first one is:
> > > > > 
> > > > > Unable to read key retriever class from CS.cfg: Property
> > > > > features.authority.keyRetrieverClass missing value
> > > > > 
> > > > > This happens only on replica, until the first restart of Dogtag
> > > > > after installation.  I have attached a patch to fix it.
> > > > > 
> > > > > The second issue is:
> > > > > 
> > > > > Failed to update certificate
> > > > >   
> > > > > 
> > > > > This needs to be fixed in Dogtag, however, the error is not
> > > > > triggered if the key retrieval succeeds when the replica first
> > > > > observes the addition of a new CA.  The attached patch does help in
> > > > > this regard, so apply it and I hope it will see you through the rest
> > > > > of the functional testing of my patches... I hope.
> > > > > 
> > > > > Thank you for testing!
> > > > > 
> > > > > Cheers,
> > > > > Fraser
> > > > > 
> > > > Rebased patches attached (VERSION conflicts; no other changes).
> > > > 
> > > 
> > > Thanks Fraser,
> > > 
> > > the certificate issuance by sub-CA now works also on replica but I must
> > > first restart PKI for it to pick up stuff from master CA.
> > > 
> > > If I set up a replica first and then create a sub-CA on master Dogtag 
> > > picks
> > > this up and uses the new sub-CA and its key for signing without restart.
> > > 
> > > This seems to be pure Dogtag issue, however and I concluded that IPA side
> > > works ok.
> > > 
> > > If no one objects then I would give functional ACK to your patches and
> > > document/track the Dogtag issue in a separate ticket.
> 
> Pushed to master: f0915e61986f545ad9b282fa90a4b1d0538829c5
> 
> > > 
> > Thanks Martin,
> > 
> > The Dogtag ticket is https://fedorahosted.org/pki/ticket/2359.  Fix
> > is merged and will go out in 10.3.3 this week or early next week.
> 
> Once released,

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-14 Thread Jan Cholasta 

On 15.6.2016 04:02, Fraser Tweedale wrote:

On Tue, Jun 14, 2016 at 03:21:24PM +0200, Martin Babinsky wrote:

On 06/14/2016 04:55 AM, Fraser Tweedale wrote:

On Tue, Jun 14, 2016 at 02:19:27AM +1000, Fraser Tweedale wrote:

On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:


Hi Fraser,

during functional review I found the following issues:

1.)

If I create a CAACL rule tied to a specific sub-CA let's say for user
certificate issuance:

"""
ipa caacl-show user_cert_issuance
  Enabled: TRUE
  User category: all
  CAs: user_sub_ca
  Profiles: caIPAuserCert
  ACL name: user_cert_issuance

"""

I can still happily request certificate for a user using root-CA:

"""
 ipa cert-request cert.csr --principal jdoe --ca ipa
  Certificate: MIID9j.../Ov8mkjFA==
  Subject: CN=jdoe,O=IPA.TEST
  Issuer: CN=Certificate Authority,O=IPA.TEST
  ...
"""

should not this be denied by CA-ACL rule?

The default IPA CAACL rule is like this:

"""
ipa caacl-show hosts_services_caIPAserviceCert
  Enabled: TRUE
  Host category: all
  Service category: all
  Profiles: caIPAserviceCert
  ACL name: hosts_services_caIPAserviceCert

"""

so the default rule should not allow users to request certs at all.


Yes, these should be denied.  Looking into it.


Were you using 'admin' account to request the cert?  admin has
permission 'Request Certificate ignoring CA ACLs' via the
'Certificate Manager' privilege.

If so, please try again with less privileges (e.g. self-service as
jdoe).



You were right when I was requesting certs as user principals themselves
everything worked as expected.

Regarding usb-CAs not present on replica, both machines run the following
version of dogtag CA:

pki-ca-10.3.2-3.fc24.noarch

Here are the last 256 lines of the debug log:

http://paste.fedoraproject.org/378512/65828332


Thanks for that.  It seems there are two issues.  The first one is:

Unable to read key retriever class from CS.cfg: Property
features.authority.keyRetrieverClass missing value

This happens only on replica, until the first restart of Dogtag
after installation.  I have attached a patch to fix it.

The second issue is:

Failed to update certificate
  

This needs to be fixed in Dogtag, however, the error is not
triggered if the key retrieval succeeds when the replica first
observes the addition of a new CA.  The attached patch does help in
this regard, so apply it and I hope it will see you through the rest
of the functional testing of my patches... I hope.

Thank you for testing!

Cheers,
Fraser


Rebased patches attached (VERSION conflicts; no other changes).



Thanks Fraser,

the certificate issuance by sub-CA now works also on replica but I must
first restart PKI for it to pick up stuff from master CA.

If I set up a replica first and then create a sub-CA on master Dogtag picks
this up and uses the new sub-CA and its key for signing without restart.

This seems to be pure Dogtag issue, however and I concluded that IPA side
works ok.

If no one objects then I would give functional ACK to your patches and
document/track the Dogtag issue in a separate ticket.


Pushed to master: f0915e61986f545ad9b282fa90a4b1d0538829c5




Thanks Martin,

The Dogtag ticket is https://fedorahosted.org/pki/ticket/2359.  Fix
is merged and will go out in 10.3.3 this week or early next week.


Once released, please send a patch bumping the minimum version in our 
spec file. I'm leaving https://fedorahosted.org/freeipa/ticket/4559 open 
until the patch is merged.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-14 Thread Fraser Tweedale 
On Tue, Jun 14, 2016 at 03:21:24PM +0200, Martin Babinsky wrote:
> On 06/14/2016 04:55 AM, Fraser Tweedale wrote:
> > On Tue, Jun 14, 2016 at 02:19:27AM +1000, Fraser Tweedale wrote:
> > > On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:
> > > > > > > 
> > > > > > > Hi Fraser,
> > > > > > > 
> > > > > > > during functional review I found the following issues:
> > > > > > > 
> > > > > > > 1.)
> > > > > > > 
> > > > > > > If I create a CAACL rule tied to a specific sub-CA let's say for 
> > > > > > > user
> > > > > > > certificate issuance:
> > > > > > > 
> > > > > > > """
> > > > > > > ipa caacl-show user_cert_issuance
> > > > > > >   Enabled: TRUE
> > > > > > >   User category: all
> > > > > > >   CAs: user_sub_ca
> > > > > > >   Profiles: caIPAuserCert
> > > > > > >   ACL name: user_cert_issuance
> > > > > > > 
> > > > > > > """
> > > > > > > 
> > > > > > > I can still happily request certificate for a user using root-CA:
> > > > > > > 
> > > > > > > """
> > > > > > >  ipa cert-request cert.csr --principal jdoe --ca ipa
> > > > > > >   Certificate: MIID9j.../Ov8mkjFA==
> > > > > > >   Subject: CN=jdoe,O=IPA.TEST
> > > > > > >   Issuer: CN=Certificate Authority,O=IPA.TEST
> > > > > > >   ...
> > > > > > > """
> > > > > > > 
> > > > > > > should not this be denied by CA-ACL rule?
> > > > > > > 
> > > > > > > The default IPA CAACL rule is like this:
> > > > > > > 
> > > > > > > """
> > > > > > > ipa caacl-show hosts_services_caIPAserviceCert
> > > > > > >   Enabled: TRUE
> > > > > > >   Host category: all
> > > > > > >   Service category: all
> > > > > > >   Profiles: caIPAserviceCert
> > > > > > >   ACL name: hosts_services_caIPAserviceCert
> > > > > > > 
> > > > > > > """
> > > > > > > 
> > > > > > > so the default rule should not allow users to request certs at 
> > > > > > > all.
> > > > > > > 
> > > > > > Yes, these should be denied.  Looking into it.
> > > > > > 
> > > > > Were you using 'admin' account to request the cert?  admin has
> > > > > permission 'Request Certificate ignoring CA ACLs' via the
> > > > > 'Certificate Manager' privilege.
> > > > > 
> > > > > If so, please try again with less privileges (e.g. self-service as
> > > > > jdoe).
> > > > > 
> > > > 
> > > > You were right when I was requesting certs as user principals themselves
> > > > everything worked as expected.
> > > > 
> > > > Regarding usb-CAs not present on replica, both machines run the 
> > > > following
> > > > version of dogtag CA:
> > > > 
> > > > pki-ca-10.3.2-3.fc24.noarch
> > > > 
> > > > Here are the last 256 lines of the debug log:
> > > > 
> > > > http://paste.fedoraproject.org/378512/65828332
> > > > 
> > > Thanks for that.  It seems there are two issues.  The first one is:
> > > 
> > > Unable to read key retriever class from CS.cfg: Property
> > > features.authority.keyRetrieverClass missing value
> > > 
> > > This happens only on replica, until the first restart of Dogtag
> > > after installation.  I have attached a patch to fix it.
> > > 
> > > The second issue is:
> > > 
> > > Failed to update certificate
> > >   
> > > 
> > > This needs to be fixed in Dogtag, however, the error is not
> > > triggered if the key retrieval succeeds when the replica first
> > > observes the addition of a new CA.  The attached patch does help in
> > > this regard, so apply it and I hope it will see you through the rest
> > > of the functional testing of my patches... I hope.
> > > 
> > > Thank you for testing!
> > > 
> > > Cheers,
> > > Fraser
> > > 
> > Rebased patches attached (VERSION conflicts; no other changes).
> > 
> 
> Thanks Fraser,
> 
> the certificate issuance by sub-CA now works also on replica but I must
> first restart PKI for it to pick up stuff from master CA.
> 
> If I set up a replica first and then create a sub-CA on master Dogtag picks
> this up and uses the new sub-CA and its key for signing without restart.
> 
> This seems to be pure Dogtag issue, however and I concluded that IPA side
> works ok.
> 
> If no one objects then I would give functional ACK to your patches and
> document/track the Dogtag issue in a separate ticket.
> 
Thanks Martin,

The Dogtag ticket is https://fedorahosted.org/pki/ticket/2359.  Fix
is merged and will go out in 10.3.3 this week or early next week.

Cheers,
Fraser

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-14 Thread Martin Babinsky 

On 06/14/2016 04:55 AM, Fraser Tweedale wrote:

On Tue, Jun 14, 2016 at 02:19:27AM +1000, Fraser Tweedale wrote:

On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:


Hi Fraser,

during functional review I found the following issues:

1.)

If I create a CAACL rule tied to a specific sub-CA let's say for user
certificate issuance:

"""
ipa caacl-show user_cert_issuance
  Enabled: TRUE
  User category: all
  CAs: user_sub_ca
  Profiles: caIPAuserCert
  ACL name: user_cert_issuance

"""

I can still happily request certificate for a user using root-CA:

"""
 ipa cert-request cert.csr --principal jdoe --ca ipa
  Certificate: MIID9j.../Ov8mkjFA==
  Subject: CN=jdoe,O=IPA.TEST
  Issuer: CN=Certificate Authority,O=IPA.TEST
  ...
"""

should not this be denied by CA-ACL rule?

The default IPA CAACL rule is like this:

"""
ipa caacl-show hosts_services_caIPAserviceCert
  Enabled: TRUE
  Host category: all
  Service category: all
  Profiles: caIPAserviceCert
  ACL name: hosts_services_caIPAserviceCert

"""

so the default rule should not allow users to request certs at all.


Yes, these should be denied.  Looking into it.


Were you using 'admin' account to request the cert?  admin has
permission 'Request Certificate ignoring CA ACLs' via the
'Certificate Manager' privilege.

If so, please try again with less privileges (e.g. self-service as
jdoe).



You were right when I was requesting certs as user principals themselves
everything worked as expected.

Regarding usb-CAs not present on replica, both machines run the following
version of dogtag CA:

pki-ca-10.3.2-3.fc24.noarch

Here are the last 256 lines of the debug log:

http://paste.fedoraproject.org/378512/65828332


Thanks for that.  It seems there are two issues.  The first one is:

Unable to read key retriever class from CS.cfg: Property
features.authority.keyRetrieverClass missing value

This happens only on replica, until the first restart of Dogtag
after installation.  I have attached a patch to fix it.

The second issue is:

Failed to update certificate
  

This needs to be fixed in Dogtag, however, the error is not
triggered if the key retrieval succeeds when the replica first
observes the addition of a new CA.  The attached patch does help in
this regard, so apply it and I hope it will see you through the rest
of the functional testing of my patches... I hope.

Thank you for testing!

Cheers,
Fraser


Rebased patches attached (VERSION conflicts; no other changes).



Thanks Fraser,

the certificate issuance by sub-CA now works also on replica but I must 
first restart PKI for it to pick up stuff from master CA.


If I set up a replica first and then create a sub-CA on master Dogtag 
picks this up and uses the new sub-CA and its key for signing without 
restart.


This seems to be pure Dogtag issue, however and I concluded that IPA 
side works ok.


If no one objects then I would give functional ACK to your patches and 
document/track the Dogtag issue in a separate ticket.


--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Fraser Tweedale 
On Tue, Jun 14, 2016 at 02:19:27AM +1000, Fraser Tweedale wrote:
> On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:
> > > > > 
> > > > > Hi Fraser,
> > > > > 
> > > > > during functional review I found the following issues:
> > > > > 
> > > > > 1.)
> > > > > 
> > > > > If I create a CAACL rule tied to a specific sub-CA let's say for user
> > > > > certificate issuance:
> > > > > 
> > > > > """
> > > > > ipa caacl-show user_cert_issuance
> > > > >   Enabled: TRUE
> > > > >   User category: all
> > > > >   CAs: user_sub_ca
> > > > >   Profiles: caIPAuserCert
> > > > >   ACL name: user_cert_issuance
> > > > > 
> > > > > """
> > > > > 
> > > > > I can still happily request certificate for a user using root-CA:
> > > > > 
> > > > > """
> > > > >  ipa cert-request cert.csr --principal jdoe --ca ipa
> > > > >   Certificate: MIID9j.../Ov8mkjFA==
> > > > >   Subject: CN=jdoe,O=IPA.TEST
> > > > >   Issuer: CN=Certificate Authority,O=IPA.TEST
> > > > >   ...
> > > > > """
> > > > > 
> > > > > should not this be denied by CA-ACL rule?
> > > > > 
> > > > > The default IPA CAACL rule is like this:
> > > > > 
> > > > > """
> > > > > ipa caacl-show hosts_services_caIPAserviceCert
> > > > >   Enabled: TRUE
> > > > >   Host category: all
> > > > >   Service category: all
> > > > >   Profiles: caIPAserviceCert
> > > > >   ACL name: hosts_services_caIPAserviceCert
> > > > > 
> > > > > """
> > > > > 
> > > > > so the default rule should not allow users to request certs at all.
> > > > > 
> > > > Yes, these should be denied.  Looking into it.
> > > > 
> > > Were you using 'admin' account to request the cert?  admin has
> > > permission 'Request Certificate ignoring CA ACLs' via the
> > > 'Certificate Manager' privilege.
> > > 
> > > If so, please try again with less privileges (e.g. self-service as
> > > jdoe).
> > > 
> > 
> > You were right when I was requesting certs as user principals themselves
> > everything worked as expected.
> > 
> > Regarding usb-CAs not present on replica, both machines run the following
> > version of dogtag CA:
> > 
> > pki-ca-10.3.2-3.fc24.noarch
> > 
> > Here are the last 256 lines of the debug log:
> > 
> > http://paste.fedoraproject.org/378512/65828332
> > 
> Thanks for that.  It seems there are two issues.  The first one is:
> 
> Unable to read key retriever class from CS.cfg: Property
> features.authority.keyRetrieverClass missing value
> 
> This happens only on replica, until the first restart of Dogtag
> after installation.  I have attached a patch to fix it.
> 
> The second issue is:
> 
> Failed to update certificate
>   
> 
> This needs to be fixed in Dogtag, however, the error is not
> triggered if the key retrieval succeeds when the replica first
> observes the addition of a new CA.  The attached patch does help in
> this regard, so apply it and I hope it will see you through the rest
> of the functional testing of my patches... I hope.
> 
> Thank you for testing!
> 
> Cheers,
> Fraser
>
Rebased patches attached (VERSION conflicts; no other changes).
From efef047bec394c0a881a641f518ea41757ca66eb Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 14 May 2015 01:46:06 -0400
Subject: [PATCH 59/63] Add 'ca' plugin

This commit adds the 'ca' plugin for creating and managing
lightweight CAs.  The initial implementation supports a single level
of sub-CAs underneath the IPA CA.

This commit also:

- adds the container for FreeIPA CA objects

- adds schema for the FreeIPA CA objects

- updates ipa-pki-proxy.conf to allow access to the Dogtag
  lightweight CAs REST API.

Part of: https://fedorahosted.org/freeipa/ticket/4559
---
 ACI.txt   |   8 ++
 API.txt   |  64 +
 VERSION   |   4 +-
 install/conf/ipa-pki-proxy.conf   |   4 +-
 install/share/60certificate-profiles.ldif |   4 +
 install/share/bootstrap-template.ldif |   6 +
 install/updates/41-lightweight-cas.update |   4 +
 install/updates/Makefile.am   |   1 +
 ipalib/constants.py   |   2 +
 ipaserver/install/cainstance.py   |   7 +
 ipaserver/install/server/upgrade.py   |  16 ++-
 ipaserver/plugins/ca.py   | 217 ++
 ipaserver/plugins/dogtag.py   |  54 +++-
 13 files changed, 385 insertions(+), 6 deletions(-)
 create mode 100644 install/updates/41-lightweight-cas.update
 create mode 100644 ipaserver/plugins/ca.py

diff --git a/ACI.txt b/ACI.txt
index 
6f691f2a7b01f834006e3c796c14c256ee87faa6..a26e2dd4bcf7c75b93d2f11cfd470beb17b18873
 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -22,6 +22,14 @@ dn: cn=automount,dc=ipa,dc=example
 aci: (targetattr = "automountmapname || description")(targetfilter = 
"(objectclass=automountmap)")(version 3.0;acl "permission:System: Modify 
Automount Maps";allow (write) groupdn = "ldap:///cn=System: Modify Automount

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Fraser Tweedale 
On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:
> > > > 
> > > > Hi Fraser,
> > > > 
> > > > during functional review I found the following issues:
> > > > 
> > > > 1.)
> > > > 
> > > > If I create a CAACL rule tied to a specific sub-CA let's say for user
> > > > certificate issuance:
> > > > 
> > > > """
> > > > ipa caacl-show user_cert_issuance
> > > >   Enabled: TRUE
> > > >   User category: all
> > > >   CAs: user_sub_ca
> > > >   Profiles: caIPAuserCert
> > > >   ACL name: user_cert_issuance
> > > > 
> > > > """
> > > > 
> > > > I can still happily request certificate for a user using root-CA:
> > > > 
> > > > """
> > > >  ipa cert-request cert.csr --principal jdoe --ca ipa
> > > >   Certificate: MIID9j.../Ov8mkjFA==
> > > >   Subject: CN=jdoe,O=IPA.TEST
> > > >   Issuer: CN=Certificate Authority,O=IPA.TEST
> > > >   ...
> > > > """
> > > > 
> > > > should not this be denied by CA-ACL rule?
> > > > 
> > > > The default IPA CAACL rule is like this:
> > > > 
> > > > """
> > > > ipa caacl-show hosts_services_caIPAserviceCert
> > > >   Enabled: TRUE
> > > >   Host category: all
> > > >   Service category: all
> > > >   Profiles: caIPAserviceCert
> > > >   ACL name: hosts_services_caIPAserviceCert
> > > > 
> > > > """
> > > > 
> > > > so the default rule should not allow users to request certs at all.
> > > > 
> > > Yes, these should be denied.  Looking into it.
> > > 
> > Were you using 'admin' account to request the cert?  admin has
> > permission 'Request Certificate ignoring CA ACLs' via the
> > 'Certificate Manager' privilege.
> > 
> > If so, please try again with less privileges (e.g. self-service as
> > jdoe).
> > 
> 
> You were right when I was requesting certs as user principals themselves
> everything worked as expected.
> 
> Regarding usb-CAs not present on replica, both machines run the following
> version of dogtag CA:
> 
> pki-ca-10.3.2-3.fc24.noarch
> 
> Here are the last 256 lines of the debug log:
> 
> http://paste.fedoraproject.org/378512/65828332
> 
Thanks for that.  It seems there are two issues.  The first one is:

Unable to read key retriever class from CS.cfg: Property
features.authority.keyRetrieverClass missing value

This happens only on replica, until the first restart of Dogtag
after installation.  I have attached a patch to fix it.

The second issue is:

Failed to update certificate
  

This needs to be fixed in Dogtag, however, the error is not
triggered if the key retrieval succeeds when the replica first
observes the addition of a new CA.  The attached patch does help in
this regard, so apply it and I hope it will see you through the rest
of the functional testing of my patches... I hope.

Thank you for testing!

Cheers,
Fraser
From 0eb877bd715033538b0cf3ef724d62c6b20d273f Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 14 Jun 2016 01:22:41 +1000
Subject: [PATCH] replica-install: configure key retriever before starting
 Dogtag

After installing a replica, Dogtag's Lightweight CA key retrieval
fails until Dogtag is restarted, because the already-running
instance doesn't pick up the changes to CS.cfg.  Configure the key
retriever before the instance is started.

Part of: https://fedorahosted.org/freeipa/ticket/4559
---
 ipaserver/install/cainstance.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 
c7f3116f62ce1158a04af23f29439c4a1d1a102f..8dfb71528d2dc020e05ccd7ff42199218a1c0839
 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1344,6 +1344,8 @@ class CAInstance(DogtagInstance):
   self.enable_pkix)
 self.step("set up client auth to db", self.__client_auth_to_db)
 self.step("destroying installation admin user", self.teardown_admin)
+self.step("Configure lightweight CA key retrieval",
+  self.setup_lightweight_ca_key_retrieval)
 self.step("starting instance", self.start_instance)
 
 self.step("importing CA chain to RA certificate database",
@@ -1362,8 +1364,6 @@ class CAInstance(DogtagInstance):
 self.step("updating IPA configuration", update_ipa_conf)
 self.step("Restart HTTP server to pick up changes",
   self.__restart_http_instance)
-self.step("Configure lightweight CA key retrieval",
-  self.setup_lightweight_ca_key_retrieval)
 
 self.step("enabling CA instance", self.__enable_instance)
 
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Fraser Tweedale 
On Mon, Jun 13, 2016 at 11:17:21PM +1000, Fraser Tweedale wrote:
> On Mon, Jun 13, 2016 at 12:48:18PM +0200, Martin Babinsky wrote:
> > On 06/13/2016 08:59 AM, Jan Cholasta wrote:
> > > On 13.6.2016 08:38, Fraser Tweedale wrote:
> > > > On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:
> > > > > On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:
> > > > > > On 9.6.2016 11:10, Fraser Tweedale wrote:
> > > > > > > On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:
> > > > > > > > On 9.6.2016 08:44, Fraser Tweedale wrote:
> > > > > > > > > On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale 
> > > > > > > > > wrote:
> > > > > > > > > > On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta 
> > > > > > > > > > wrote:
> > > > > > > > > > > On 8.6.2016 05:15, Fraser Tweedale wrote:
> > > > > > > > > > > > On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser 
> > > > > > > > > > > > Tweedale wrote:
> > > > > > > > > > > > > On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser 
> > > > > > > > > > > > > Tweedale wrote:
> > > > > > > > > > > > > > Hi team,
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > This patchset implements the 'ca' plugin for 
> > > > > > > > > > > > > > creating and
> > > > > > > > > > > > > > managing
> > > > > > > > > > > > > > lightweight sub-CAs, and updates the 'caacl' plugin 
> > > > > > > > > > > > > > and
> > > > > > > > > > > > > > 'cert-request' command to support multiple CAs.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > A brief overview of the patches:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 0059
> > > > > > > > > > > > > >   'ca' plugin, associated schema changes and 
> > > > > > > > > > > > > > container objects,
> > > > > > > > > > > > > >   Dogtag REST API wrapper
> > > > > > > > > > > > > > 0060
> > > > > > > > > > > > > >   Add CA entry for the IPA CA on install/upgrade
> > > > > > > > > > > > > > 0061
> > > > > > > > > > > > > >   Update 'caacl' plugin with CA support (including 
> > > > > > > > > > > > > > enforcement)
> > > > > > > > > > > > > > 0062
> > > > > > > > > > > > > >   Update ra.request_certificate() to support 
> > > > > > > > > > > > > > specifying
> > > > > > > > > > > > > > target CA
> > > > > > > > > > > > > > 0063
> > > > > > > > > > > > > >   Add '--ca' option to 'cert-request' command
> > > > > > > > > > > > > > 0064
> > > > > > > > > > > > > >   Add '--issuer' option to 'cert-find' command
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > These patches depend on other pending patches:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 0051, 0052, 0053, 0054, 0055, 0056
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Signing key replication depends on unmerged Dogtag 
> > > > > > > > > > > > > > patches.
> > > > > > > > > > > > > > Builds
> > > > > > > > > > > > > > of Dogtag with the required patches, and of FreeIPA 
> > > > > > > > > > > > > > with all
> > > > > > > > > > > > > > completed sub-CAs work, should be available from my 
> > > > > > > > > > > > > > COPR soon:
> > > > > > > > > > > > > > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Some parts of the design are not implemented in the 
> > > > > > > > > > > > > > current
> > > > > > > > > > > > > > patchset, including:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > - local parent CA (ipaca object) references
> > > > > > > > > > > > > > - sub-CA certificate renewal
> > > > > > > > > > > > > > - 'cert-show' command '--ca=NAME' option
> > > > > > > > > > > > > > - certmonger support for specifying CA
> > > > > > > > > > > > > > - revocation of deleted CAs
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > I look forward to your reviews!
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > Fraser
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > Rebased and updated patches attached.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Substantive changes:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > - add required attributes for issuer DN and subject DN
> > > > > > > > > > > > > - prevent rename of IPA CA
> > > > > > > > > > > > > - when adding IPA CA entry, contact Dogtag to learn 
> > > > > > > > > > > > > authority
> > > > > > > > > > > > > id,
> > > > > > > > > > > > >   issuer DN and subject DN
> > > > > > > > > > > > > - add 'read_ca' method to Dogtag interface
> > > > > > > > > > > > > - tighten ACIs to prevent modification of ipacaid 
> > > > > > > > > > > > > attribute
> > > > > > > > > > > > > 
> > > > > > > > > > > > Updated patch 0064-3; adds --issuer option to cert-show 
> > > > > > > > > > > > and --ca
> > > > > > > > > > > > option to cert-show and cert-find.
> > > > > > > > > > > 
> > > > > > > > > > > Patch 0059:
> > > > > > > > > > > 
> > > > > > > > > > > 1) On upgrade, why is the lightweight CA container created
> > > > > > > > > > > twice -

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Fraser Tweedale 
On Mon, Jun 13, 2016 at 12:48:18PM +0200, Martin Babinsky wrote:
> On 06/13/2016 08:59 AM, Jan Cholasta wrote:
> > On 13.6.2016 08:38, Fraser Tweedale wrote:
> > > On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:
> > > > On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:
> > > > > On 9.6.2016 11:10, Fraser Tweedale wrote:
> > > > > > On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:
> > > > > > > On 9.6.2016 08:44, Fraser Tweedale wrote:
> > > > > > > > On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:
> > > > > > > > > On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:
> > > > > > > > > > On 8.6.2016 05:15, Fraser Tweedale wrote:
> > > > > > > > > > > On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale 
> > > > > > > > > > > wrote:
> > > > > > > > > > > > On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser 
> > > > > > > > > > > > Tweedale wrote:
> > > > > > > > > > > > > Hi team,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > This patchset implements the 'ca' plugin for creating 
> > > > > > > > > > > > > and
> > > > > > > > > > > > > managing
> > > > > > > > > > > > > lightweight sub-CAs, and updates the 'caacl' plugin 
> > > > > > > > > > > > > and
> > > > > > > > > > > > > 'cert-request' command to support multiple CAs.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > A brief overview of the patches:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 0059
> > > > > > > > > > > > >   'ca' plugin, associated schema changes and 
> > > > > > > > > > > > > container objects,
> > > > > > > > > > > > >   Dogtag REST API wrapper
> > > > > > > > > > > > > 0060
> > > > > > > > > > > > >   Add CA entry for the IPA CA on install/upgrade
> > > > > > > > > > > > > 0061
> > > > > > > > > > > > >   Update 'caacl' plugin with CA support (including 
> > > > > > > > > > > > > enforcement)
> > > > > > > > > > > > > 0062
> > > > > > > > > > > > >   Update ra.request_certificate() to support 
> > > > > > > > > > > > > specifying
> > > > > > > > > > > > > target CA
> > > > > > > > > > > > > 0063
> > > > > > > > > > > > >   Add '--ca' option to 'cert-request' command
> > > > > > > > > > > > > 0064
> > > > > > > > > > > > >   Add '--issuer' option to 'cert-find' command
> > > > > > > > > > > > > 
> > > > > > > > > > > > > These patches depend on other pending patches:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 0051, 0052, 0053, 0054, 0055, 0056
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Signing key replication depends on unmerged Dogtag 
> > > > > > > > > > > > > patches.
> > > > > > > > > > > > > Builds
> > > > > > > > > > > > > of Dogtag with the required patches, and of FreeIPA 
> > > > > > > > > > > > > with all
> > > > > > > > > > > > > completed sub-CAs work, should be available from my 
> > > > > > > > > > > > > COPR soon:
> > > > > > > > > > > > > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Some parts of the design are not implemented in the 
> > > > > > > > > > > > > current
> > > > > > > > > > > > > patchset, including:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > - local parent CA (ipaca object) references
> > > > > > > > > > > > > - sub-CA certificate renewal
> > > > > > > > > > > > > - 'cert-show' command '--ca=NAME' option
> > > > > > > > > > > > > - certmonger support for specifying CA
> > > > > > > > > > > > > - revocation of deleted CAs
> > > > > > > > > > > > > 
> > > > > > > > > > > > > I look forward to your reviews!
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > Fraser
> > > > > > > > > > > > > 
> > > > > > > > > > > > Rebased and updated patches attached.
> > > > > > > > > > > > 
> > > > > > > > > > > > Substantive changes:
> > > > > > > > > > > > 
> > > > > > > > > > > > - add required attributes for issuer DN and subject DN
> > > > > > > > > > > > - prevent rename of IPA CA
> > > > > > > > > > > > - when adding IPA CA entry, contact Dogtag to learn 
> > > > > > > > > > > > authority
> > > > > > > > > > > > id,
> > > > > > > > > > > >   issuer DN and subject DN
> > > > > > > > > > > > - add 'read_ca' method to Dogtag interface
> > > > > > > > > > > > - tighten ACIs to prevent modification of ipacaid 
> > > > > > > > > > > > attribute
> > > > > > > > > > > > 
> > > > > > > > > > > Updated patch 0064-3; adds --issuer option to cert-show 
> > > > > > > > > > > and --ca
> > > > > > > > > > > option to cert-show and cert-find.
> > > > > > > > > > 
> > > > > > > > > > Patch 0059:
> > > > > > > > > > 
> > > > > > > > > > 1) On upgrade, why is the lightweight CA container created
> > > > > > > > > > twice - once in
> > > > > > > > > > 41-subca.update, once using ensure_entry() call? It should 
> > > > > > > > > > be
> > > > > > > > > > done only
> > > > > > > > > > once.
> > > > > > > > > > 
> > > > > > > > > I'll remove 41-subca.update; the routine in cainstance is the 
> > > > > > > > > one
>

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Martin Babinsky 

On 06/13/2016 08:59 AM, Jan Cholasta wrote:

On 13.6.2016 08:38, Fraser Tweedale wrote:

On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:

On 9.6.2016 11:10, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:

On 9.6.2016 08:44, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:

On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and
managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying
target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.
Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority
id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created
twice - once in
41-subca.update, once using ensure_entry() call? It should be
done only
once.


I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.


2) In ca_del, every CA specified in args[0] should be deleted,
not just the
first one.

3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid?
I guess not,
so you should be able to remove the check altogether and don't
bother with
the warning.


ipacaid is mandatory now, so I'll remove the check.



Patch 0060-0062: LGTM


Yippee \o/



Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key
params, which
unfortunately we cannot leverage in cert_request currently.


No problemo.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show -
show commands
are supposed to retrieve an object given primary key(s), and
the primary key
of CA objects is just their cn.


The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.


Correct, but in IPA the issuer is represented by the CA object, so
in IPA
the primary key for a certificate is actually (CA name, serial).

Certificate lookup by issuer name and serial is actually a search
operation,
analogical to how CA lookup by subject name is also a search
operation, so
it should be done by cert-find.


OK, I will remove the --issuer option for cert-find.




3) In find commands, the options form a filter, so instead of
raising
MutuallyExclusiveError in cert-find, return an empty result, as
with any
other unmatched filter.


Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).


That's one way to look at it, but it's true only if you assume that
cert-find can only search certificates in Dogtag. This will very
soon became
untrue, as we will allow cert-find to also search certificates
anywhere in
LDAP (the server part of ticket #5381). There, the difference
between the
options would be that with --ca you search for

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Jan Cholasta 

On 13.6.2016 08:38, Fraser Tweedale wrote:

On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:

On 9.6.2016 11:10, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:

On 9.6.2016 08:44, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:

On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created twice - once in
41-subca.update, once using ensure_entry() call? It should be done only
once.


I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.


2) In ca_del, every CA specified in args[0] should be deleted, not just the
first one.

3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid? I guess not,
so you should be able to remove the check altogether and don't bother with
the warning.


ipacaid is mandatory now, so I'll remove the check.



Patch 0060-0062: LGTM


Yippee \o/



Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key params, which
unfortunately we cannot leverage in cert_request currently.


No problemo.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show - show commands
are supposed to retrieve an object given primary key(s), and the primary key
of CA objects is just their cn.


The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.


Correct, but in IPA the issuer is represented by the CA object, so in IPA
the primary key for a certificate is actually (CA name, serial).

Certificate lookup by issuer name and serial is actually a search operation,
analogical to how CA lookup by subject name is also a search operation, so
it should be done by cert-find.


OK, I will remove the --issuer option for cert-find.




3) In find commands, the options form a filter, so instead of raising
MutuallyExclusiveError in cert-find, return an empty result, as with any
other unmatched filter.


Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).


That's one way to look at it, but it's true only if you assume that
cert-find can only search certificates in Dogtag. This will very soon became
untrue, as we will allow cert-find to also search certificates anywhere in
LDAP (the server part of ticket #5381). There, the difference between the
options would be that with --ca you search for certificates issued by the
specified managed CA, but

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-13 Thread Fraser Tweedale 
On Fri, Jun 10, 2016 at 12:48:00AM +1000, Fraser Tweedale wrote:
> On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:
> > On 9.6.2016 11:10, Fraser Tweedale wrote:
> > > On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:
> > > > On 9.6.2016 08:44, Fraser Tweedale wrote:
> > > > > On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:
> > > > > > On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:
> > > > > > > On 8.6.2016 05:15, Fraser Tweedale wrote:
> > > > > > > > On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:
> > > > > > > > > On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale 
> > > > > > > > > wrote:
> > > > > > > > > > Hi team,
> > > > > > > > > > 
> > > > > > > > > > This patchset implements the 'ca' plugin for creating and 
> > > > > > > > > > managing
> > > > > > > > > > lightweight sub-CAs, and updates the 'caacl' plugin and
> > > > > > > > > > 'cert-request' command to support multiple CAs.
> > > > > > > > > > 
> > > > > > > > > > A brief overview of the patches:
> > > > > > > > > > 
> > > > > > > > > > 0059
> > > > > > > > > >   'ca' plugin, associated schema changes and container 
> > > > > > > > > > objects,
> > > > > > > > > >   Dogtag REST API wrapper
> > > > > > > > > > 0060
> > > > > > > > > >   Add CA entry for the IPA CA on install/upgrade
> > > > > > > > > > 0061
> > > > > > > > > >   Update 'caacl' plugin with CA support (including 
> > > > > > > > > > enforcement)
> > > > > > > > > > 0062
> > > > > > > > > >   Update ra.request_certificate() to support specifying 
> > > > > > > > > > target CA
> > > > > > > > > > 0063
> > > > > > > > > >   Add '--ca' option to 'cert-request' command
> > > > > > > > > > 0064
> > > > > > > > > >   Add '--issuer' option to 'cert-find' command
> > > > > > > > > > 
> > > > > > > > > > These patches depend on other pending patches:
> > > > > > > > > > 
> > > > > > > > > > 0051, 0052, 0053, 0054, 0055, 0056
> > > > > > > > > > 
> > > > > > > > > > Signing key replication depends on unmerged Dogtag patches. 
> > > > > > > > > >  Builds
> > > > > > > > > > of Dogtag with the required patches, and of FreeIPA with all
> > > > > > > > > > completed sub-CAs work, should be available from my COPR 
> > > > > > > > > > soon:
> > > > > > > > > > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > > > > > > > > > 
> > > > > > > > > > Some parts of the design are not implemented in the current
> > > > > > > > > > patchset, including:
> > > > > > > > > > 
> > > > > > > > > > - local parent CA (ipaca object) references
> > > > > > > > > > - sub-CA certificate renewal
> > > > > > > > > > - 'cert-show' command '--ca=NAME' option
> > > > > > > > > > - certmonger support for specifying CA
> > > > > > > > > > - revocation of deleted CAs
> > > > > > > > > > 
> > > > > > > > > > I look forward to your reviews!
> > > > > > > > > > 
> > > > > > > > > > Thanks,
> > > > > > > > > > Fraser
> > > > > > > > > > 
> > > > > > > > > Rebased and updated patches attached.
> > > > > > > > > 
> > > > > > > > > Substantive changes:
> > > > > > > > > 
> > > > > > > > > - add required attributes for issuer DN and subject DN
> > > > > > > > > - prevent rename of IPA CA
> > > > > > > > > - when adding IPA CA entry, contact Dogtag to learn authority 
> > > > > > > > > id,
> > > > > > > > >   issuer DN and subject DN
> > > > > > > > > - add 'read_ca' method to Dogtag interface
> > > > > > > > > - tighten ACIs to prevent modification of ipacaid attribute
> > > > > > > > > 
> > > > > > > > Updated patch 0064-3; adds --issuer option to cert-show and --ca
> > > > > > > > option to cert-show and cert-find.
> > > > > > > 
> > > > > > > Patch 0059:
> > > > > > > 
> > > > > > > 1) On upgrade, why is the lightweight CA container created twice 
> > > > > > > - once in
> > > > > > > 41-subca.update, once using ensure_entry() call? It should be 
> > > > > > > done only
> > > > > > > once.
> > > > > > > 
> > > > > > I'll remove 41-subca.update; the routine in cainstance is the one
> > > > > > that's needed.
> > > > > > 
> > > > > > > 2) In ca_del, every CA specified in args[0] should be deleted, 
> > > > > > > not just the
> > > > > > > first one.
> > > > > > > 
> > > > > > > 3) Do not use NonFatalError, issue a warning instead:
> > > > > > > 
> > > > > > > self.add_message(MyNewWarningClass(name=...))
> > > > > > > 
> > > > > > > 4) Can it actually happen that ca_show does not return ipacaid? I 
> > > > > > > guess not,
> > > > > > > so you should be able to remove the check altogether and don't 
> > > > > > > bother with
> > > > > > > the warning.
> > > > > > > 
> > > > > > ipacaid is mandatory now, so I'll remove the check.
> > > > > > 
> > > > > > > 
> > > > > > > Patch 0060-0062: LGTM
> > > > > > > 
> > > > > > Yippee \o/
> > > > > > 
> > > > > > > 
> > > > > > > Patch 0063:
> > > > > > > 
> > > > > > > Could you please define the CA param as follows:
> > > > > > > 
> > > > > > > Str('cacn?',
> > > > > > >

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-09 Thread Fraser Tweedale 
On Thu, Jun 09, 2016 at 12:36:35PM +0200, Jan Cholasta wrote:
> On 9.6.2016 11:10, Fraser Tweedale wrote:
> > On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:
> > > On 9.6.2016 08:44, Fraser Tweedale wrote:
> > > > On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:
> > > > > On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:
> > > > > > On 8.6.2016 05:15, Fraser Tweedale wrote:
> > > > > > > On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:
> > > > > > > > On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:
> > > > > > > > > Hi team,
> > > > > > > > > 
> > > > > > > > > This patchset implements the 'ca' plugin for creating and 
> > > > > > > > > managing
> > > > > > > > > lightweight sub-CAs, and updates the 'caacl' plugin and
> > > > > > > > > 'cert-request' command to support multiple CAs.
> > > > > > > > > 
> > > > > > > > > A brief overview of the patches:
> > > > > > > > > 
> > > > > > > > > 0059
> > > > > > > > >   'ca' plugin, associated schema changes and container 
> > > > > > > > > objects,
> > > > > > > > >   Dogtag REST API wrapper
> > > > > > > > > 0060
> > > > > > > > >   Add CA entry for the IPA CA on install/upgrade
> > > > > > > > > 0061
> > > > > > > > >   Update 'caacl' plugin with CA support (including 
> > > > > > > > > enforcement)
> > > > > > > > > 0062
> > > > > > > > >   Update ra.request_certificate() to support specifying 
> > > > > > > > > target CA
> > > > > > > > > 0063
> > > > > > > > >   Add '--ca' option to 'cert-request' command
> > > > > > > > > 0064
> > > > > > > > >   Add '--issuer' option to 'cert-find' command
> > > > > > > > > 
> > > > > > > > > These patches depend on other pending patches:
> > > > > > > > > 
> > > > > > > > > 0051, 0052, 0053, 0054, 0055, 0056
> > > > > > > > > 
> > > > > > > > > Signing key replication depends on unmerged Dogtag patches.  
> > > > > > > > > Builds
> > > > > > > > > of Dogtag with the required patches, and of FreeIPA with all
> > > > > > > > > completed sub-CAs work, should be available from my COPR soon:
> > > > > > > > > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > > > > > > > > 
> > > > > > > > > Some parts of the design are not implemented in the current
> > > > > > > > > patchset, including:
> > > > > > > > > 
> > > > > > > > > - local parent CA (ipaca object) references
> > > > > > > > > - sub-CA certificate renewal
> > > > > > > > > - 'cert-show' command '--ca=NAME' option
> > > > > > > > > - certmonger support for specifying CA
> > > > > > > > > - revocation of deleted CAs
> > > > > > > > > 
> > > > > > > > > I look forward to your reviews!
> > > > > > > > > 
> > > > > > > > > Thanks,
> > > > > > > > > Fraser
> > > > > > > > > 
> > > > > > > > Rebased and updated patches attached.
> > > > > > > > 
> > > > > > > > Substantive changes:
> > > > > > > > 
> > > > > > > > - add required attributes for issuer DN and subject DN
> > > > > > > > - prevent rename of IPA CA
> > > > > > > > - when adding IPA CA entry, contact Dogtag to learn authority 
> > > > > > > > id,
> > > > > > > >   issuer DN and subject DN
> > > > > > > > - add 'read_ca' method to Dogtag interface
> > > > > > > > - tighten ACIs to prevent modification of ipacaid attribute
> > > > > > > > 
> > > > > > > Updated patch 0064-3; adds --issuer option to cert-show and --ca
> > > > > > > option to cert-show and cert-find.
> > > > > > 
> > > > > > Patch 0059:
> > > > > > 
> > > > > > 1) On upgrade, why is the lightweight CA container created twice - 
> > > > > > once in
> > > > > > 41-subca.update, once using ensure_entry() call? It should be done 
> > > > > > only
> > > > > > once.
> > > > > > 
> > > > > I'll remove 41-subca.update; the routine in cainstance is the one
> > > > > that's needed.
> > > > > 
> > > > > > 2) In ca_del, every CA specified in args[0] should be deleted, not 
> > > > > > just the
> > > > > > first one.
> > > > > > 
> > > > > > 3) Do not use NonFatalError, issue a warning instead:
> > > > > > 
> > > > > > self.add_message(MyNewWarningClass(name=...))
> > > > > > 
> > > > > > 4) Can it actually happen that ca_show does not return ipacaid? I 
> > > > > > guess not,
> > > > > > so you should be able to remove the check altogether and don't 
> > > > > > bother with
> > > > > > the warning.
> > > > > > 
> > > > > ipacaid is mandatory now, so I'll remove the check.
> > > > > 
> > > > > > 
> > > > > > Patch 0060-0062: LGTM
> > > > > > 
> > > > > Yippee \o/
> > > > > 
> > > > > > 
> > > > > > Patch 0063:
> > > > > > 
> > > > > > Could you please define the CA param as follows:
> > > > > > 
> > > > > > Str('cacn?',
> > > > > > cli_name='ca',
> > > > > > query=True,
> > > > > > label=_("CA"),
> > > > > > doc=_("CA to use"),
> > > > > > ),
> > > > > > 
> > > > > > ?
> > > > > > 
> > > > > > This is for consitency with framework-generated parent key params, 
> > > > > > which
> > > > > > unfortunately we cannot leverage in

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-09 Thread Martin Babinsky 

On 06/09/2016 08:44 AM, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:

On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created twice - once in
41-subca.update, once using ensure_entry() call? It should be done only
once.


I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.


2) In ca_del, every CA specified in args[0] should be deleted, not just the
first one.

3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid? I guess not,
so you should be able to remove the check altogether and don't bother with
the warning.


ipacaid is mandatory now, so I'll remove the check.



Patch 0060-0062: LGTM


Yippee \o/



Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key params, which
unfortunately we cannot leverage in cert_request currently.


No problemo.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show - show commands
are supposed to retrieve an object given primary key(s), and the primary key
of CA objects is just their cn.


The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.


3) In find commands, the options form a filter, so instead of raising
MutuallyExclusiveError in cert-find, return an empty result, as with any
other unmatched filter.


Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).

Do you think it would be clearer to provide a single option that
takes issuer DN or the name of CA?  I considered this but decided
against it because collisions are possible (one could name an IPA CA
something that passes for a stingified DN).

Thanks for reviewing!
Fraser


Updated patches attached (0059-3..0063-3, 0064-4).  No substantive
changes to 0060..0062.




Hi Fraser,

I'm doing functional review of your sub-CA patches and your first patch 
breaks upgrade [1].


Bear in mind that all the system configuration is upgraded after LDAP 
upgrade, so you actually have to use update file to add 
"cn=cas,cn=ca,$SUFFIX$" subtree. Otherwise the code tries to add ACIs to 
entries that are not yet created resulting in the pasted error.


[1] https://paste.fedoraproject.org/376600/74804146/

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-09 Thread Jan Cholasta 

On 9.6.2016 11:10, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 10:12:40AM +0200, Jan Cholasta wrote:

On 9.6.2016 08:44, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:

On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created twice - once in
41-subca.update, once using ensure_entry() call? It should be done only
once.


I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.


2) In ca_del, every CA specified in args[0] should be deleted, not just the
first one.

3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid? I guess not,
so you should be able to remove the check altogether and don't bother with
the warning.


ipacaid is mandatory now, so I'll remove the check.



Patch 0060-0062: LGTM


Yippee \o/



Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key params, which
unfortunately we cannot leverage in cert_request currently.


No problemo.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show - show commands
are supposed to retrieve an object given primary key(s), and the primary key
of CA objects is just their cn.


The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.


Correct, but in IPA the issuer is represented by the CA object, so in IPA
the primary key for a certificate is actually (CA name, serial).

Certificate lookup by issuer name and serial is actually a search operation,
analogical to how CA lookup by subject name is also a search operation, so
it should be done by cert-find.


OK, I will remove the --issuer option for cert-find.




3) In find commands, the options form a filter, so instead of raising
MutuallyExclusiveError in cert-find, return an empty result, as with any
other unmatched filter.


Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).


That's one way to look at it, but it's true only if you assume that
cert-find can only search certificates in Dogtag. This will very soon became
untrue, as we will allow cert-find to also search certificates anywhere in
LDAP (the server part of ticket #5381). There, the difference between the
options would be that with --ca you search for certificates issued by the
specified managed CA, but with --issuer you search for certificates with the
given issuer name, be it managed CA or not.


--ca is just a "shorthand" for --issuer - it merely looks up subject
DN of the

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-09 Thread Jan Cholasta 

On 9.6.2016 08:44, Fraser Tweedale wrote:

On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:

On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created twice - once in
41-subca.update, once using ensure_entry() call? It should be done only
once.


I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.


2) In ca_del, every CA specified in args[0] should be deleted, not just the
first one.

3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid? I guess not,
so you should be able to remove the check altogether and don't bother with
the warning.


ipacaid is mandatory now, so I'll remove the check.



Patch 0060-0062: LGTM


Yippee \o/



Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key params, which
unfortunately we cannot leverage in cert_request currently.


No problemo.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show - show commands
are supposed to retrieve an object given primary key(s), and the primary key
of CA objects is just their cn.


The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.


Correct, but in IPA the issuer is represented by the CA object, so in 
IPA the primary key for a certificate is actually (CA name, serial).


Certificate lookup by issuer name and serial is actually a search 
operation, analogical to how CA lookup by subject name is also a search 
operation, so it should be done by cert-find.





3) In find commands, the options form a filter, so instead of raising
MutuallyExclusiveError in cert-find, return an empty result, as with any
other unmatched filter.


Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).


That's one way to look at it, but it's true only if you assume that 
cert-find can only search certificates in Dogtag. This will very soon 
became untrue, as we will allow cert-find to also search certificates 
anywhere in LDAP (the server part of ticket #5381). There, the 
difference between the options would be that with --ca you search for 
certificates issued by the specified managed CA, but with --issuer you 
search for certificates with the given issuer name, be it managed CA or not.


For now, IMO the correct behavior should be that if both are specified 
and the issuer name of the specified CA does not match the specified 
issuer name, empty result is returned, otherwise carry on with the 
search in Dogtag.

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-09 Thread Fraser Tweedale 
On Thu, Jun 09, 2016 at 01:21:29AM +1000, Fraser Tweedale wrote:
> On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:
> > On 8.6.2016 05:15, Fraser Tweedale wrote:
> > > On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:
> > > > On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:
> > > > > Hi team,
> > > > > 
> > > > > This patchset implements the 'ca' plugin for creating and managing
> > > > > lightweight sub-CAs, and updates the 'caacl' plugin and
> > > > > 'cert-request' command to support multiple CAs.
> > > > > 
> > > > > A brief overview of the patches:
> > > > > 
> > > > > 0059
> > > > >   'ca' plugin, associated schema changes and container objects,
> > > > >   Dogtag REST API wrapper
> > > > > 0060
> > > > >   Add CA entry for the IPA CA on install/upgrade
> > > > > 0061
> > > > >   Update 'caacl' plugin with CA support (including enforcement)
> > > > > 0062
> > > > >   Update ra.request_certificate() to support specifying target CA
> > > > > 0063
> > > > >   Add '--ca' option to 'cert-request' command
> > > > > 0064
> > > > >   Add '--issuer' option to 'cert-find' command
> > > > > 
> > > > > These patches depend on other pending patches:
> > > > > 
> > > > > 0051, 0052, 0053, 0054, 0055, 0056
> > > > > 
> > > > > Signing key replication depends on unmerged Dogtag patches.  Builds
> > > > > of Dogtag with the required patches, and of FreeIPA with all
> > > > > completed sub-CAs work, should be available from my COPR soon:
> > > > > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > > > > 
> > > > > Some parts of the design are not implemented in the current
> > > > > patchset, including:
> > > > > 
> > > > > - local parent CA (ipaca object) references
> > > > > - sub-CA certificate renewal
> > > > > - 'cert-show' command '--ca=NAME' option
> > > > > - certmonger support for specifying CA
> > > > > - revocation of deleted CAs
> > > > > 
> > > > > I look forward to your reviews!
> > > > > 
> > > > > Thanks,
> > > > > Fraser
> > > > > 
> > > > Rebased and updated patches attached.
> > > > 
> > > > Substantive changes:
> > > > 
> > > > - add required attributes for issuer DN and subject DN
> > > > - prevent rename of IPA CA
> > > > - when adding IPA CA entry, contact Dogtag to learn authority id,
> > > >   issuer DN and subject DN
> > > > - add 'read_ca' method to Dogtag interface
> > > > - tighten ACIs to prevent modification of ipacaid attribute
> > > > 
> > > Updated patch 0064-3; adds --issuer option to cert-show and --ca
> > > option to cert-show and cert-find.
> > 
> > Patch 0059:
> > 
> > 1) On upgrade, why is the lightweight CA container created twice - once in
> > 41-subca.update, once using ensure_entry() call? It should be done only
> > once.
> > 
> I'll remove 41-subca.update; the routine in cainstance is the one
> that's needed.
> 
> > 2) In ca_del, every CA specified in args[0] should be deleted, not just the
> > first one.
> > 
> > 3) Do not use NonFatalError, issue a warning instead:
> > 
> > self.add_message(MyNewWarningClass(name=...))
> > 
> > 4) Can it actually happen that ca_show does not return ipacaid? I guess not,
> > so you should be able to remove the check altogether and don't bother with
> > the warning.
> > 
> ipacaid is mandatory now, so I'll remove the check.
> 
> > 
> > Patch 0060-0062: LGTM
> > 
> Yippee \o/
> 
> > 
> > Patch 0063:
> > 
> > Could you please define the CA param as follows:
> > 
> > Str('cacn?',
> > cli_name='ca',
> > query=True,
> > label=_("CA"),
> > doc=_("CA to use"),
> > ),
> > 
> > ?
> > 
> > This is for consitency with framework-generated parent key params, which
> > unfortunately we cannot leverage in cert_request currently.
> > 
> No problemo.
> 
> > 
> > Patch 0064:
> > 
> > 1) See my comment for patch 0063, it applies here as well.
> > 
> > 2) The --issuer option should not be included in cert_show - show commands
> > are supposed to retrieve an object given primary key(s), and the primary key
> > of CA objects is just their cn.
> > 
> The --issuer argument is because primary key for a cert is really
> (issuer, serial).  So it show the cert _with_ that issuer (and
> serial), not the cert _for_ that issuer.
> 
> > 3) In find commands, the options form a filter, so instead of raising
> > MutuallyExclusiveError in cert-find, return an empty result, as with any
> > other unmatched filter.
> > 
> Here, --issuer and --ca are two different ways to specify the
> issuer.  --issuer lets you give the issuer DN straight up; --ca
> takes the name of an IPA CA object and looks up its issuer DN.
> (Thus it makes no sense to give both options at once).
> 
> Do you think it would be clearer to provide a single option that
> takes issuer DN or the name of CA?  I considered this but decided
> against it because collisions are possible (one could name an IPA CA
> something that passes for a stingified DN).
> 
> Thanks for reviewing!
> Fraser
>
Updated patches

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-08 Thread Fraser Tweedale 
On Wed, Jun 08, 2016 at 01:00:36PM +0200, Jan Cholasta wrote:
> On 8.6.2016 05:15, Fraser Tweedale wrote:
> > On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:
> > > On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:
> > > > Hi team,
> > > > 
> > > > This patchset implements the 'ca' plugin for creating and managing
> > > > lightweight sub-CAs, and updates the 'caacl' plugin and
> > > > 'cert-request' command to support multiple CAs.
> > > > 
> > > > A brief overview of the patches:
> > > > 
> > > > 0059
> > > >   'ca' plugin, associated schema changes and container objects,
> > > >   Dogtag REST API wrapper
> > > > 0060
> > > >   Add CA entry for the IPA CA on install/upgrade
> > > > 0061
> > > >   Update 'caacl' plugin with CA support (including enforcement)
> > > > 0062
> > > >   Update ra.request_certificate() to support specifying target CA
> > > > 0063
> > > >   Add '--ca' option to 'cert-request' command
> > > > 0064
> > > >   Add '--issuer' option to 'cert-find' command
> > > > 
> > > > These patches depend on other pending patches:
> > > > 
> > > > 0051, 0052, 0053, 0054, 0055, 0056
> > > > 
> > > > Signing key replication depends on unmerged Dogtag patches.  Builds
> > > > of Dogtag with the required patches, and of FreeIPA with all
> > > > completed sub-CAs work, should be available from my COPR soon:
> > > > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > > > 
> > > > Some parts of the design are not implemented in the current
> > > > patchset, including:
> > > > 
> > > > - local parent CA (ipaca object) references
> > > > - sub-CA certificate renewal
> > > > - 'cert-show' command '--ca=NAME' option
> > > > - certmonger support for specifying CA
> > > > - revocation of deleted CAs
> > > > 
> > > > I look forward to your reviews!
> > > > 
> > > > Thanks,
> > > > Fraser
> > > > 
> > > Rebased and updated patches attached.
> > > 
> > > Substantive changes:
> > > 
> > > - add required attributes for issuer DN and subject DN
> > > - prevent rename of IPA CA
> > > - when adding IPA CA entry, contact Dogtag to learn authority id,
> > >   issuer DN and subject DN
> > > - add 'read_ca' method to Dogtag interface
> > > - tighten ACIs to prevent modification of ipacaid attribute
> > > 
> > Updated patch 0064-3; adds --issuer option to cert-show and --ca
> > option to cert-show and cert-find.
> 
> Patch 0059:
> 
> 1) On upgrade, why is the lightweight CA container created twice - once in
> 41-subca.update, once using ensure_entry() call? It should be done only
> once.
> 
I'll remove 41-subca.update; the routine in cainstance is the one
that's needed.

> 2) In ca_del, every CA specified in args[0] should be deleted, not just the
> first one.
> 
> 3) Do not use NonFatalError, issue a warning instead:
> 
> self.add_message(MyNewWarningClass(name=...))
> 
> 4) Can it actually happen that ca_show does not return ipacaid? I guess not,
> so you should be able to remove the check altogether and don't bother with
> the warning.
> 
ipacaid is mandatory now, so I'll remove the check.

> 
> Patch 0060-0062: LGTM
> 
Yippee \o/

> 
> Patch 0063:
> 
> Could you please define the CA param as follows:
> 
> Str('cacn?',
> cli_name='ca',
> query=True,
> label=_("CA"),
> doc=_("CA to use"),
> ),
> 
> ?
> 
> This is for consitency with framework-generated parent key params, which
> unfortunately we cannot leverage in cert_request currently.
> 
No problemo.

> 
> Patch 0064:
> 
> 1) See my comment for patch 0063, it applies here as well.
> 
> 2) The --issuer option should not be included in cert_show - show commands
> are supposed to retrieve an object given primary key(s), and the primary key
> of CA objects is just their cn.
> 
The --issuer argument is because primary key for a cert is really
(issuer, serial).  So it show the cert _with_ that issuer (and
serial), not the cert _for_ that issuer.

> 3) In find commands, the options form a filter, so instead of raising
> MutuallyExclusiveError in cert-find, return an empty result, as with any
> other unmatched filter.
> 
Here, --issuer and --ca are two different ways to specify the
issuer.  --issuer lets you give the issuer DN straight up; --ca
takes the name of an IPA CA object and looks up its issuer DN.
(Thus it makes no sense to give both options at once).

Do you think it would be clearer to provide a single option that
takes issuer DN or the name of CA?  I considered this but decided
against it because collisions are possible (one could name an IPA CA
something that passes for a stingified DN).

Thanks for reviewing!
Fraser

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-08 Thread Jan Cholasta 

On 8.6.2016 13:37, Pavel Vomacka wrote:



On 06/08/2016 01:21 PM, Pavel Vomacka wrote:




On 06/08/2016 05:15 AM, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.



Hello,

why is there --rename option in ca-mod command? Shouldn't it be rather
--cn to be consistent with ca-show?

Actually, I meant to be consistent with attribute name in result of API
call of ca-show command.

Is there any reason why to have there rename? Just a note: I look at
it mainly from point of view of WebUI.


It is consistent with other mod commands, they all have --rename if the 
object is renameable. You can't use the primary key name (cn) because 
that's already taken by the positional argument of the show command.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-08 Thread Pavel Vomacka 



On 06/08/2016 01:21 PM, Pavel Vomacka wrote:




On 06/08/2016 05:15 AM, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
   'ca' plugin, associated schema changes and container objects,
   Dogtag REST API wrapper
0060
   Add CA entry for the IPA CA on install/upgrade
0061
   Update 'caacl' plugin with CA support (including enforcement)
0062
   Update ra.request_certificate() to support specifying target CA
0063
   Add '--ca' option to 'cert-request' command
0064
   Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

 0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
   issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.



Hello,

why is there --rename option in ca-mod command? Shouldn't it be rather 
--cn to be consistent with ca-show? 
Actually, I meant to be consistent with attribute name in result of API 
call of ca-show command.
Is there any reason why to have there rename? Just a note: I look at 
it mainly from point of view of WebUI.


--
Pavel^3 Vomacka




-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-08 Thread Pavel Vomacka 



On 06/08/2016 05:15 AM, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
   'ca' plugin, associated schema changes and container objects,
   Dogtag REST API wrapper
0060
   Add CA entry for the IPA CA on install/upgrade
0061
   Update 'caacl' plugin with CA support (including enforcement)
0062
   Update ra.request_certificate() to support specifying target CA
0063
   Add '--ca' option to 'cert-request' command
0064
   Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

 0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
   issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.



Hello,

why is there --rename option in ca-mod command? Shouldn't it be rather 
--cn to be consistent with ca-show? Is there any reason why to have 
there rename? Just a note: I look at it mainly from point of view of WebUI.


--
Pavel^3 Vomacka
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-08 Thread Jan Cholasta 

On 8.6.2016 05:15, Fraser Tweedale wrote:

On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:

On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:

Hi team,

This patchset implements the 'ca' plugin for creating and managing
lightweight sub-CAs, and updates the 'caacl' plugin and
'cert-request' command to support multiple CAs.

A brief overview of the patches:

0059
  'ca' plugin, associated schema changes and container objects,
  Dogtag REST API wrapper
0060
  Add CA entry for the IPA CA on install/upgrade
0061
  Update 'caacl' plugin with CA support (including enforcement)
0062
  Update ra.request_certificate() to support specifying target CA
0063
  Add '--ca' option to 'cert-request' command
0064
  Add '--issuer' option to 'cert-find' command

These patches depend on other pending patches:

0051, 0052, 0053, 0054, 0055, 0056

Signing key replication depends on unmerged Dogtag patches.  Builds
of Dogtag with the required patches, and of FreeIPA with all
completed sub-CAs work, should be available from my COPR soon:
https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/

Some parts of the design are not implemented in the current
patchset, including:

- local parent CA (ipaca object) references
- sub-CA certificate renewal
- 'cert-show' command '--ca=NAME' option
- certmonger support for specifying CA
- revocation of deleted CAs

I look forward to your reviews!

Thanks,
Fraser


Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute


Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.


Patch 0059:

1) On upgrade, why is the lightweight CA container created twice - once 
in 41-subca.update, once using ensure_entry() call? It should be done 
only once.


2) In ca_del, every CA specified in args[0] should be deleted, not just 
the first one.


3) Do not use NonFatalError, issue a warning instead:

self.add_message(MyNewWarningClass(name=...))

4) Can it actually happen that ca_show does not return ipacaid? I guess 
not, so you should be able to remove the check altogether and don't 
bother with the warning.



Patch 0060-0062: LGTM


Patch 0063:

Could you please define the CA param as follows:

Str('cacn?',
cli_name='ca',
query=True,
label=_("CA"),
doc=_("CA to use"),
),

?

This is for consitency with framework-generated parent key params, which 
unfortunately we cannot leverage in cert_request currently.



Patch 0064:

1) See my comment for patch 0063, it applies here as well.

2) The --issuer option should not be included in cert_show - show 
commands are supposed to retrieve an object given primary key(s), and 
the primary key of CA objects is just their cn.


3) In find commands, the options form a filter, so instead of raising 
MutuallyExclusiveError in cert-find, return an empty result, as with any 
other unmatched filter.



--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-07 Thread Fraser Tweedale 
On Tue, Jun 07, 2016 at 03:42:22PM +1000, Fraser Tweedale wrote:
> On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:
> > Hi team,
> > 
> > This patchset implements the 'ca' plugin for creating and managing
> > lightweight sub-CAs, and updates the 'caacl' plugin and
> > 'cert-request' command to support multiple CAs.
> > 
> > A brief overview of the patches:
> > 
> > 0059
> >   'ca' plugin, associated schema changes and container objects,
> >   Dogtag REST API wrapper
> > 0060
> >   Add CA entry for the IPA CA on install/upgrade
> > 0061
> >   Update 'caacl' plugin with CA support (including enforcement)
> > 0062
> >   Update ra.request_certificate() to support specifying target CA
> > 0063
> >   Add '--ca' option to 'cert-request' command
> > 0064
> >   Add '--issuer' option to 'cert-find' command
> > 
> > These patches depend on other pending patches:
> > 
> > 0051, 0052, 0053, 0054, 0055, 0056
> > 
> > Signing key replication depends on unmerged Dogtag patches.  Builds
> > of Dogtag with the required patches, and of FreeIPA with all
> > completed sub-CAs work, should be available from my COPR soon:
> > https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> > 
> > Some parts of the design are not implemented in the current
> > patchset, including:
> > 
> > - local parent CA (ipaca object) references
> > - sub-CA certificate renewal
> > - 'cert-show' command '--ca=NAME' option
> > - certmonger support for specifying CA
> > - revocation of deleted CAs
> > 
> > I look forward to your reviews!
> > 
> > Thanks,
> > Fraser
> >
> Rebased and updated patches attached.
> 
> Substantive changes:
> 
> - add required attributes for issuer DN and subject DN
> - prevent rename of IPA CA
> - when adding IPA CA entry, contact Dogtag to learn authority id,
>   issuer DN and subject DN
> - add 'read_ca' method to Dogtag interface
> - tighten ACIs to prevent modification of ipacaid attribute
> 
Updated patch 0064-3; adds --issuer option to cert-show and --ca
option to cert-show and cert-find.
From 7367f8efb5e8d8f1edeaaa74b3f35b79fc5a9a46 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 10 May 2016 13:56:40 +1000
Subject: [PATCH] Add issuer options to 'cert-show' and 'cert-find'

Add options to cert-show and cert-find for specifying the issuer as
a DN, or a CA name.

Also add the issuer DN to the output of cert-find.

Part of: https://fedorahosted.org/freeipa/ticket/4559
---
 API.txt |   8 +++-
 VERSION |   4 +-
 ipaserver/plugins/cert.py   | 106 +++-
 ipaserver/plugins/dogtag.py |   9 
 4 files changed, 93 insertions(+), 34 deletions(-)

diff --git a/API.txt b/API.txt
index 
571c646c7b973b3107e9abf9f60caa4ded297c2f..f6853991659bfcaaae12d68fe8b4b8edefd6855f
 100644
--- a/API.txt
+++ b/API.txt
@@ -730,11 +730,13 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: cert_find
-args: 0,17,4
+args: 0,19,4
 option: Flag('all', autofill=True, cli_name='all', default=False)
+option: Str('ca?', autofill=False)
 option: Flag('exactly?', autofill=True, default=False)
 option: Str('issuedon_from?', autofill=False)
 option: Str('issuedon_to?', autofill=False)
+option: Str('issuer?', autofill=False)
 option: Int('max_serial_number?', autofill=False)
 option: Int('min_serial_number?', autofill=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
@@ -774,8 +776,10 @@ option: Int('revocation_reason', autofill=True, default=0)
 option: Str('version?')
 output: Output('result')
 command: cert_show
-args: 1,2,1
+args: 1,4,1
 arg: Str('serial_number')
+option: Str('ca?', autofill=False)
+option: Str('issuer?', autofill=False)
 option: Str('out?')
 option: Str('version?')
 output: Output('result')
diff --git a/VERSION b/VERSION
index 
6577c80904f99ab6b30217ddc82625ccda928df0..600ab8182332de68bc356100ae0e232c68e30d90
 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=179
-# Last change: ftweedal - add --ca option to cert-request
+IPA_API_VERSION_MINOR=180
+# Last change: ftweedal - add issuer options to cert-show and cert-find
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
f09947337efc3d5b5872937df4a82663dfb6eff9..ce7d042d4f872c9f8176e887fd2bbfe07c1fe1a9
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -583,37 +583,17 @@ class cert_show(VirtualCommand):
 
 takes_args = _serial_number
 
-has_output_params = (
-Str('certificate',
-label=_('Certificate'),
-),
-Str('subject',
-label=_('Subject'),
-),
-Str('issuer',
-label=_('Issuer'),
-),
-Str('valid_not_before',
-label=_('Not Before'),

Re: [Freeipa-devel] [PATCH] 0059..0064 Lightweight sub-CAs

2016-06-06 Thread Fraser Tweedale 
On Wed, Jun 01, 2016 at 02:51:04PM +1000, Fraser Tweedale wrote:
> Hi team,
> 
> This patchset implements the 'ca' plugin for creating and managing
> lightweight sub-CAs, and updates the 'caacl' plugin and
> 'cert-request' command to support multiple CAs.
> 
> A brief overview of the patches:
> 
> 0059
>   'ca' plugin, associated schema changes and container objects,
>   Dogtag REST API wrapper
> 0060
>   Add CA entry for the IPA CA on install/upgrade
> 0061
>   Update 'caacl' plugin with CA support (including enforcement)
> 0062
>   Update ra.request_certificate() to support specifying target CA
> 0063
>   Add '--ca' option to 'cert-request' command
> 0064
>   Add '--issuer' option to 'cert-find' command
> 
> These patches depend on other pending patches:
> 
> 0051, 0052, 0053, 0054, 0055, 0056
> 
> Signing key replication depends on unmerged Dogtag patches.  Builds
> of Dogtag with the required patches, and of FreeIPA with all
> completed sub-CAs work, should be available from my COPR soon:
> https://copr.fedorainfracloud.org/coprs/ftweedal/freeipa/
> 
> Some parts of the design are not implemented in the current
> patchset, including:
> 
> - local parent CA (ipaca object) references
> - sub-CA certificate renewal
> - 'cert-show' command '--ca=NAME' option
> - certmonger support for specifying CA
> - revocation of deleted CAs
> 
> I look forward to your reviews!
> 
> Thanks,
> Fraser
>
Rebased and updated patches attached.

Substantive changes:

- add required attributes for issuer DN and subject DN
- prevent rename of IPA CA
- when adding IPA CA entry, contact Dogtag to learn authority id,
  issuer DN and subject DN
- add 'read_ca' method to Dogtag interface
- tighten ACIs to prevent modification of ipacaid attribute

Thanks,
Fraser
From a36659566a06e78b1af83828f01052ecbe5ebbbe Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Thu, 14 May 2015 01:46:06 -0400
Subject: [PATCH 59/64] Add 'ca' plugin

This commit adds the 'ca' plugin for creating and managing
lightweight CAs.  The initial implementation supports a single level
of sub-CAs underneath the IPA CA.

This commit also:

- adds the container for FreeIPA CA objects

- adds schema for the FreeIPA CA objects

- updates ipa-pki-proxy.conf to allow access to the Dogtag
  lightweight CAs REST API.

Part of: https://fedorahosted.org/freeipa/ticket/4559
---
 ACI.txt   |   8 +
 API.txt   |  64 
 VERSION   |   4 +-
 install/conf/ipa-pki-proxy.conf   |   4 +-
 install/share/60certificate-profiles.ldif |   4 +
 install/share/bootstrap-template.ldif |   6 +
 install/updates/41-subca.update   |   4 +
 install/updates/Makefile.am   |   1 +
 ipalib/constants.py   |   2 +
 ipaserver/install/cainstance.py   |   7 +
 ipaserver/install/server/upgrade.py   |  16 +-
 ipaserver/plugins/ca.py   | 235 ++
 ipaserver/plugins/dogtag.py   |  54 ++-
 13 files changed, 403 insertions(+), 6 deletions(-)
 create mode 100644 install/updates/41-subca.update
 create mode 100644 ipaserver/plugins/ca.py

diff --git a/ACI.txt b/ACI.txt
index 
a09495e5a445d7c3bc51f0b082c538b80bbb425a..586f7343ef13b4b30e23543027220ed37ba2cb48
 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -22,6 +22,14 @@ dn: cn=automount,dc=ipa,dc=example
 aci: (targetattr = "automountmapname || description")(targetfilter = 
"(objectclass=automountmap)")(version 3.0;acl "permission:System: Modify 
Automount Maps";allow (write) groupdn = "ldap:///cn=System: Modify Automount 
Maps,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=automount,dc=ipa,dc=example
 aci: (targetfilter = "(objectclass=automountmap)")(version 3.0;acl 
"permission:System: Remove Automount Maps";allow (delete) groupdn = 
"ldap:///cn=System: Remove Automount 
Maps,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=cas,cn=ca,dc=ipa,dc=example
+aci: (targetfilter = "(objectclass=ipaca)")(version 3.0;acl 
"permission:System: Add CA";allow (add) groupdn = "ldap:///cn=System: Add 
CA,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=cas,cn=ca,dc=ipa,dc=example
+aci: (targetfilter = "(objectclass=ipaca)")(version 3.0;acl 
"permission:System: Delete CA";allow (delete) groupdn = "ldap:///cn=System: 
Delete CA,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=cas,cn=ca,dc=ipa,dc=example
+aci: (targetattr = "cn || description")(targetfilter = 
"(objectclass=ipaca)")(version 3.0;acl "permission:System: Modify CA";allow 
(write) groupdn = "ldap:///cn=System: Modify 
CA,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=cas,cn=ca,dc=ipa,dc=example
+aci: (targetattr = "cn || createtimestamp || description || entryusn || 
ipacaid || ipacaissuerdn || ipacasubjectdn || modifytimestamp || 
objectclass")(targetfilter = "(objectclass=ipaca)")(version 3.0;acl 
"permission:System: Read CAs";allow (compare,read,search)