Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 23.6.2014 13:01, Martin Kosek wrote:
On 06/18/2014 02:09 PM, Jan Cholasta wrote:
...
3) I am thinking why do we need to introduce all the ASN parsing? I am talking
about _decode_krb5principalname and others. If we do not use the result
anywhere, why should we include this part at all?
To work around shortcomings of NSS/python-nss. In particular,
_decode_krb5principalname is used to decode KRB5PrincipalName SANs to a string.
This is necessary because certmonger puts such SANs in certificate requests it
generates.
I know, but we do not use the values besides DNS SAN type, right? I was just
afraid that a decode error in a decoding of an unused SAN type would cause the
entire CSR processing to crash.
If we do not allow principal name SANs, requests from certmonger will fail. If
we allow them, but ignore them, you could request a certificate for an
arbitrary unrelated principal. Thus, the only thing we can do is allow them and
validate them, which is what the patch does and why decoding KRB5PrincipalName
is necessary.
True, we need to make sure the principal type SANs match. ack on the intent.
4) I get crash in the certmonger request:
ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 -r
-N cn=`hostname` -K host/`hostname` -D test1.example.com -D
test2.example.com
-E mko...@redhat.com
# ipa-getcert list -i test-san-1
Number of certificates and requests being tracked: 8.
Request ID 'test-san-1':
status: CA_UNREACHABLE
ca-error: Server failed request, will retry: -504 (HTTP response code is
500,
not 200).
stuck: yes
HTTPD traceback
[Mon Jun 16 13:06:14.528642 2014] [:error] [pid 12941] [remote
...
PyAsn1Error: TagSet(Tag(tagClass=128, tagFormat=0, tagId=2)) not in asn1Spec:
_GeneralName()
What version of certmonger are you using?
Latest Fedora 20, i.e. should be certmonger-0.71.2-1.fc20 (do not have access
to my VM atm). Is this a bug in certmonger?
No, it's bug in my code. Fixed.
Works.
Also added patch 301 which fixes a bug in ldap2.get_effective_rights I was
hitting with patch 234.
Updated patches attached.
Thanks. Functionally, the patch looks ok to me, we are close to final ack. Just
couple more minor changes need to happen:
1) We decided to drop ipaVirtualOperation concept in the end and allow people
reading this list. This implies following changes:
234 - drop ipaVirtualOperation from cn=request certificate with
subjectaltname,cn=virtual operations,cn=etc,$SUFFIX
300 - drop the entire patch (sorry)
OK.
2) I would like to have more confidence that no unauthorized SAN extension gets
in. I know we have the
+for name_type, name in subjectaltname:
+if name_type not in (pkcs10.SAN_DNSNAME,
loop, but I would also like to add else path to the SAN type validation loop,
in case somebody just expands the part above, but forgot to also add
validation. I want us to be explicit in this case:
+for name_type, name in subjectaltname:
+if name_type == pkcs10.SAN_DNSNAME:
...
+elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME,
+ pkcs10.SAN_OTHERNAME_UPN):
...
else:
raise errors.ACIError(info=_(
Unauthorized subject alt name '%s'.) % name)
Fixed.
When this is fixed, we should be done.
Thanks,
Martin
Updated and rebased patches attached.
--
Jan Cholasta
From 26cfd1e4b12e84861b19028e8c81befc8e4bca8e Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 5 Dec 2013 14:34:14 +0100
Subject: [PATCH 1/3] Allow SAN in IPA certificate profile.
https://fedorahosted.org/freeipa/ticket/3977
---
install/tools/ipa-upgradeconfig | 7 +-
ipaserver/install/cainstance.py | 51 +
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 99dfbdf..688e178 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -330,9 +330,14 @@ def upgrade_ipa_profile(ca, domain, fqdn):
root_logger.debug('Subject Key Identifier updated.')
else:
root_logger.debug('Subject Key Identifier already set.')
+san = ca.enable_subject_alternative_name()
+if san:
+root_logger.debug('Subject Alternative Name updated.')
+else:
+root_logger.debug('Subject Alternative Name already set.')
audit = ca.set_audit_renewal()
uri = ca.set_crl_ocsp_extensions(domain, fqdn)
-if audit or ski or uri:
+if audit or ski or san or uri:
return True
else:
root_logger.info('CA is not configured')
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index b5c6cdc..b13a77d 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -464,6 +464,7 @@ class CAInstance(service.Service):
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 06/24/2014 11:30 AM, Jan Cholasta wrote: On 23.6.2014 13:01, Martin Kosek wrote: On 06/18/2014 02:09 PM, Jan Cholasta wrote: ... 3) I am thinking why do we need to introduce all the ASN parsing? I am talking about _decode_krb5principalname and others. If we do not use the result anywhere, why should we include this part at all? To work around shortcomings of NSS/python-nss. In particular, _decode_krb5principalname is used to decode KRB5PrincipalName SANs to a string. This is necessary because certmonger puts such SANs in certificate requests it generates. I know, but we do not use the values besides DNS SAN type, right? I was just afraid that a decode error in a decoding of an unused SAN type would cause the entire CSR processing to crash. If we do not allow principal name SANs, requests from certmonger will fail. If we allow them, but ignore them, you could request a certificate for an arbitrary unrelated principal. Thus, the only thing we can do is allow them and validate them, which is what the patch does and why decoding KRB5PrincipalName is necessary. True, we need to make sure the principal type SANs match. ack on the intent. 4) I get crash in the certmonger request: ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 -r -N cn=`hostname` -K host/`hostname` -D test1.example.com -D test2.example.com -E mko...@redhat.com # ipa-getcert list -i test-san-1 Number of certificates and requests being tracked: 8. Request ID 'test-san-1': status: CA_UNREACHABLE ca-error: Server failed request, will retry: -504 (HTTP response code is 500, not 200). stuck: yes HTTPD traceback [Mon Jun 16 13:06:14.528642 2014] [:error] [pid 12941] [remote ... PyAsn1Error: TagSet(Tag(tagClass=128, tagFormat=0, tagId=2)) not in asn1Spec: _GeneralName() What version of certmonger are you using? Latest Fedora 20, i.e. should be certmonger-0.71.2-1.fc20 (do not have access to my VM atm). Is this a bug in certmonger? No, it's bug in my code. Fixed. Works. Also added patch 301 which fixes a bug in ldap2.get_effective_rights I was hitting with patch 234. Updated patches attached. Thanks. Functionally, the patch looks ok to me, we are close to final ack. Just couple more minor changes need to happen: 1) We decided to drop ipaVirtualOperation concept in the end and allow people reading this list. This implies following changes: 234 - drop ipaVirtualOperation from cn=request certificate with subjectaltname,cn=virtual operations,cn=etc,$SUFFIX 300 - drop the entire patch (sorry) OK. 2) I would like to have more confidence that no unauthorized SAN extension gets in. I know we have the +for name_type, name in subjectaltname: +if name_type not in (pkcs10.SAN_DNSNAME, loop, but I would also like to add else path to the SAN type validation loop, in case somebody just expands the part above, but forgot to also add validation. I want us to be explicit in this case: +for name_type, name in subjectaltname: +if name_type == pkcs10.SAN_DNSNAME: ... +elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME, + pkcs10.SAN_OTHERNAME_UPN): ... else: raise errors.ACIError(info=_( Unauthorized subject alt name '%s'.) % name) Fixed. When this is fixed, we should be done. Thanks, Martin Updated and rebased patches attached. This looks good. We can finally push this beast. Note that we should also add proper tests in future exercising the new functionality. ACK. Pushed to master: 8b8774d138348ab4b938f98dc106ea983e261262 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 06/18/2014 02:09 PM, Jan Cholasta wrote: ... 3) I am thinking why do we need to introduce all the ASN parsing? I am talking about _decode_krb5principalname and others. If we do not use the result anywhere, why should we include this part at all? To work around shortcomings of NSS/python-nss. In particular, _decode_krb5principalname is used to decode KRB5PrincipalName SANs to a string. This is necessary because certmonger puts such SANs in certificate requests it generates. I know, but we do not use the values besides DNS SAN type, right? I was just afraid that a decode error in a decoding of an unused SAN type would cause the entire CSR processing to crash. If we do not allow principal name SANs, requests from certmonger will fail. If we allow them, but ignore them, you could request a certificate for an arbitrary unrelated principal. Thus, the only thing we can do is allow them and validate them, which is what the patch does and why decoding KRB5PrincipalName is necessary. True, we need to make sure the principal type SANs match. ack on the intent. 4) I get crash in the certmonger request: ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 -r -N cn=`hostname` -K host/`hostname` -D test1.example.com -D test2.example.com -E mko...@redhat.com # ipa-getcert list -i test-san-1 Number of certificates and requests being tracked: 8. Request ID 'test-san-1': status: CA_UNREACHABLE ca-error: Server failed request, will retry: -504 (HTTP response code is 500, not 200). stuck: yes HTTPD traceback [Mon Jun 16 13:06:14.528642 2014] [:error] [pid 12941] [remote ... PyAsn1Error: TagSet(Tag(tagClass=128, tagFormat=0, tagId=2)) not in asn1Spec: _GeneralName() What version of certmonger are you using? Latest Fedora 20, i.e. should be certmonger-0.71.2-1.fc20 (do not have access to my VM atm). Is this a bug in certmonger? No, it's bug in my code. Fixed. Works. Also added patch 301 which fixes a bug in ldap2.get_effective_rights I was hitting with patch 234. Updated patches attached. Thanks. Functionally, the patch looks ok to me, we are close to final ack. Just couple more minor changes need to happen: 1) We decided to drop ipaVirtualOperation concept in the end and allow people reading this list. This implies following changes: 234 - drop ipaVirtualOperation from cn=request certificate with subjectaltname,cn=virtual operations,cn=etc,$SUFFIX 300 - drop the entire patch (sorry) 2) I would like to have more confidence that no unauthorized SAN extension gets in. I know we have the +for name_type, name in subjectaltname: +if name_type not in (pkcs10.SAN_DNSNAME, loop, but I would also like to add else path to the SAN type validation loop, in case somebody just expands the part above, but forgot to also add validation. I want us to be explicit in this case: +for name_type, name in subjectaltname: +if name_type == pkcs10.SAN_DNSNAME: ... +elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME, + pkcs10.SAN_OTHERNAME_UPN): ... else: raise errors.ACIError(info=_( Unauthorized subject alt name '%s'.) % name) When this is fixed, we should be done. Thanks, Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 06/11/2014 02:59 PM, Jan Cholasta wrote:
On 11.6.2014 13:29, Martin Kosek wrote:
On 06/11/2014 10:58 AM, Jan Cholasta wrote:
On 10.6.2014 09:55, Martin Kosek wrote:
On 06/06/2014 12:50 PM, Jan Cholasta wrote:
On 23.1.2014 14:34, Jan Cholasta wrote:
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing
managedby_host
to needing write access to an entry, I am not sure I understand
why that
was changed. not saying it is necessarily wrong, but why the
original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has
write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for write
access to the SAN services, so that access control can be (partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write
permission
when the host is in the managed-by attribute, is that the reasoning ?
Exactly. The ACIs that allow this by default are named Hosts can
manage
service Certificates and kerberos keys and Hosts can manage other
host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that requesting
certificate with SAN is allowed only to users which have write access
to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a try
for a formal ack.
OK, thanks.
Bump.
I have added stricter validation of subject alt names as well as
certificate
extensions in general to the second patch.
Thanks!
Updated patches attached.
Note that you will need python-nss 0.15 in order to test, you can get a
RPM for
Fedora here:
http://koji.fedoraproject.org/koji/buildinfo?buildID=514739.
John, do you think we could build python-nss 0.15 also for Fedora 20? The
fix
incorporated in it is pretty important to warrant bugfix update in bodhi.
It
would also allow FreeIPA 4.0 to be installed on Fedora 20.
Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does not
work,
because https://fedorahosted.org/freeipa/ticket/4370.
This worked for me, I suspect this is a DS bug. More info in the ticket.
Now back to review of the patch:
210.4: looks ok
234.4:
1) Virtual operation request certificate with subjectaltname should be a
member of Certificate Administrators privilege
OK.
2) I would write Request Certificate With SubjectAltName with with
instead
of With. I.e.:
Request Certificate with SubjectAltName
OK.
3) Why is the extension check only enforced for non-hosts?
+if not bind_principal.startswith('host/'):
+for ext in extensions:
+operation = self._allowed_extensions.get(ext)
+if operation:
+self.check_access(operation)
My tricky certmonger request passed:
# ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g
2048 -r
-N cn=`hostname` -K host/`hostname` -D test1.example.com -D
test2.example.com
-E mko...@redhat.com
while when I posted the same CSR as privileged user, it was rejected:
$ klist
Ticket cache: KEYRING:persistent:96203:96203
Default principal: f...@idm.lab.eng.brq.redhat.com
$ ipa cert-request --principal test/`hostname` certmonger.csr
ipa: ERROR: invalid 'csr': subject alt name type RFC822 Name is forbidden
Right, I meant to NACK myself, but you were faster. Fixed.
4) Regular users cannot read Virtual Operations, so even if I assign them a
permission, command is refused:
$ ipa cert-request --principal test/`hostname` openssl.csr
ipa: ERROR: Insufficient access: No such virtual command
I think we will need to create new managed permission Read Virtual
Operations
and assign it to Certificate Administrators privilege by default as this
privilege has the virtual operation permissions assigned. Petr3, what is
your
take on this?
Otherwise the patch worked well for me, the authorization part looked OK.
My
biggest concern was just the host/user differentiation wrt unknown
subjectaltname types.
Martin
Updated patches attached.
1) I could not compile the patch set:
$ make rpms
...
if [ != yes ]; then \
./makeapi --validate; \
./makeaci --validate; \
fi
Traceback (most recent call last):
File ./makeapi, line 457, in module
sys.exit(main())
File ./makeapi, line 428, in
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 16.6.2014 13:31, Martin Kosek wrote:
On 06/11/2014 02:59 PM, Jan Cholasta wrote:
On 11.6.2014 13:29, Martin Kosek wrote:
On 06/11/2014 10:58 AM, Jan Cholasta wrote:
On 10.6.2014 09:55, Martin Kosek wrote:
On 06/06/2014 12:50 PM, Jan Cholasta wrote:
On 23.1.2014 14:34, Jan Cholasta wrote:
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing
managedby_host
to needing write access to an entry, I am not sure I understand
why that
was changed. not saying it is necessarily wrong, but why the
original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for write
access to the SAN services, so that access control can be (partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write permission
when the host is in the managed-by attribute, is that the reasoning ?
Exactly. The ACIs that allow this by default are named Hosts can manage
service Certificates and kerberos keys and Hosts can manage other host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that requesting
certificate with SAN is allowed only to users which have write access to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a try
for a formal ack.
OK, thanks.
Bump.
I have added stricter validation of subject alt names as well as certificate
extensions in general to the second patch.
Thanks!
Updated patches attached.
Note that you will need python-nss 0.15 in order to test, you can get a
RPM for
Fedora here: http://koji.fedoraproject.org/koji/buildinfo?buildID=514739.
John, do you think we could build python-nss 0.15 also for Fedora 20? The fix
incorporated in it is pretty important to warrant bugfix update in bodhi. It
would also allow FreeIPA 4.0 to be installed on Fedora 20.
Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does not
work,
because https://fedorahosted.org/freeipa/ticket/4370.
This worked for me, I suspect this is a DS bug. More info in the ticket.
Now back to review of the patch:
210.4: looks ok
234.4:
1) Virtual operation request certificate with subjectaltname should be a
member of Certificate Administrators privilege
OK.
2) I would write Request Certificate With SubjectAltName with with instead
of With. I.e.:
Request Certificate with SubjectAltName
OK.
3) Why is the extension check only enforced for non-hosts?
+if not bind_principal.startswith('host/'):
+for ext in extensions:
+operation = self._allowed_extensions.get(ext)
+if operation:
+self.check_access(operation)
My tricky certmonger request passed:
# ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g
2048 -r
-N cn=`hostname` -K host/`hostname` -D test1.example.com -D
test2.example.com
-E mko...@redhat.com
while when I posted the same CSR as privileged user, it was rejected:
$ klist
Ticket cache: KEYRING:persistent:96203:96203
Default principal: f...@idm.lab.eng.brq.redhat.com
$ ipa cert-request --principal test/`hostname` certmonger.csr
ipa: ERROR: invalid 'csr': subject alt name type RFC822 Name is forbidden
Right, I meant to NACK myself, but you were faster. Fixed.
4) Regular users cannot read Virtual Operations, so even if I assign them a
permission, command is refused:
$ ipa cert-request --principal test/`hostname` openssl.csr
ipa: ERROR: Insufficient access: No such virtual command
I think we will need to create new managed permission Read Virtual
Operations
and assign it to Certificate Administrators privilege by default as this
privilege has the virtual operation permissions assigned. Petr3, what is your
take on this?
Otherwise the patch worked well for me, the authorization part looked OK. My
biggest concern was just the host/user differentiation wrt unknown
subjectaltname types.
Martin
Updated patches attached.
1) I could not compile the patch set:
$ make rpms
...
if [ != yes ]; then \
./makeapi --validate; \
./makeaci --validate; \
fi
Traceback (most recent call last):
File ./makeapi, line 457, in module
sys.exit(main())
File ./makeapi, line 428, in main
api.finalize()
File
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 06/16/2014 02:57 PM, Jan Cholasta wrote:
On 16.6.2014 13:31, Martin Kosek wrote:
On 06/11/2014 02:59 PM, Jan Cholasta wrote:
On 11.6.2014 13:29, Martin Kosek wrote:
On 06/11/2014 10:58 AM, Jan Cholasta wrote:
On 10.6.2014 09:55, Martin Kosek wrote:
On 06/06/2014 12:50 PM, Jan Cholasta wrote:
On 23.1.2014 14:34, Jan Cholasta wrote:
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing
managedby_host
to needing write access to an entry, I am not sure I understand
why that
was changed. not saying it is necessarily wrong, but why the
original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has
write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for
write
access to the SAN services, so that access control can be
(partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the
right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write
permission
when the host is in the managed-by attribute, is that the reasoning
?
Exactly. The ACIs that allow this by default are named Hosts can
manage
service Certificates and kerberos keys and Hosts can manage other
host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that
requesting
certificate with SAN is allowed only to users which have write
access to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a
try
for a formal ack.
OK, thanks.
Bump.
I have added stricter validation of subject alt names as well as
certificate
extensions in general to the second patch.
Thanks!
Updated patches attached.
Note that you will need python-nss 0.15 in order to test, you can get a
RPM for
Fedora here:
http://koji.fedoraproject.org/koji/buildinfo?buildID=514739.
John, do you think we could build python-nss 0.15 also for Fedora 20? The
fix
incorporated in it is pretty important to warrant bugfix update in
bodhi. It
would also allow FreeIPA 4.0 to be installed on Fedora 20.
Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does
not
work,
because https://fedorahosted.org/freeipa/ticket/4370.
This worked for me, I suspect this is a DS bug. More info in the ticket.
Now back to review of the patch:
210.4: looks ok
234.4:
1) Virtual operation request certificate with subjectaltname should be
a
member of Certificate Administrators privilege
OK.
2) I would write Request Certificate With SubjectAltName with with
instead
of With. I.e.:
Request Certificate with SubjectAltName
OK.
3) Why is the extension check only enforced for non-hosts?
+if not bind_principal.startswith('host/'):
+for ext in extensions:
+operation = self._allowed_extensions.get(ext)
+if operation:
+self.check_access(operation)
My tricky certmonger request passed:
# ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g
2048 -r
-N cn=`hostname` -K host/`hostname` -D test1.example.com -D
test2.example.com
-E mko...@redhat.com
while when I posted the same CSR as privileged user, it was rejected:
$ klist
Ticket cache: KEYRING:persistent:96203:96203
Default principal: f...@idm.lab.eng.brq.redhat.com
$ ipa cert-request --principal test/`hostname` certmonger.csr
ipa: ERROR: invalid 'csr': subject alt name type RFC822 Name is forbidden
Right, I meant to NACK myself, but you were faster. Fixed.
4) Regular users cannot read Virtual Operations, so even if I assign
them a
permission, command is refused:
$ ipa cert-request --principal test/`hostname` openssl.csr
ipa: ERROR: Insufficient access: No such virtual command
I think we will need to create new managed permission Read Virtual
Operations
and assign it to Certificate Administrators privilege by default as
this
privilege has the virtual operation permissions assigned. Petr3, what is
your
take on this?
Otherwise the patch worked well for me, the authorization part looked
OK. My
biggest concern was just the host/user differentiation wrt unknown
subjectaltname types.
Martin
Updated patches attached.
1) I could not compile the patch set:
$ make rpms
...
if [ != yes ]; then \
./makeapi --validate; \
./makeaci --validate; \
fi
Traceback (most recent call
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 06/10/2014 09:55 AM, Martin Kosek wrote: On 06/06/2014 12:50 PM, Jan Cholasta wrote: ... Updated patches attached. Note that you will need python-nss 0.15 in order to test, you can get a RPM for Fedora here: http://koji.fedoraproject.org/koji/buildinfo?buildID=514739. John, do you think we could build python-nss 0.15 also for Fedora 20? The fix incorporated in it is pretty important to warrant bugfix update in bodhi. It would also allow FreeIPA 4.0 to be installed on Fedora 20. John kindly built python-nss also for Fedora 20: https://admin.fedoraproject.org/updates/python-nss-0.15.0-1.fc20 I did a quick test and parsing SAN still worked fine, so I gave karma. I would like to ask others who test the library to do the same as karma will be required for it to move to stable updates repo. Thanks, Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 10.6.2014 09:55, Martin Kosek wrote:
On 06/06/2014 12:50 PM, Jan Cholasta wrote:
On 23.1.2014 14:34, Jan Cholasta wrote:
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing
managedby_host
to needing write access to an entry, I am not sure I understand
why that
was changed. not saying it is necessarily wrong, but why the
original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for write
access to the SAN services, so that access control can be (partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write permission
when the host is in the managed-by attribute, is that the reasoning ?
Exactly. The ACIs that allow this by default are named Hosts can manage
service Certificates and kerberos keys and Hosts can manage other host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that requesting
certificate with SAN is allowed only to users which have write access to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a try
for a formal ack.
OK, thanks.
Bump.
I have added stricter validation of subject alt names as well as certificate
extensions in general to the second patch.
Thanks!
Updated patches attached.
Note that you will need python-nss 0.15 in order to test, you can get a RPM for
Fedora here: http://koji.fedoraproject.org/koji/buildinfo?buildID=514739.
John, do you think we could build python-nss 0.15 also for Fedora 20? The fix
incorporated in it is pretty important to warrant bugfix update in bodhi. It
would also allow FreeIPA 4.0 to be installed on Fedora 20.
Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does not work,
because https://fedorahosted.org/freeipa/ticket/4370.
This worked for me, I suspect this is a DS bug. More info in the ticket.
Now back to review of the patch:
210.4: looks ok
234.4:
1) Virtual operation request certificate with subjectaltname should be a
member of Certificate Administrators privilege
OK.
2) I would write Request Certificate With SubjectAltName with with instead
of With. I.e.:
Request Certificate with SubjectAltName
OK.
3) Why is the extension check only enforced for non-hosts?
+if not bind_principal.startswith('host/'):
+for ext in extensions:
+operation = self._allowed_extensions.get(ext)
+if operation:
+self.check_access(operation)
My tricky certmonger request passed:
# ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 -r
-N cn=`hostname` -K host/`hostname` -D test1.example.com -D test2.example.com
-E mko...@redhat.com
while when I posted the same CSR as privileged user, it was rejected:
$ klist
Ticket cache: KEYRING:persistent:96203:96203
Default principal: f...@idm.lab.eng.brq.redhat.com
$ ipa cert-request --principal test/`hostname` certmonger.csr
ipa: ERROR: invalid 'csr': subject alt name type RFC822 Name is forbidden
Right, I meant to NACK myself, but you were faster. Fixed.
4) Regular users cannot read Virtual Operations, so even if I assign them a
permission, command is refused:
$ ipa cert-request --principal test/`hostname` openssl.csr
ipa: ERROR: Insufficient access: No such virtual command
I think we will need to create new managed permission Read Virtual Operations
and assign it to Certificate Administrators privilege by default as this
privilege has the virtual operation permissions assigned. Petr3, what is your
take on this?
Otherwise the patch worked well for me, the authorization part looked OK. My
biggest concern was just the host/user differentiation wrt unknown
subjectaltname types.
Martin
Updated patches attached.
--
Jan Cholasta
From 16cdfdee68c39caf00bbc44e881053940592a567 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 5 Dec 2013 14:34:14 +0100
Subject: [PATCH 1/2] Allow SAN in IPA certificate profile.
https://fedorahosted.org/freeipa/ticket/3977
---
install/tools/ipa-upgradeconfig | 7 +-
ipaserver/install/cainstance.py | 51 +
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/install/tools/ipa-upgradeconfig
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 11.6.2014 13:29, Martin Kosek wrote:
On 06/11/2014 10:58 AM, Jan Cholasta wrote:
On 10.6.2014 09:55, Martin Kosek wrote:
On 06/06/2014 12:50 PM, Jan Cholasta wrote:
On 23.1.2014 14:34, Jan Cholasta wrote:
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing
managedby_host
to needing write access to an entry, I am not sure I understand
why that
was changed. not saying it is necessarily wrong, but why the
original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for write
access to the SAN services, so that access control can be (partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write permission
when the host is in the managed-by attribute, is that the reasoning ?
Exactly. The ACIs that allow this by default are named Hosts can manage
service Certificates and kerberos keys and Hosts can manage other host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that requesting
certificate with SAN is allowed only to users which have write access to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a try
for a formal ack.
OK, thanks.
Bump.
I have added stricter validation of subject alt names as well as certificate
extensions in general to the second patch.
Thanks!
Updated patches attached.
Note that you will need python-nss 0.15 in order to test, you can get a RPM for
Fedora here: http://koji.fedoraproject.org/koji/buildinfo?buildID=514739.
John, do you think we could build python-nss 0.15 also for Fedora 20? The fix
incorporated in it is pretty important to warrant bugfix update in bodhi. It
would also allow FreeIPA 4.0 to be installed on Fedora 20.
Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does not work,
because https://fedorahosted.org/freeipa/ticket/4370.
This worked for me, I suspect this is a DS bug. More info in the ticket.
Now back to review of the patch:
210.4: looks ok
234.4:
1) Virtual operation request certificate with subjectaltname should be a
member of Certificate Administrators privilege
OK.
2) I would write Request Certificate With SubjectAltName with with instead
of With. I.e.:
Request Certificate with SubjectAltName
OK.
3) Why is the extension check only enforced for non-hosts?
+if not bind_principal.startswith('host/'):
+for ext in extensions:
+operation = self._allowed_extensions.get(ext)
+if operation:
+self.check_access(operation)
My tricky certmonger request passed:
# ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 -r
-N cn=`hostname` -K host/`hostname` -D test1.example.com -D test2.example.com
-E mko...@redhat.com
while when I posted the same CSR as privileged user, it was rejected:
$ klist
Ticket cache: KEYRING:persistent:96203:96203
Default principal: f...@idm.lab.eng.brq.redhat.com
$ ipa cert-request --principal test/`hostname` certmonger.csr
ipa: ERROR: invalid 'csr': subject alt name type RFC822 Name is forbidden
Right, I meant to NACK myself, but you were faster. Fixed.
4) Regular users cannot read Virtual Operations, so even if I assign them a
permission, command is refused:
$ ipa cert-request --principal test/`hostname` openssl.csr
ipa: ERROR: Insufficient access: No such virtual command
I think we will need to create new managed permission Read Virtual Operations
and assign it to Certificate Administrators privilege by default as this
privilege has the virtual operation permissions assigned. Petr3, what is your
take on this?
Otherwise the patch worked well for me, the authorization part looked OK. My
biggest concern was just the host/user differentiation wrt unknown
subjectaltname types.
Martin
Updated patches attached.
1) I could not compile the patch set:
$ make rpms
...
if [ != yes ]; then \
./makeapi --validate; \
./makeaci --validate; \
fi
Traceback (most recent call last):
File ./makeapi, line 457, in module
sys.exit(main())
File ./makeapi, line 428, in main
api.finalize()
File /root/freeipa-master/ipalib/plugable.py, line 708, in finalize
self.__do_if_not_done('load_plugins')
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 23.1.2014 14:34, Jan Cholasta wrote:
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing
managedby_host
to needing write access to an entry, I am not sure I understand
why that
was changed. not saying it is necessarily wrong, but why the
original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for write
access to the SAN services, so that access control can be (partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write permission
when the host is in the managed-by attribute, is that the reasoning ?
Exactly. The ACIs that allow this by default are named Hosts can manage
service Certificates and kerberos keys and Hosts can manage other host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that requesting
certificate with SAN is allowed only to users which have write access to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a try
for a formal ack.
OK, thanks.
Bump.
I have added stricter validation of subject alt names as well as
certificate extensions in general to the second patch.
Updated patches attached.
Note that you will need python-nss 0.15 in order to test, you can get a
RPM for Fedora here:
http://koji.fedoraproject.org/koji/buildinfo?buildID=514739.
Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does
not work, because https://fedorahosted.org/freeipa/ticket/4370.
--
Jan Cholasta
From d5393642e6492af32d095ed2562be12f93b6be22 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 5 Dec 2013 14:34:14 +0100
Subject: [PATCH 1/2] Allow SAN in IPA certificate profile.
https://fedorahosted.org/freeipa/ticket/3977
---
install/tools/ipa-upgradeconfig | 7 +-
ipaserver/install/cainstance.py | 51 +
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 265d71c..431cca0 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -328,9 +328,14 @@ def upgrade_ipa_profile(ca, domain, fqdn):
root_logger.debug('Subject Key Identifier updated.')
else:
root_logger.debug('Subject Key Identifier already set.')
+san = ca.enable_subject_alternative_name()
+if san:
+root_logger.debug('Subject Alternative Name updated.')
+else:
+root_logger.debug('Subject Alternative Name already set.')
audit = ca.set_audit_renewal()
uri = ca.set_crl_ocsp_extensions(domain, fqdn)
-if audit or ski or uri:
+if audit or ski or san or uri:
return True
else:
root_logger.info('CA is not configured')
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index f528704..755d63b 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -462,6 +462,7 @@ class CAInstance(service.Service):
self.step(setting up signing cert profile, self.__setup_sign_profile)
self.step(set certificate subject base, self.__set_subject_in_config)
self.step(enabling Subject Key Identifier, self.enable_subject_key_identifier)
+self.step(enabling Subject Alternative Name, self.enable_subject_alternative_name)
self.step(enabling CRL and OCSP extensions for certificates, self.__set_crl_ocsp_extensions)
self.step(setting audit signing renewal to 2 years, self.set_audit_renewal)
self.step(configuring certificate server to start on boot, self.__enable)
@@ -1196,6 +1197,8 @@ class CAInstance(service.Service):
new_set_list = '1,2,3,4,5,6,7,8,9'
elif setlist == '1,2,3,4,5,6,7,8,10':
new_set_list = '1,2,3,4,5,6,7,8,9,10'
+elif setlist == '1,2,3,4,5,6,7,8,10,11':
+new_set_list = '1,2,3,4,5,6,7,8,9,10,11'
if new_set_list:
installutils.set_directive(self.dogtag_constants.IPA_SERVICE_PROFILE,
@@ -1511,6 +1514,54 @@ class CAInstance(service.Service):
# No update was done
return False
+def
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 22.1.2014 16:43, Simo Sorce wrote:
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
On 22.1.2014 15:34, Simo Sorce wrote:
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
On 21.1.2014 17:12, Simo Sorce wrote:
Later in the patch you seem to be changing from needing managedby_host
to needing write access to an entry, I am not sure I understand why that
was changed. not saying it is necessarily wrong, but why the original
check is not right anymore ?
The original check is wrong, see
https://fedorahosted.org/freeipa/ticket/3977#comment:23.
The check in my patch allows SAN only if the requesting host has write
access to all of the SAN services. I'm not entirely sure if this is
right, but even if it is not, I think we should still check for write
access to the SAN services, so that access control can be (partially)
handled by ACIs.
Right, I remembered that comment, but it just says to check the right
object's managed-by, here instead you changed it to check if you can
write the usercertificate.
I guess it is the same *if* there is an ACI that gives write permission
when the host is in the managed-by attribute, is that the reasoning ?
Exactly. The ACIs that allow this by default are named Hosts can manage
service Certificates and kerberos keys and Hosts can manage other host
Certificates and kerberos keys.
I think the check can be extended to users as well, so that requesting
certificate with SAN is allowed only to users which have write access to
the SAN services.
I have done the modification, see attached patches.
Sounds good to me then, thanks for explaining.
The patches also look good, but I would like someone to give them a try
for a formal ack.
OK, thanks.
Simo.
--
Jan Cholasta
From 49c49dbd4cb004934cbc3b66aad382086a5a7b8a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 5 Dec 2013 14:34:14 +0100
Subject: [PATCH 1/2] Allow SAN in IPA certificate profile.
https://fedorahosted.org/freeipa/ticket/3977
---
install/tools/ipa-upgradeconfig | 7 +-
ipaserver/install/cainstance.py | 51 +
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index b281eb4..a9220c1 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -328,9 +328,14 @@ def upgrade_ipa_profile(ca, domain, fqdn):
root_logger.debug('Subject Key Identifier updated.')
else:
root_logger.debug('Subject Key Identifier already set.')
+san = ca.enable_subject_alternative_name()
+if san:
+root_logger.debug('Subject Alternative Name updated.')
+else:
+root_logger.debug('Subject Alternative Name already set.')
audit = ca.set_audit_renewal()
uri = ca.set_crl_ocsp_extensions(domain, fqdn)
-if audit or ski or uri:
+if audit or ski or san or uri:
return True
else:
root_logger.info('CA is not configured')
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 52c91b6..d1a4d45 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -460,6 +460,7 @@ class CAInstance(service.Service):
self.step(setting up signing cert profile, self.__setup_sign_profile)
self.step(set certificate subject base, self.__set_subject_in_config)
self.step(enabling Subject Key Identifier, self.enable_subject_key_identifier)
+self.step(enabling Subject Alternative Name, self.enable_subject_alternative_name)
self.step(enabling CRL and OCSP extensions for certificates, self.__set_crl_ocsp_extensions)
self.step(setting audit signing renewal to 2 years, self.set_audit_renewal)
self.step(configuring certificate server to start on boot, self.__enable)
@@ -1207,6 +1208,8 @@ class CAInstance(service.Service):
new_set_list = '1,2,3,4,5,6,7,8,9'
elif setlist == '1,2,3,4,5,6,7,8,10':
new_set_list = '1,2,3,4,5,6,7,8,9,10'
+elif setlist == '1,2,3,4,5,6,7,8,10,11':
+new_set_list = '1,2,3,4,5,6,7,8,9,10,11'
if new_set_list:
installutils.set_directive(self.dogtag_constants.IPA_SERVICE_PROFILE,
@@ -1526,6 +1529,54 @@ class CAInstance(service.Service):
# No update was done
return False
+def enable_subject_alternative_name(self):
+
+See if Subject Alternative Name is set in the profile and if not, add
+it.
+
+setlist = installutils.get_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.list', separator='=')
+
+# this is the default setting from pki-ca/pki-tomcat. Don't touch it
+# if a user has manually modified it.
+if setlist == '1,2,3,4,5,6,7,8,10' or setlist ==
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 21.1.2014 17:12, Simo Sorce wrote: On Tue, 2014-01-21 at 14:02 +0100, Jan Cholasta wrote: +request = None +try: +request = pkcs10.load_certificate_request(csr) +subject = pkcs10.get_subject(request) +subjectaltname = pkcs10.get_subjectaltname(request) Will this make the request fail if there is no subjectaltname ? No. Later in the patch you seem to be changing from needing managedby_host to needing write access to an entry, I am not sure I understand why that was changed. not saying it is necessarily wrong, but why the original check is not right anymore ? The original check is wrong, see https://fedorahosted.org/freeipa/ticket/3977#comment:23. The check in my patch allows SAN only if the requesting host has write access to all of the SAN services. I'm not entirely sure if this is right, but even if it is not, I think we should still check for write access to the SAN services, so that access control can be (partially) handled by ACIs. Simo. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote: On 21.1.2014 17:12, Simo Sorce wrote: On Tue, 2014-01-21 at 14:02 +0100, Jan Cholasta wrote: +request = None +try: +request = pkcs10.load_certificate_request(csr) +subject = pkcs10.get_subject(request) +subjectaltname = pkcs10.get_subjectaltname(request) Will this make the request fail if there is no subjectaltname ? No. Good. Later in the patch you seem to be changing from needing managedby_host to needing write access to an entry, I am not sure I understand why that was changed. not saying it is necessarily wrong, but why the original check is not right anymore ? The original check is wrong, see https://fedorahosted.org/freeipa/ticket/3977#comment:23. The check in my patch allows SAN only if the requesting host has write access to all of the SAN services. I'm not entirely sure if this is right, but even if it is not, I think we should still check for write access to the SAN services, so that access control can be (partially) handled by ACIs. Right, I remembered that comment, but it just says to check the right object's managed-by, here instead you changed it to check if you can write the usercertificate. I guess it is the same *if* there is an ACI that gives write permission when the host is in the managed-by attribute, is that the reasoning ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 22.1.2014 15:34, Simo Sorce wrote: On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote: On 21.1.2014 17:12, Simo Sorce wrote: On Tue, 2014-01-21 at 14:02 +0100, Jan Cholasta wrote: +request = None +try: +request = pkcs10.load_certificate_request(csr) +subject = pkcs10.get_subject(request) +subjectaltname = pkcs10.get_subjectaltname(request) Will this make the request fail if there is no subjectaltname ? No. Good. Later in the patch you seem to be changing from needing managedby_host to needing write access to an entry, I am not sure I understand why that was changed. not saying it is necessarily wrong, but why the original check is not right anymore ? The original check is wrong, see https://fedorahosted.org/freeipa/ticket/3977#comment:23. The check in my patch allows SAN only if the requesting host has write access to all of the SAN services. I'm not entirely sure if this is right, but even if it is not, I think we should still check for write access to the SAN services, so that access control can be (partially) handled by ACIs. Right, I remembered that comment, but it just says to check the right object's managed-by, here instead you changed it to check if you can write the usercertificate. I guess it is the same *if* there is an ACI that gives write permission when the host is in the managed-by attribute, is that the reasoning ? Exactly. The ACIs that allow this by default are named Hosts can manage service Certificates and kerberos keys and Hosts can manage other host Certificates and kerberos keys. I think the check can be extended to users as well, so that requesting certificate with SAN is allowed only to users which have write access to the SAN services. Simo. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote: On 22.1.2014 15:34, Simo Sorce wrote: On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote: On 21.1.2014 17:12, Simo Sorce wrote: On Tue, 2014-01-21 at 14:02 +0100, Jan Cholasta wrote: +request = None +try: +request = pkcs10.load_certificate_request(csr) +subject = pkcs10.get_subject(request) +subjectaltname = pkcs10.get_subjectaltname(request) Will this make the request fail if there is no subjectaltname ? No. Good. Later in the patch you seem to be changing from needing managedby_host to needing write access to an entry, I am not sure I understand why that was changed. not saying it is necessarily wrong, but why the original check is not right anymore ? The original check is wrong, see https://fedorahosted.org/freeipa/ticket/3977#comment:23. The check in my patch allows SAN only if the requesting host has write access to all of the SAN services. I'm not entirely sure if this is right, but even if it is not, I think we should still check for write access to the SAN services, so that access control can be (partially) handled by ACIs. Right, I remembered that comment, but it just says to check the right object's managed-by, here instead you changed it to check if you can write the usercertificate. I guess it is the same *if* there is an ACI that gives write permission when the host is in the managed-by attribute, is that the reasoning ? Exactly. The ACIs that allow this by default are named Hosts can manage service Certificates and kerberos keys and Hosts can manage other host Certificates and kerberos keys. I think the check can be extended to users as well, so that requesting certificate with SAN is allowed only to users which have write access to the SAN services. Sounds good to me then, thanks for explaining. The patches also look good, but I would like someone to give them a try for a formal ack. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 20.1.2014 18:35, Simo Sorce wrote: On Mon, 2014-01-20 at 17:49 +0100, Jan Cholasta wrote: On 20.1.2014 16:36, Simo Sorce wrote: On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote: On 17.1.2014 11:39, Jan Cholasta wrote: On 10.1.2014 13:34, Martin Kosek wrote: On 01/09/2014 04:49 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote: Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. They can remove the host right after they create a cert, I honestly do not think this is a valid concern. If your admin is rouge he can already take full ownership of your infrastructure in many ways, preventing setting a name in a cert doesn't really make a difference IMO. However I would be ok to limit this to some new Security Admin/CA Admin role that is not assigned by default. Simo. Ok, let's reach some conclusion here. I would really like to not defer this feature for too long, it is quite wanted. Would creating new virtual operation Request certificate with SAN make the situation better? It would not be so difficult to do, the check_access function can already access virtual operation name as a parameter, we just need to call it. Why don't we treat SAN hostnames the same way as the subject hostname? The way I see it, with SAN the only difference is that there is a set of hostnames instead of just a single hostname, so maybe we should support requesting a certificate for a set of hosts/services instead of just a single host/service. As far as authorization is concerned, currently you can request a certificate for a single host/service, if you have the Request certificate permission and write access to the host/service entry. With multiple hosts/services, you would be able to request a certificate if you have the Request certificate permission and write access to *all* of the host/certificate entries you are requesting the certificate for. Effectively this means that cert-request would accept multiple principals instead of single principal and the automatic revocation code in cert-request, host-del and service-del would take into account that a single certificate might be assigned to multiple entities. See attachment for patch which implements the above design. Hi Jan, I am a bit too far removed from the code to fully understand the change just from the diff. Could you please add a short explanation in the commit message, a bout what the code does now differently than it did before. Done. For example although I understand the checks you do on subjectname +subjectaltname I do not know where the principals come from or why you have a comment that says principals must all be of the same service type. Principals come from the --principal option, which can have multiple values with the patch. The service name constraint is there so that there is a 1-to-1 mapping between principals and hostnames in the request (both subject and SAN). Without this you would be able to request a certificate for completely unrelated services and I was not sure if it's OK to allow that, since the use case here is load balancing (right?) Simo. Updated patch attached. Sorry have to NACK. I was confused by the code so asked on IRC: simo so if I have subectname = one.ipa.lan and altname = two.ipa.net then the certificate is anchored to both HTTP/one.ipa.net AND HTTP/two.ipa.net ? jcholast yep simo and what happens when I create other.ipa.net with altname two.ipa.net ? simo does HTTP/two.ipa.net now have 2 certificates anchored ? jcholast the old certificate gets revoked and removed from HTTP/one.ipa.net and HTTP/two.ipa.net, then the new certificate is issued and anchored to HTTP/two.ipa.net and HTTP/other.ipa.net This defeats the purpose of having altnames. There are 2 reasons to have altnames: 1. just add aliases that are not shared by any other service 2. have a common alias between multiple service to allow loadbalancing (1) will not be affected, but (2) would be impossible, because as soon as I try to create the cert for one of the other nodes to be balanced the previous nodes get their certificates revoked, which defeats the whole point of creating them with a common altname. Simo. Updated patches attached. -- Jan Cholasta From 8c0d512b077b1e62af1be5c11f786b8458856e3b Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 5 Dec 2013 14:34:14 +0100 Subject: [PATCH 1/2] Allow SAN in IPA certificate profile. https://fedorahosted.org/freeipa/ticket/3977 --- install/tools/ipa-upgradeconfig | 7 +- ipaserver/install/cainstance.py | 51 + 2 files changed, 57
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Tue, 2014-01-21 at 14:02 +0100, Jan Cholasta wrote: +request = None +try: +request = pkcs10.load_certificate_request(csr) +subject = pkcs10.get_subject(request) +subjectaltname = pkcs10.get_subjectaltname(request) Will this make the request fail if there is no subjectaltname ? Later in the patch you seem to be changing from needing managedby_host to needing write access to an entry, I am not sure I understand why that was changed. not saying it is necessarily wrong, but why the original check is not right anymore ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 17.1.2014 11:39, Jan Cholasta wrote:
On 10.1.2014 13:34, Martin Kosek wrote:
On 01/09/2014 04:49 PM, Simo Sorce wrote:
On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/09/2014 03:12 PM, Simo Sorce wrote:
Also maybe we should allow admins to bypass the need to have an
actual
object to represent the alt name ?
I'd rather not. This would allow a rogue admin to create a cert for
www.google.com. Sure, they could also create a host for that but
forcing
them to add more entries increases the chances of them getting caught
doing it.
They can remove the host right after they create a cert, I honestly do
not think this is a valid concern. If your admin is rouge he can already
take full ownership of your infrastructure in many ways, preventing
setting a name in a cert doesn't really make a difference IMO.
However I would be ok to limit this to some new Security Admin/CA
Admin role that is not assigned by default.
Simo.
Ok, let's reach some conclusion here. I would really like to not defer
this
feature for too long, it is quite wanted. Would creating new virtual
operation
Request certificate with SAN make the situation better? It would not
be so
difficult to do, the check_access function can already access virtual
operation
name as a parameter, we just need to call it.
Why don't we treat SAN hostnames the same way as the subject hostname?
The way I see it, with SAN the only difference is that there is a set of
hostnames instead of just a single hostname, so maybe we should support
requesting a certificate for a set of hosts/services instead of just a
single host/service.
As far as authorization is concerned, currently you can request a
certificate for a single host/service, if you have the Request
certificate permission and write access to the host/service entry. With
multiple hosts/services, you would be able to request a certificate if
you have the Request certificate permission and write access to *all*
of the host/certificate entries you are requesting the certificate for.
Effectively this means that cert-request would accept multiple
principals instead of single principal and the automatic revocation code
in cert-request, host-del and service-del would take into account that a
single certificate might be assigned to multiple entities.
See attachment for patch which implements the above design.
--
Jan Cholasta
From bb95b3afd6786adc04aa4cd5ac114fbace964907 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 20 Jan 2014 10:59:08 +0100
Subject: [PATCH] Support SAN in cert-request.
https://fedorahosted.org/freeipa/ticket/3977
---
API.txt| 2 +-
VERSION| 3 +-
ipalib/plugins/cert.py | 191 +++--
3 files changed, 107 insertions(+), 89 deletions(-)
diff --git a/API.txt b/API.txt
index a6c3aed..015d829 100644
--- a/API.txt
+++ b/API.txt
@@ -479,7 +479,7 @@ command: cert_request
args: 1,4,1
arg: File('csr', cli_name='csr_file')
option: Flag('add', autofill=True, default=False)
-option: Str('principal')
+option: Str('principal+')
option: Str('request_type', autofill=True, default=u'pkcs10')
option: Str('version?', exclude='webui')
output: Output('result', type 'dict', None)
diff --git a/VERSION b/VERSION
index 5ce16b5..c9f06d1 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=72
+IPA_API_VERSION_MINOR=73
+# Last change: jcholast - SAN certificate requests
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index 762f13b..d343f99 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -249,7 +249,7 @@ class cert_request(VirtualCommand):
operation=request certificate
takes_options = (
-Str('principal',
+Str('principal+',
label=_('Principal'),
doc=_('Service principal for this certificate (e.g. HTTP/test.example.com)'),
),
@@ -301,13 +301,7 @@ class cert_request(VirtualCommand):
),
)
-def execute(self, csr, **kw):
-ldap = self.api.Backend.ldap2
-principal = kw.get('principal')
-add = kw.get('add')
-request_type = kw.get('request_type')
-service = None
-
+def execute(self, csr, **options):
Access control is partially handled by the ACI titled
'Hosts can modify service userCertificate'. This is for the case
@@ -324,73 +318,101 @@ class cert_request(VirtualCommand):
if not bind_principal.startswith('host/'):
self.check_access()
-# FIXME: add support for subject alt name
-
-# Ensure that the hostname in the CSR matches the principal
-subject_host = get_csr_hostname(csr)
-if not subject_host:
-raise
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote: On 17.1.2014 11:39, Jan Cholasta wrote: On 10.1.2014 13:34, Martin Kosek wrote: On 01/09/2014 04:49 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote: Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. They can remove the host right after they create a cert, I honestly do not think this is a valid concern. If your admin is rouge he can already take full ownership of your infrastructure in many ways, preventing setting a name in a cert doesn't really make a difference IMO. However I would be ok to limit this to some new Security Admin/CA Admin role that is not assigned by default. Simo. Ok, let's reach some conclusion here. I would really like to not defer this feature for too long, it is quite wanted. Would creating new virtual operation Request certificate with SAN make the situation better? It would not be so difficult to do, the check_access function can already access virtual operation name as a parameter, we just need to call it. Why don't we treat SAN hostnames the same way as the subject hostname? The way I see it, with SAN the only difference is that there is a set of hostnames instead of just a single hostname, so maybe we should support requesting a certificate for a set of hosts/services instead of just a single host/service. As far as authorization is concerned, currently you can request a certificate for a single host/service, if you have the Request certificate permission and write access to the host/service entry. With multiple hosts/services, you would be able to request a certificate if you have the Request certificate permission and write access to *all* of the host/certificate entries you are requesting the certificate for. Effectively this means that cert-request would accept multiple principals instead of single principal and the automatic revocation code in cert-request, host-del and service-del would take into account that a single certificate might be assigned to multiple entities. See attachment for patch which implements the above design. Hi Jan, I am a bit too far removed from the code to fully understand the change just from the diff. Could you please add a short explanation in the commit message, a bout what the code does now differently than it did before. For example although I understand the checks you do on subjectname +subjectaltname I do not know where the principals come from or why you have a comment that says principals must all be of the same service type. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 20.1.2014 16:36, Simo Sorce wrote:
On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote:
On 17.1.2014 11:39, Jan Cholasta wrote:
On 10.1.2014 13:34, Martin Kosek wrote:
On 01/09/2014 04:49 PM, Simo Sorce wrote:
On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On 01/09/2014 03:12 PM, Simo Sorce wrote:
Also maybe we should allow admins to bypass the need to have an
actual
object to represent the alt name ?
I'd rather not. This would allow a rogue admin to create a cert for
www.google.com. Sure, they could also create a host for that but
forcing
them to add more entries increases the chances of them getting caught
doing it.
They can remove the host right after they create a cert, I honestly do
not think this is a valid concern. If your admin is rouge he can already
take full ownership of your infrastructure in many ways, preventing
setting a name in a cert doesn't really make a difference IMO.
However I would be ok to limit this to some new Security Admin/CA
Admin role that is not assigned by default.
Simo.
Ok, let's reach some conclusion here. I would really like to not defer
this
feature for too long, it is quite wanted. Would creating new virtual
operation
Request certificate with SAN make the situation better? It would not
be so
difficult to do, the check_access function can already access virtual
operation
name as a parameter, we just need to call it.
Why don't we treat SAN hostnames the same way as the subject hostname?
The way I see it, with SAN the only difference is that there is a set of
hostnames instead of just a single hostname, so maybe we should support
requesting a certificate for a set of hosts/services instead of just a
single host/service.
As far as authorization is concerned, currently you can request a
certificate for a single host/service, if you have the Request
certificate permission and write access to the host/service entry. With
multiple hosts/services, you would be able to request a certificate if
you have the Request certificate permission and write access to *all*
of the host/certificate entries you are requesting the certificate for.
Effectively this means that cert-request would accept multiple
principals instead of single principal and the automatic revocation code
in cert-request, host-del and service-del would take into account that a
single certificate might be assigned to multiple entities.
See attachment for patch which implements the above design.
Hi Jan, I am a bit too far removed from the code to fully understand the
change just from the diff.
Could you please add a short explanation in the commit message, a bout
what the code does now differently than it did before.
Done.
For example although I understand the checks you do on subjectname
+subjectaltname I do not know where the principals come from or why you
have a comment that says principals must all be of the same service
type.
Principals come from the --principal option, which can have multiple
values with the patch.
The service name constraint is there so that there is a 1-to-1 mapping
between principals and hostnames in the request (both subject and SAN).
Without this you would be able to request a certificate for completely
unrelated services and I was not sure if it's OK to allow that, since
the use case here is load balancing (right?)
Simo.
Updated patch attached.
--
Jan Cholasta
From ce8a650b6bd8a3d1c353d31c70fc7edb0af7f1c0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 20 Jan 2014 10:59:08 +0100
Subject: [PATCH] Support requests with SAN in cert-request.
The command now accepts multiple values for the principal option. This makes
it possible to request a certificate for a service load-balanced on multiple
hosts. Each principal's hostname must match a hostname in the request, either
in the subject or a SAN, and all principals must have the same service name.
https://fedorahosted.org/freeipa/ticket/3977
---
API.txt| 2 +-
VERSION| 3 +-
ipalib/plugins/cert.py | 191 +++--
3 files changed, 107 insertions(+), 89 deletions(-)
diff --git a/API.txt b/API.txt
index a6c3aed..015d829 100644
--- a/API.txt
+++ b/API.txt
@@ -479,7 +479,7 @@ command: cert_request
args: 1,4,1
arg: File('csr', cli_name='csr_file')
option: Flag('add', autofill=True, default=False)
-option: Str('principal')
+option: Str('principal+')
option: Str('request_type', autofill=True, default=u'pkcs10')
option: Str('version?', exclude='webui')
output: Output('result', type 'dict', None)
diff --git a/VERSION b/VERSION
index 5ce16b5..c9f06d1 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=72
+IPA_API_VERSION_MINOR=73
+# Last change: jcholast - SAN
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Mon, 2014-01-20 at 17:49 +0100, Jan Cholasta wrote: On 20.1.2014 16:36, Simo Sorce wrote: On Mon, 2014-01-20 at 11:07 +0100, Jan Cholasta wrote: On 17.1.2014 11:39, Jan Cholasta wrote: On 10.1.2014 13:34, Martin Kosek wrote: On 01/09/2014 04:49 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote: Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. They can remove the host right after they create a cert, I honestly do not think this is a valid concern. If your admin is rouge he can already take full ownership of your infrastructure in many ways, preventing setting a name in a cert doesn't really make a difference IMO. However I would be ok to limit this to some new Security Admin/CA Admin role that is not assigned by default. Simo. Ok, let's reach some conclusion here. I would really like to not defer this feature for too long, it is quite wanted. Would creating new virtual operation Request certificate with SAN make the situation better? It would not be so difficult to do, the check_access function can already access virtual operation name as a parameter, we just need to call it. Why don't we treat SAN hostnames the same way as the subject hostname? The way I see it, with SAN the only difference is that there is a set of hostnames instead of just a single hostname, so maybe we should support requesting a certificate for a set of hosts/services instead of just a single host/service. As far as authorization is concerned, currently you can request a certificate for a single host/service, if you have the Request certificate permission and write access to the host/service entry. With multiple hosts/services, you would be able to request a certificate if you have the Request certificate permission and write access to *all* of the host/certificate entries you are requesting the certificate for. Effectively this means that cert-request would accept multiple principals instead of single principal and the automatic revocation code in cert-request, host-del and service-del would take into account that a single certificate might be assigned to multiple entities. See attachment for patch which implements the above design. Hi Jan, I am a bit too far removed from the code to fully understand the change just from the diff. Could you please add a short explanation in the commit message, a bout what the code does now differently than it did before. Done. For example although I understand the checks you do on subjectname +subjectaltname I do not know where the principals come from or why you have a comment that says principals must all be of the same service type. Principals come from the --principal option, which can have multiple values with the patch. The service name constraint is there so that there is a 1-to-1 mapping between principals and hostnames in the request (both subject and SAN). Without this you would be able to request a certificate for completely unrelated services and I was not sure if it's OK to allow that, since the use case here is load balancing (right?) Simo. Updated patch attached. Sorry have to NACK. I was confused by the code so asked on IRC: simo so if I have subectname = one.ipa.lan and altname = two.ipa.net then the certificate is anchored to both HTTP/one.ipa.net AND HTTP/two.ipa.net ? jcholast yep simo and what happens when I create other.ipa.net with altname two.ipa.net ? simo does HTTP/two.ipa.net now have 2 certificates anchored ? jcholast the old certificate gets revoked and removed from HTTP/one.ipa.net and HTTP/two.ipa.net, then the new certificate is issued and anchored to HTTP/two.ipa.net and HTTP/other.ipa.net This defeats the purpose of having altnames. There are 2 reasons to have altnames: 1. just add aliases that are not shared by any other service 2. have a common alias between multiple service to allow loadbalancing (1) will not be affected, but (2) would be impossible, because as soon as I try to create the cert for one of the other nodes to be balanced the previous nodes get their certificates revoked, which defeats the whole point of creating them with a common altname. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 10.1.2014 13:34, Martin Kosek wrote: On 01/09/2014 04:49 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote: Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. They can remove the host right after they create a cert, I honestly do not think this is a valid concern. If your admin is rouge he can already take full ownership of your infrastructure in many ways, preventing setting a name in a cert doesn't really make a difference IMO. However I would be ok to limit this to some new Security Admin/CA Admin role that is not assigned by default. Simo. Ok, let's reach some conclusion here. I would really like to not defer this feature for too long, it is quite wanted. Would creating new virtual operation Request certificate with SAN make the situation better? It would not be so difficult to do, the check_access function can already access virtual operation name as a parameter, we just need to call it. Why don't we treat SAN hostnames the same way as the subject hostname? The way I see it, with SAN the only difference is that there is a set of hostnames instead of just a single hostname, so maybe we should support requesting a certificate for a set of hosts/services instead of just a single host/service. As far as authorization is concerned, currently you can request a certificate for a single host/service, if you have the Request certificate permission and write access to the host/service entry. With multiple hosts/services, you would be able to request a certificate if you have the Request certificate permission and write access to *all* of the host/certificate entries you are requesting the certificate for. Effectively this means that cert-request would accept multiple principals instead of single principal and the automatic revocation code in cert-request, host-del and service-del would take into account that a single certificate might be assigned to multiple entities. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 01/09/2014 03:37 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 15:27 +0100, Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 09:04 -0500, Simo Sorce wrote: On Thu, 2014-01-09 at 09:51 +0100, Martin Kosek wrote: On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). This means that in order to add a subjectAltName you have to register a Host with that name ? That is not really convenient, but if it works at least it properly constrains potential hijacking. Right. The host does not need to even be enrolled to make this work. But it is still better to make it this way that to allow hosts to request SANs. But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Yes we definitely need a test to make 100% sure this cannot be worked around, the security consequences would be disastrous. +1 Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? cert requests issued by admin can bypass this check, it is only applied to principals starting with host/, i.e. for example to certs renewed by certmonger. Other requests are authorized differently. cert-request is a VirtualCommand and people are authorized by checking if they have a write access to the appropriate virtual operation LDAP entry, living in cn=virtual operations,cn=etc,SUFFIX... Which is by default only the admins group members which have the god-like special ACI we discussed yesterday. Rob is that correct? You also participated in the VirtualCommand coding... The situation sounds sub-optimal, I would think that giving managedby to a 'junior-admin' user would allow him to also fetch certs but only for the machines they manage. I sense some confusion here. You do not give managedby to a junior admin. You set managedby to a host or service. Example: ipa host-add lowsecure.example.com ipa host-add highsecure.example.com ipa host-add-managedby highsecure.example.com --hosts lowsecure.example.com Then host/lowsecure.example.com@REALM can request cert with SAN highsecure.example.com. Is the Virtual Permission an all or nothing thing ? or can it be scoped to group of machines ? If the latter, fine, if not, not so much. Not really. But this pain is shared also in the standard permissions. Modify hosts permission also means all or nothing. Although, if the junior-admin can gain root on the host, he can still get certs using the host/ key so maybe that is not fundamental, but it would need clear documentation on how to allow a less privileged admin to perform these functions. We will need this type of functionality if we want to allow admins to create wildcard certificates anyway, which is another important use case for hosting/cloud-like services. I was also thinking admins may want to allow a lower privileged admin to manage a host, but not allow them to add a special subjectaltname to random other hosts he manages. In this case again we need the ability for an admin to be able to provide the cert to the host. Also then a special case arises on automatic renew from certmonger, all names need to be checked against the old certificate being renewed, so authorization in that case would have to be based on the previous cert names and not on managedby, right ? If not automatic renewal would fail ? Hmm, the renewal would fail in this case. admin would have to request new certificate manually in this case (when mangedby attribute is not set properly). Previous certificate is not checked by the cert-request authorization code. I think the renewal problem is more important. Failing renewals cripples one of the most important features of using FreeIPA as your CA for enrolled hosts. Simo. I am still not sure if there is not a confusion about the managedby attribute. If you have an admin of lowsecure.example.com + have the managedby attributes on other hosts correctly, local admin/certmonger on lowsecure.example.com can still happily request or renew cert with the approved SANs. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
Simo Sorce wrote: On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote: Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. They can remove the host right after they create a cert, I honestly do not think this is a valid concern. If your admin is rouge he can already take full ownership of your infrastructure in many ways, preventing setting a name in a cert doesn't really make a difference IMO. However I would be ok to limit this to some new Security Admin/CA Admin role that is not assigned by default. That's an interesting point. So right now if you remove a host we delete its services and revoke its certificates. This wouldn't apply to a SAN though, it probably should. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Thu, 2014-01-09 at 09:51 +0100, Martin Kosek wrote: On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). This means that in order to add a subjectAltName you have to register a Host with that name ? That is not really convenient, but if it works at least it properly constrains potential hijacking. But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Yes we definitely need a test to make 100% sure this cannot be worked around, the security consequences would be disastrous. Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? We will need this type of functionality if we want to allow admins to create wildcard certificates anyway, which is another important use case for hosting/cloud-like services. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Thu, 2014-01-09 at 09:04 -0500, Simo Sorce wrote: On Thu, 2014-01-09 at 09:51 +0100, Martin Kosek wrote: On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). This means that in order to add a subjectAltName you have to register a Host with that name ? That is not really convenient, but if it works at least it properly constrains potential hijacking. But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Yes we definitely need a test to make 100% sure this cannot be worked around, the security consequences would be disastrous. Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? We will need this type of functionality if we want to allow admins to create wildcard certificates anyway, which is another important use case for hosting/cloud-like services. I was also thinking admins may want to allow a lower privileged admin to manage a host, but not allow them to add a special subjectaltname to random other hosts he manages. In this case again we need the ability for an admin to be able to provide the cert to the host. Also then a special case arises on automatic renew from certmonger, all names need to be checked against the old certificate being renewed, so authorization in that case would have to be based on the previous cert names and not on managedby, right ? If not automatic renewal would fail ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On 01/09/2014 03:12 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 09:04 -0500, Simo Sorce wrote: On Thu, 2014-01-09 at 09:51 +0100, Martin Kosek wrote: On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). This means that in order to add a subjectAltName you have to register a Host with that name ? That is not really convenient, but if it works at least it properly constrains potential hijacking. Right. The host does not need to even be enrolled to make this work. But it is still better to make it this way that to allow hosts to request SANs. But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Yes we definitely need a test to make 100% sure this cannot be worked around, the security consequences would be disastrous. +1 Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? cert requests issued by admin can bypass this check, it is only applied to principals starting with host/, i.e. for example to certs renewed by certmonger. Other requests are authorized differently. cert-request is a VirtualCommand and people are authorized by checking if they have a write access to the appropriate virtual operation LDAP entry, living in cn=virtual operations,cn=etc,SUFFIX... Which is by default only the admins group members which have the god-like special ACI we discussed yesterday. Rob is that correct? You also participated in the VirtualCommand coding... We will need this type of functionality if we want to allow admins to create wildcard certificates anyway, which is another important use case for hosting/cloud-like services. I was also thinking admins may want to allow a lower privileged admin to manage a host, but not allow them to add a special subjectaltname to random other hosts he manages. In this case again we need the ability for an admin to be able to provide the cert to the host. Also then a special case arises on automatic renew from certmonger, all names need to be checked against the old certificate being renewed, so authorization in that case would have to be based on the previous cert names and not on managedby, right ? If not automatic renewal would fail ? Hmm, the renewal would fail in this case. admin would have to request new certificate manually in this case (when mangedby attribute is not set properly). Previous certificate is not checked by the cert-request authorization code. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Thu, 2014-01-09 at 15:27 +0100, Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 09:04 -0500, Simo Sorce wrote: On Thu, 2014-01-09 at 09:51 +0100, Martin Kosek wrote: On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). This means that in order to add a subjectAltName you have to register a Host with that name ? That is not really convenient, but if it works at least it properly constrains potential hijacking. Right. The host does not need to even be enrolled to make this work. But it is still better to make it this way that to allow hosts to request SANs. But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Yes we definitely need a test to make 100% sure this cannot be worked around, the security consequences would be disastrous. +1 Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? cert requests issued by admin can bypass this check, it is only applied to principals starting with host/, i.e. for example to certs renewed by certmonger. Other requests are authorized differently. cert-request is a VirtualCommand and people are authorized by checking if they have a write access to the appropriate virtual operation LDAP entry, living in cn=virtual operations,cn=etc,SUFFIX... Which is by default only the admins group members which have the god-like special ACI we discussed yesterday. Rob is that correct? You also participated in the VirtualCommand coding... The situation sounds sub-optimal, I would think that giving managedby to a 'junior-admin' user would allow him to also fetch certs but only for the machines they manage. Is the Virtual Permission an all or nothing thing ? or can it be scoped to group of machines ? If the latter, fine, if not, not so much. Although, if the junior-admin can gain root on the host, he can still get certs using the host/ key so maybe that is not fundamental, but it would need clear documentation on how to allow a less privileged admin to perform these functions. We will need this type of functionality if we want to allow admins to create wildcard certificates anyway, which is another important use case for hosting/cloud-like services. I was also thinking admins may want to allow a lower privileged admin to manage a host, but not allow them to add a special subjectaltname to random other hosts he manages. In this case again we need the ability for an admin to be able to provide the cert to the host. Also then a special case arises on automatic renew from certmonger, all names need to be checked against the old certificate being renewed, so authorization in that case would have to be based on the previous cert names and not on managedby, right ? If not automatic renewal would fail ? Hmm, the renewal would fail in this case. admin would have to request new certificate manually in this case (when mangedby attribute is not set properly). Previous certificate is not checked by the cert-request authorization code. I think the renewal problem is more important. Failing renewals cripples one of the most important features of using FreeIPA as your CA for enrolled hosts. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: On Thu, 2014-01-09 at 09:04 -0500, Simo Sorce wrote: On Thu, 2014-01-09 at 09:51 +0100, Martin Kosek wrote: On 01/09/2014 12:26 AM, Simo Sorce wrote: On Thu, 2013-12-05 at 14:37 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977. See the additional comments on 3977, I think this patch should be NACKed with extreme prejudice if it allows setting arbitrary subjectAltNames. Simo. It does not allow them - SANs are being authorized by using the managedBy attribute on the SAN-ed host/service (i.e. host-add-managedby/service-add-host commands). This means that in order to add a subjectAltName you have to register a Host with that name ? That is not really convenient, but if it works at least it properly constrains potential hijacking. Right. The host does not need to even be enrolled to make this work. But it is still better to make it this way that to allow hosts to request SANs. But you are right that the authorization part should not be taken lightly and should be verified before we allow SANs in default profile. I added a comment in the Trac as well. Yes we definitely need a test to make 100% sure this cannot be worked around, the security consequences would be disastrous. +1 Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. cert requests issued by admin can bypass this check, it is only applied to principals starting with host/, i.e. for example to certs renewed by certmonger. Other requests are authorized differently. cert-request is a VirtualCommand and people are authorized by checking if they have a write access to the appropriate virtual operation LDAP entry, living in cn=virtual operations,cn=etc,SUFFIX... Which is by default only the admins group members which have the god-like special ACI we discussed yesterday. Rob is that correct? You also participated in the VirtualCommand coding... Right. I think I mentioned yesterday that there is currently no special permission for issuing SAN certs (at least partly because we don't actually do it). Adding a rule for this should be fairly straightforward and I think should be part of any effort to enable SAN certs. Doing the ACI will require a bit of work since currently there is a 1-1 relationship between a command and an ACI right now (via self.check_access()). It shouldn't be too much work to be able to pass in another ACI to check though. We will need this type of functionality if we want to allow admins to create wildcard certificates anyway, which is another important use case for hosting/cloud-like services. I was also thinking admins may want to allow a lower privileged admin to manage a host, but not allow them to add a special subjectaltname to random other hosts he manages. In this case again we need the ability for an admin to be able to provide the cert to the host. Also then a special case arises on automatic renew from certmonger, all names need to be checked against the old certificate being renewed, so authorization in that case would have to be based on the previous cert names and not on managedby, right ? If not automatic renewal would fail ? Hmm, the renewal would fail in this case. admin would have to request new certificate manually in this case (when mangedby attribute is not set properly). Previous certificate is not checked by the cert-request authorization code. Yup. If the host can't manage all the hosts in the SAN then renewal will be rejected. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
On Thu, 2014-01-09 at 10:44 -0500, Rob Crittenden wrote: Martin Kosek wrote: On 01/09/2014 03:12 PM, Simo Sorce wrote: Also maybe we should allow admins to bypass the need to have an actual object to represent the alt name ? I'd rather not. This would allow a rogue admin to create a cert for www.google.com. Sure, they could also create a host for that but forcing them to add more entries increases the chances of them getting caught doing it. They can remove the host right after they create a cert, I honestly do not think this is a valid concern. If your admin is rouge he can already take full ownership of your infrastructure in many ways, preventing setting a name in a cert doesn't really make a difference IMO. However I would be ok to limit this to some new Security Admin/CA Admin role that is not assigned by default. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/3977.
Honza
--
Jan Cholasta
From 101547fae92dfa6dea0db34f68cb855f471af54d Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 5 Dec 2013 14:34:14 +0100
Subject: [PATCH] Allow SAN in IPA certificate profile.
https://fedorahosted.org/freeipa/ticket/3977
---
install/tools/ipa-upgradeconfig | 7 +-
ipaserver/install/cainstance.py | 51 +
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index 10526f2..fe39624 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -328,9 +328,14 @@ def upgrade_ipa_profile(ca, domain, fqdn):
root_logger.debug('Subject Key Identifier updated.')
else:
root_logger.debug('Subject Key Identifier already set.')
+san = ca.enable_subject_alternative_name()
+if san:
+root_logger.debug('Subject Alternative Name updated.')
+else:
+root_logger.debug('Subject Alternative Name already set.')
audit = ca.set_audit_renewal()
uri = ca.set_crl_ocsp_extensions(domain, fqdn)
-if audit or ski or uri:
+if audit or ski or san or uri:
return True
else:
root_logger.info('CA is not configured')
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index ac5c81d..54012db 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -460,6 +460,7 @@ class CAInstance(service.Service):
self.step(setting up signing cert profile, self.__setup_sign_profile)
self.step(set certificate subject base, self.__set_subject_in_config)
self.step(enabling Subject Key Identifier, self.enable_subject_key_identifier)
+self.step(enabling Subject Alternative Name, self.enable_subject_alternative_name)
self.step(enabling CRL and OCSP extensions for certificates, self.__set_crl_ocsp_extensions)
self.step(setting audit signing renewal to 2 years, self.set_audit_renewal)
self.step(configuring certificate server to start on boot, self.__enable)
@@ -1207,6 +1208,8 @@ class CAInstance(service.Service):
new_set_list = '1,2,3,4,5,6,7,8,9'
elif setlist == '1,2,3,4,5,6,7,8,10':
new_set_list = '1,2,3,4,5,6,7,8,9,10'
+elif setlist == '1,2,3,4,5,6,7,8,10,11':
+new_set_list = '1,2,3,4,5,6,7,8,9,10,11'
if new_set_list:
installutils.set_directive(self.dogtag_constants.IPA_SERVICE_PROFILE,
@@ -1526,6 +1529,54 @@ class CAInstance(service.Service):
# No update was done
return False
+def enable_subject_alternative_name(self):
+
+See if Subject Alternative Name is set in the profile and if not, add
+it.
+
+setlist = installutils.get_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.list', separator='=')
+
+# this is the default setting from pki-ca/pki-tomcat. Don't touch it
+# if a user has manually modified it.
+if setlist == '1,2,3,4,5,6,7,8,10' or setlist == '1,2,3,4,5,6,7,8,9,10':
+setlist = setlist + ',11'
+installutils.set_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.list',
+setlist,
+quotes=False, separator='=')
+installutils.set_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.11.constraint.class_id',
+'noConstraintImpl',
+quotes=False, separator='=')
+installutils.set_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.11.constraint.name',
+'No Constraint',
+quotes=False, separator='=')
+installutils.set_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.11.default.class_id',
+'userExtensionDefaultImpl',
+quotes=False, separator='=')
+installutils.set_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.11.default.name',
+'User Supplied Extension Default',
+quotes=False, separator='=')
+installutils.set_directive(
+self.dogtag_constants.IPA_SERVICE_PROFILE,
+'policyset.serverCertSet.11.default.params.userExtOID',
+'2.5.29.17',
+quotes=False, separator='=')
+return True
+
+# No update was done
+return False
+
def set_audit_renewal(self):
