Re: [Freeipa-devel] [PATCH] 0101 Add ca-disable and ca-enable commands
On 09/06/2016 04:49 PM, Fraser Tweedale wrote: On Tue, Aug 30, 2016 at 10:23:10AM +0200, Martin Babinsky wrote: On 08/30/2016 10:09 AM, Jan Cholasta wrote: Hi, On 30.8.2016 09:56, Martin Babinsky wrote: On 08/25/2016 10:25 AM, Fraser Tweedale wrote: Hi team, The attached patch fixes https://fedorahosted.org/freeipa/ticket/6257. The behaviour of cert-request when the CA is disabled is not very nice (it reports a server error from Dogtag). The Dogtag REST interface gives much better errors so I plan to move to it in a later change (which will also address https://fedorahosted.org/freeipa/ticket/3473, in part). Thanks, Fraser HI Fraser, I have a couple of comments below: 1.) @@ -25,6 +33,10 @@ EXAMPLES: ipa ca-add puppet --desc "Puppet" \\ --subject "CN=Puppet CA,O=EXAMPLE.COM" + Disable a CA. + +ipa ca-disable puppet + """) You missed an example of `ca-enable` command in the doc string. 2.) Regarding implementation of ca_enable/disable, I think you can reduce the amount of code duplication by employing a base class which will look up the required sub-CA and call the RA backend method required by the subclass. See the attached untested diff (passes lint) for details. Looks like I forgot how to OOP while on PTO :) Honza is right, of course, see the example code in the attached diff (again not tested, just a quick example). Updated patch attached, implemented inheritance suggestion and expanding plugin help. Thanks, Fraser Thanks, ACK. Pushed to: master: c7e0dbc4e174d0bb7577de18cdb2f414f4199c57 ipa-4-4: b037e54e457d731cd16144df7573f4c85d79368a -- 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] 0101 Add ca-disable and ca-enable commands
On Tue, Aug 30, 2016 at 10:23:10AM +0200, Martin Babinsky wrote:
> On 08/30/2016 10:09 AM, Jan Cholasta wrote:
> > Hi,
> >
> > On 30.8.2016 09:56, Martin Babinsky wrote:
> > > On 08/25/2016 10:25 AM, Fraser Tweedale wrote:
> > > > Hi team,
> > > >
> > > > The attached patch fixes
> > > > https://fedorahosted.org/freeipa/ticket/6257.
> > > >
> > > > The behaviour of cert-request when the CA is disabled is not very
> > > > nice (it reports a server error from Dogtag). The Dogtag REST
> > > > interface gives much better errors so I plan to move to it in a
> > > > later change (which will also address
> > > > https://fedorahosted.org/freeipa/ticket/3473, in part).
> > > >
> > > > Thanks,
> > > > Fraser
> > > >
> > > >
> > > >
> > >
> > > HI Fraser,
> > >
> > > I have a couple of comments below:
> > >
> > > 1.)
> > > @@ -25,6 +33,10 @@ EXAMPLES:
> > > ipa ca-add puppet --desc "Puppet" \\
> > > --subject "CN=Puppet CA,O=EXAMPLE.COM"
> > >
> > > + Disable a CA.
> > > +
> > > +ipa ca-disable puppet
> > > +
> > > """)
> > >
> > > You missed an example of `ca-enable` command in the doc string.
> > >
> > > 2.)
> > >
> > > Regarding implementation of ca_enable/disable, I think you can reduce
> > > the amount of code duplication by employing a base class which will look
> > > up the required sub-CA and call the RA backend method required by the
> > > subclass. See the attached untested diff (passes lint) for details.
>
> Looks like I forgot how to OOP while on PTO :) Honza is right, of course,
> see the example code in the attached diff (again not tested, just a quick
> example).
>
Updated patch attached, implemented inheritance suggestion and
expanding plugin help.
Thanks,
Fraser
From 61adc46ec9a19f1044231d193a0d9cdef0adba64 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale
Date: Thu, 25 Aug 2016 17:00:01 +1000
Subject: [PATCH] Add ca-disable and ca-enable commands
We soon plan to revoke certificates upon lightweight CA deletion.
This makes it important to provide a way to prevent a CA from
issuing certificates whilst not deleting and revoking it, and
continuing to allow management of issued certs.
This commit adds the ca-disable and ca-enable commands.
Fixes: https://fedorahosted.org/freeipa/ticket/6257
---
API.txt | 16 +++
VERSION | 4 +--
ipaserver/plugins/ca.py | 66 +++--
ipaserver/plugins/dogtag.py | 6 +
4 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/API.txt b/API.txt
index
5b83bfbd0b457b77e0522ab7d83abfae4df3ebe9..27b64ee143fa4f5f55c1b8a32446f004a8e3bb22
100644
--- a/API.txt
+++ b/API.txt
@@ -465,6 +465,20 @@ option: Str('version?')
output: Output('result', type=[])
output: Output('summary', type=[, ])
output: ListOfPrimaryKeys('value')
+command: ca_disable/1
+args: 1,1,3
+arg: Str('cn', cli_name='name')
+option: Str('version?')
+output: Output('result', type=[])
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
+command: ca_enable/1
+args: 1,1,3
+arg: Str('cn', cli_name='name')
+option: Str('version?')
+output: Output('result', type=[])
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
command: ca_find/1
args: 1,11,4
arg: Str('criteria?')
@@ -6249,6 +6263,8 @@ default: batch/1
default: ca/1
default: ca_add/1
default: ca_del/1
+default: ca_disable/1
+default: ca_enable/1
default: ca_find/1
default: ca_is_enabled/1
default: ca_mod/1
diff --git a/VERSION b/VERSION
index
bf8160a5deb1f7a5148ef6833cec318af144b5d7..c6fb1cba2757d919a88093ca3f060f80b2d30621
100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=212
-# Last change: ab: service: add flag to allow S4U2Self
+IPA_API_VERSION_MINOR=213
+# Last change: ftweedal: add ca-disable and ca-enable commands
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index
966ae2b1bdb4bb0207dfa58f0e9c951bc930f766..4d83fe81c951b01d06d3c85d74fe94e24bce0b1f
100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -2,12 +2,12 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
-from ipalib import api, errors, DNParam, Str
+from ipalib import api, errors, output, DNParam, Str
from ipalib.constants import IPA_CA_CN
from ipalib.plugable import Registry
from ipaserver.plugins.baseldap import (
LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete,
-LDAPUpdate, LDAPRetrieve)
+LDAPUpdate, LDAPRetrieve, LDAPQuery, pkey_to_value)
from ipaserver.plugins.cert import ca_enabled_check
from ipalib import _, ngettext
@@ -18,6 +18,14 @@ Manage Certificate Authorities
Subordinate Certificate Authorities (Sub-CAs) can be added for scoped issuance
of X.509 certificates.
+CAs are enabled on creation, but their use is subject to CA ACLs unl
Re: [Freeipa-devel] [PATCH] 0101 Add ca-disable and ca-enable commands
On 08/30/2016 10:09 AM, Jan Cholasta wrote:
Hi,
On 30.8.2016 09:56, Martin Babinsky wrote:
On 08/25/2016 10:25 AM, Fraser Tweedale wrote:
Hi team,
The attached patch fixes
https://fedorahosted.org/freeipa/ticket/6257.
The behaviour of cert-request when the CA is disabled is not very
nice (it reports a server error from Dogtag). The Dogtag REST
interface gives much better errors so I plan to move to it in a
later change (which will also address
https://fedorahosted.org/freeipa/ticket/3473, in part).
Thanks,
Fraser
HI Fraser,
I have a couple of comments below:
1.)
@@ -25,6 +33,10 @@ EXAMPLES:
ipa ca-add puppet --desc "Puppet" \\
--subject "CN=Puppet CA,O=EXAMPLE.COM"
+ Disable a CA.
+
+ipa ca-disable puppet
+
""")
You missed an example of `ca-enable` command in the doc string.
2.)
Regarding implementation of ca_enable/disable, I think you can reduce
the amount of code duplication by employing a base class which will look
up the required sub-CA and call the RA backend method required by the
subclass. See the attached untested diff (passes lint) for details.
NACK, please don't use getattr() this way. Since you are subclassing
here, you should use polymorphism:
class CAQuery(LDAPQuery):
def execute(...):
...
self.perform_action(ca_api, ca_id)
...
def perform_action(self, ca_api, ca_id):
raise NotImplementedError()
class ca_enable(CAQuery):
def perform_action(self, ca_api, ca_id):
ca_api.enable_ca(ca_id)
Honza
Looks like I forgot how to OOP while on PTO :) Honza is right, of
course, see the example code in the attached diff (again not tested,
just a quick example).
--
Martin^3 Babinsky
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index 93c4872..efeab74 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -236,25 +236,18 @@ class ca_mod(LDAPUpdate):
return dn
-@register()
-class ca_disable(LDAPQuery):
-__doc__ = _('Disable a CA.')
-
-msg_summary = _('Disabled CA "%(value)s"')
+class CAQuery(LDAPQuery):
has_output = output.standard_value
+def perform_action(self, ca_api, ca_id):
+raise NotImplementedError()
+
def execute(self, cn, **options):
ca_enabled_check()
-if cn == IPA_CA_CN:
-raise errors.ProtectedEntryError(
-label=_("CA"),
-key=cn,
-reason=_("IPA CA cannot be disabled"))
-
ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
with self.api.Backend.ra_lightweight_ca as ca_api:
-ca_api.disable_ca(ca_id)
+self.perform_action(ca_api, ca_id)
return dict(
result=True,
@@ -263,20 +256,29 @@ class ca_disable(LDAPQuery):
@register()
-class ca_enable(LDAPQuery):
-__doc__ = _('Enable a CA.')
+class ca_disable(CAQuery):
+__doc__ = _('Disable a CA.')
-msg_summary = _('Enabled CA "%(value)s"')
-has_output = output.standard_value
+msg_summary = _('Disabled CA "%(value)s"')
def execute(self, cn, **options):
-ca_enabled_check()
+if cn == IPA_CA_CN:
+raise errors.ProtectedEntryError(
+label=_("CA"),
+key=cn,
+reason=_("IPA CA cannot be disabled"))
-ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
-with self.api.Backend.ra_lightweight_ca as ca_api:
-ca_api.enable_ca(ca_id)
+return super(ca_disable, self).execute(cn, **options)
-return dict(
-result=True,
-value=pkey_to_value(cn, options),
-)
+def perform_action(self, ca_api, ca_id):
+ca_api.disable_ca(ca_id)
+
+
+@register()
+class ca_enable(CAQuery):
+__doc__ = _('Enable a CA.')
+
+msg_summary = _('Enabled CA "%(value)s"')
+
+def perform_action(self, ca_api, ca_id):
+ca_api.enable_ca(ca_id)
--
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] 0101 Add ca-disable and ca-enable commands
Hi, On 30.8.2016 09:56, Martin Babinsky wrote: On 08/25/2016 10:25 AM, Fraser Tweedale wrote: Hi team, The attached patch fixes https://fedorahosted.org/freeipa/ticket/6257. The behaviour of cert-request when the CA is disabled is not very nice (it reports a server error from Dogtag). The Dogtag REST interface gives much better errors so I plan to move to it in a later change (which will also address https://fedorahosted.org/freeipa/ticket/3473, in part). Thanks, Fraser HI Fraser, I have a couple of comments below: 1.) @@ -25,6 +33,10 @@ EXAMPLES: ipa ca-add puppet --desc "Puppet" \\ --subject "CN=Puppet CA,O=EXAMPLE.COM" + Disable a CA. + +ipa ca-disable puppet + """) You missed an example of `ca-enable` command in the doc string. 2.) Regarding implementation of ca_enable/disable, I think you can reduce the amount of code duplication by employing a base class which will look up the required sub-CA and call the RA backend method required by the subclass. See the attached untested diff (passes lint) for details. NACK, please don't use getattr() this way. Since you are subclassing here, you should use polymorphism: class CAQuery(LDAPQuery): def execute(...): ... self.perform_action(ca_api, ca_id) ... def perform_action(self, ca_api, ca_id): raise NotImplementedError() class ca_enable(CAQuery): def perform_action(self, ca_api, ca_id): ca_api.enable_ca(ca_id) Honza -- 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] 0101 Add ca-disable and ca-enable commands
On 08/25/2016 10:25 AM, Fraser Tweedale wrote:
Hi team,
The attached patch fixes
https://fedorahosted.org/freeipa/ticket/6257.
The behaviour of cert-request when the CA is disabled is not very
nice (it reports a server error from Dogtag). The Dogtag REST
interface gives much better errors so I plan to move to it in a
later change (which will also address
https://fedorahosted.org/freeipa/ticket/3473, in part).
Thanks,
Fraser
HI Fraser,
I have a couple of comments below:
1.)
@@ -25,6 +33,10 @@ EXAMPLES:
ipa ca-add puppet --desc "Puppet" \\
--subject "CN=Puppet CA,O=EXAMPLE.COM"
+ Disable a CA.
+
+ipa ca-disable puppet
+
""")
You missed an example of `ca-enable` command in the doc string.
2.)
Regarding implementation of ca_enable/disable, I think you can reduce
the amount of code duplication by employing a base class which will look
up the required sub-CA and call the RA backend method required by the
subclass. See the attached untested diff (passes lint) for details.
--
Martin^3 Babinsky
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index 93c4872..a274dbc 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -236,12 +236,31 @@ class ca_mod(LDAPUpdate):
return dn
+class BaseCAQuery(LDAPQuery):
+has_output = output.standard_value
+
+def perform_action(self, cn, action='disable_ca', **options):
+"""
+Perform action on lightweight CA using Dogtag RA backend
+:param ca_id: sub-CA id
+:param action: action to perform of sub-CA represented by
+Dogtag API method
+"""
+ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
+with self.api.Backend.ra_lightweight_ca as ca_api:
+getattr(ca_api, action)(ca_id)
+
+return dict(
+result=True,
+value=pkey_to_value(cn, options),
+)
+
+
@register()
-class ca_disable(LDAPQuery):
+class ca_disable(BaseCAQuery):
__doc__ = _('Disable a CA.')
msg_summary = _('Disabled CA "%(value)s"')
-has_output = output.standard_value
def execute(self, cn, **options):
ca_enabled_check()
@@ -252,31 +271,16 @@ class ca_disable(LDAPQuery):
key=cn,
reason=_("IPA CA cannot be disabled"))
-ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
-with self.api.Backend.ra_lightweight_ca as ca_api:
-ca_api.disable_ca(ca_id)
-
-return dict(
-result=True,
-value=pkey_to_value(cn, options),
-)
+return self.perform_action(cn, action='disable_ca', **options)
@register()
-class ca_enable(LDAPQuery):
+class ca_enable(BaseCAQuery):
__doc__ = _('Enable a CA.')
msg_summary = _('Enabled CA "%(value)s"')
-has_output = output.standard_value
def execute(self, cn, **options):
ca_enabled_check()
-ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
-with self.api.Backend.ra_lightweight_ca as ca_api:
-ca_api.enable_ca(ca_id)
-
-return dict(
-result=True,
-value=pkey_to_value(cn, options),
-)
+return self.perform_action(cn, action='enable_ca', **options)
--
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
[Freeipa-devel] [PATCH] 0101 Add ca-disable and ca-enable commands
Hi team,
The attached patch fixes
https://fedorahosted.org/freeipa/ticket/6257.
The behaviour of cert-request when the CA is disabled is not very
nice (it reports a server error from Dogtag). The Dogtag REST
interface gives much better errors so I plan to move to it in a
later change (which will also address
https://fedorahosted.org/freeipa/ticket/3473, in part).
Thanks,
Fraser
From 1d99777c2145d33278d2b1d8a4e8a2d1341c8e4d Mon Sep 17 00:00:00 2001
From: Fraser Tweedale
Date: Thu, 25 Aug 2016 17:00:01 +1000
Subject: [PATCH] Add ca-disable and ca-enable commands
We soon plan to revoke certificates upon lightweight CA deletion.
This makes it important to provide a way to prevent a CA from
issuing certificates whilst not deleting and revoking it, and
continuing to allow management of issued certs.
This commit adds the ca-disable and ca-enable commands.
Fixes: https://fedorahosted.org/freeipa/ticket/6257
---
API.txt | 16
VERSION | 4 +--
ipaserver/plugins/ca.py | 62 +++--
ipaserver/plugins/dogtag.py | 6 +
4 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/API.txt b/API.txt
index
5b83bfbd0b457b77e0522ab7d83abfae4df3ebe9..27b64ee143fa4f5f55c1b8a32446f004a8e3bb22
100644
--- a/API.txt
+++ b/API.txt
@@ -465,6 +465,20 @@ option: Str('version?')
output: Output('result', type=[])
output: Output('summary', type=[, ])
output: ListOfPrimaryKeys('value')
+command: ca_disable/1
+args: 1,1,3
+arg: Str('cn', cli_name='name')
+option: Str('version?')
+output: Output('result', type=[])
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
+command: ca_enable/1
+args: 1,1,3
+arg: Str('cn', cli_name='name')
+option: Str('version?')
+output: Output('result', type=[])
+output: Output('summary', type=[, ])
+output: PrimaryKey('value')
command: ca_find/1
args: 1,11,4
arg: Str('criteria?')
@@ -6249,6 +6263,8 @@ default: batch/1
default: ca/1
default: ca_add/1
default: ca_del/1
+default: ca_disable/1
+default: ca_enable/1
default: ca_find/1
default: ca_is_enabled/1
default: ca_mod/1
diff --git a/VERSION b/VERSION
index
a8b89ed305bcfdf2990a7400d005a68d734fa7e8..8cc8b11c7c3e985ab53279b27a4701021e4271ba
100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=212
-# Last change: ab: service: add flag to allow S4U2Self
+IPA_API_VERSION_MINOR=213
+# Last change: ftweedal: add ca-disable and ca-enable commands
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index
966ae2b1bdb4bb0207dfa58f0e9c951bc930f766..93c48722720e8509c2d096d66f9f2bd1c5c631d8
100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -2,12 +2,12 @@
# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
#
-from ipalib import api, errors, DNParam, Str
+from ipalib import api, errors, output, DNParam, Str
from ipalib.constants import IPA_CA_CN
from ipalib.plugable import Registry
from ipaserver.plugins.baseldap import (
LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete,
-LDAPUpdate, LDAPRetrieve)
+LDAPUpdate, LDAPRetrieve, LDAPQuery, pkey_to_value)
from ipaserver.plugins.cert import ca_enabled_check
from ipalib import _, ngettext
@@ -18,6 +18,14 @@ Manage Certificate Authorities
Subordinate Certificate Authorities (Sub-CAs) can be added for scoped issuance
of X.509 certificates.
+CAs are enabled on creation, but their use is subject to CA ACLs unless the
+operator has permission to bypass CA ACLs.
+
+All CAs except the 'IPA' CA can be disabled or re-enabled. Disabling a CA
+prevents it from issuing certificates but does not affect the validity of its
+certificate.
+
+
EXAMPLES:
Create new CA, subordinate to the IPA CA.
@@ -25,6 +33,10 @@ EXAMPLES:
ipa ca-add puppet --desc "Puppet" \\
--subject "CN=Puppet CA,O=EXAMPLE.COM"
+ Disable a CA.
+
+ipa ca-disable puppet
+
""")
@@ -222,3 +234,49 @@ class ca_mod(LDAPUpdate):
reason=u'IPA CA cannot be renamed')
return dn
+
+
+@register()
+class ca_disable(LDAPQuery):
+__doc__ = _('Disable a CA.')
+
+msg_summary = _('Disabled CA "%(value)s"')
+has_output = output.standard_value
+
+def execute(self, cn, **options):
+ca_enabled_check()
+
+if cn == IPA_CA_CN:
+raise errors.ProtectedEntryError(
+label=_("CA"),
+key=cn,
+reason=_("IPA CA cannot be disabled"))
+
+ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
+with self.api.Backend.ra_lightweight_ca as ca_api:
+ca_api.disable_ca(ca_id)
+
+return dict(
+result=True,
+value=pkey_to_value(cn, options),
+)
+
+
+@register()
+class ca_enable(LDAPQuery):
+
