On 16.6.2014 16:08, Martin Kosek wrote:
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:962000003:962000003
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')
     File "/root/freeipa-master/ipalib/plugable.py", line 482, in
__do_if_not_done
       getattr(self, name)()
     File "/root/freeipa-master/ipalib/plugable.py", line 645, in load_plugins
       self.import_plugins('ipalib')
     File "/root/freeipa-master/ipalib/plugable.py", line 689, in
import_plugins
       __import__(fullname)
     File "/root/freeipa-master/ipalib/plugins/cert.py", line 30, in <module>
       from ipalib import pkcs10
     File "/root/freeipa-master/ipalib/pkcs10.py", line 77
       .replace('@', '\\@')
...

The rest of the notes are thus only from reading.

Sorry, last minute change gone wrong.


2) There are lines like

+        if name_type == 'Other Name (OID.1.3.6.1.5.2.2)':


or

+            if name_type not in ('DNS name',
+                                 'Other Name (OID.1.3.6.1.4.1.311.20.2.3)',
+                                 'Other Name (OID.1.3.6.1.5.2.2)'):

or

+            elif name_type in ('Other Name (OID.1.3.6.1.4.1.311.20.2.3)',
+                               'Other Name (OID.1.3.6.1.5.2.2)'):

Can we do something better? Getting the extension type based on it's
description seems extremely unstable to me.

These are SAN types, not extension types. Unfortunately the textual
descriptions are the only SAN type representation available in python-nss which
includes OIDs of other names.


Can we for example modify get_subjectaltname to get the type based on
CertificateExtension object's oid or oid_tag attributes?

No, see above.


I would rather see get_subjectaltname return solid type like
CERT_EXTENSION_DNS_NAME defined in pkcs10.py than consumers comparing
descriptions. It would be more readable, too.

Done.


Martin


Updated patches attached.

1) We still miss the managed permission allowing Certificate Administrators to
read Virtual Commands

I was under the impression that Petr would fix this.

Previously I was asking for consultancy, new managed permissions should make it
much easier to add new ACIs, so I think you should be able to do it. If not,
you can ask Petr for help.

Added, see patch 300.


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



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 10.34.47.236:12]
Traceback (most recent call last):
[Mon Jun 16 13:06:14.528844 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/share/ipa/wsgi.py", line 49, in application
[Mon Jun 16 13:06:14.529466 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      return api.Backend.wsgi_dispatch(environ, start_response)
[Mon Jun 16 13:06:14.529614 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 251, in
__call__
[Mon Jun 16 13:06:14.553116 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      return self.route(environ, start_response)
[Mon Jun 16 13:06:14.553261 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 263, in
route
[Mon Jun 16 13:06:14.553410 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      return app(environ, start_response)
[Mon Jun 16 13:06:14.553604 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 637, in
__call__
[Mon Jun 16 13:06:14.553749 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      environ, start_response)
[Mon Jun 16 13:06:14.553895 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 397, in
__call__
[Mon Jun 16 13:06:14.554023 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      response = self.wsgi_execute(environ)
[Mon Jun 16 13:06:14.554138 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 343, in
wsgi_execute
[Mon Jun 16 13:06:14.554276 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      result = self.Command[name](*args, **options)
[Mon Jun 16 13:06:14.554413 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 439, in
__call__
[Mon Jun 16 13:06:14.585933 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      ret = self.run(*args, **options)
[Mon Jun 16 13:06:14.586170 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 754, in run
[Mon Jun 16 13:06:14.586305 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      return self.execute(*args, **options)
[Mon Jun 16 13:06:14.586490 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipalib/plugins/cert.py", line 316, in
execute
[Mon Jun 16 13:06:14.592390 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      subjectaltname = pkcs10.get_subjectaltname(csr) or ()
[Mon Jun 16 13:06:14.592565 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/ipalib/pkcs10.py", line 129, in
get_subjectaltname
[Mon Jun 16 13:06:14.596411 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      san = decoder.decode(san, asn1Spec=_SubjectAltName())[0]
[Mon Jun 16 13:06:14.596555 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/pyasn1/codec/ber/decoder.py", line
792, in __call__
[Mon Jun 16 13:06:14.598268 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      stGetValueDecoder, self, substrateFun
[Mon Jun 16 13:06:14.598404 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/pyasn1/codec/ber/decoder.py", line
367, in valueDecoder
[Mon Jun 16 13:06:14.598534 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      component, head = decodeFun(head, asn1Spec)
[Mon Jun 16 13:06:14.598633 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
    File "/usr/lib/python2.7/site-packages/pyasn1/codec/ber/decoder.py", line
798, in __call__
[Mon Jun 16 13:06:14.598746 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
      '%r not in asn1Spec: %r' % (tagSet, asn1Spec)
[Mon Jun 16 13:06:14.598954 2014] [:error] [pid 12941] [remote 10.34.47.236:12]
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.


I would like FreeIPA 4.0 to run on vanilla Fedora 20, so far it does, the
required python-nss 0.15 is already on it's way to stable updates.

Thanks,
Martin


Also added patch 301 which fixes a bug in ldap2.get_effective_rights I was hitting with patch 234.

Updated patches attached.

--
Jan Cholasta
>From df58ac54d8baa4a6362e402cea3764ff9ac3748d 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/4] 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):
             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)
@@ -1198,6 +1199,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,
@@ -1513,6 +1516,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):
         """
         The default renewal time for the audit signing certificate is
-- 
1.9.0

>From 48ffb206ea4f98ede93bd2208ea718e80422160c Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 18 Jun 2014 09:02:03 +0200
Subject: [PATCH 2/4] Support requests with SAN in cert-request.

For each SAN in a request there must be a matching service entry writable by
the requestor. Users can request certificates with SAN only if they have
"Request Certificate With SubjectAltName" permission.

https://fedorahosted.org/freeipa/ticket/3977
---
 freeipa.spec.in                      |   2 +-
 install/updates/40-delegation.update |  16 +++++
 ipalib/pkcs10.py                     | 116 ++++++++++++++++++++++++++++++++---
 ipalib/plugins/cert.py               | 100 ++++++++++++++++++++----------
 4 files changed, 191 insertions(+), 43 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b719b4a..0d2221d 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -299,7 +299,7 @@ Requires: gnupg
 Requires: iproute
 Requires: keyutils
 Requires: pyOpenSSL
-Requires: python-nss
+Requires: python-nss >= 0.15
 Requires: python-lxml
 Requires: python-netaddr
 Requires: libipa_hbac-python
diff --git a/install/updates/40-delegation.update b/install/updates/40-delegation.update
index 7c3a284..8905497 100644
--- a/install/updates/40-delegation.update
+++ b/install/updates/40-delegation.update
@@ -391,6 +391,22 @@ default:objectClass: top
 default:objectClass: nsContainer
 default:cn: certificate remove hold
 
+dn: cn=request certificate with subjectaltname,cn=virtual operations,cn=etc,$SUFFIX
+default:objectClass: top
+default:objectClass: nsContainer
+default:objectClass: ipaVirtualOperation
+default:cn: request certificate with subjectaltname
+
+dn: cn=Request Certificate with SubjectAltName,cn=permissions,cn=pbac,$SUFFIX
+default:objectClass: top
+default:objectClass: groupofnames
+default:objectClass: ipapermission
+default:cn: Request Certificate with SubjectAltName
+default:member: cn=Certificate Administrators,cn=privileges,cn=pbac,$SUFFIX
+
+dn: $SUFFIX
+add:aci:'(targetattr = "objectclass")(target = "ldap:///cn=request certificate with subjectaltname,cn=virtual operations,cn=etc,$SUFFIX" )(version 3.0; acl "permission:Request Certificate with SubjectAltName"; allow (write) groupdn = "ldap:///cn=Request Certificate with SubjectAltName,cn=permissions,cn=pbac,$SUFFIX";)'
+
 
 # Read privileges
 dn: cn=RBAC Readers,cn=privileges,cn=pbac,$SUFFIX
diff --git a/ipalib/pkcs10.py b/ipalib/pkcs10.py
index 5958d5a..f35e200 100644
--- a/ipalib/pkcs10.py
+++ b/ipalib/pkcs10.py
@@ -21,7 +21,7 @@ import os
 import sys
 import base64
 import nss.nss as nss
-from pyasn1.type import univ, namedtype, tag
+from pyasn1.type import univ, char, namedtype, tag
 from pyasn1.codec.der import decoder
 from ipapython import ipautil
 from ipalib import api
@@ -29,6 +29,10 @@ from ipalib import api
 PEM = 0
 DER = 1
 
+SAN_DNSNAME = 'DNS name'
+SAN_OTHERNAME_UPN = 'Other Name (OID.1.3.6.1.4.1.311.20.2.3)'
+SAN_OTHERNAME_KRB5PRINCIPALNAME = 'Other Name (OID.1.3.6.1.5.2.2)'
+
 def get_subject(csr, datatype=PEM):
     """
     Given a CSR return the subject value.
@@ -41,6 +45,89 @@ def get_subject(csr, datatype=PEM):
     finally:
         del request
 
+def get_extensions(csr, datatype=PEM):
+    """
+    Given a CSR return OIDs of certificate extensions.
+
+    The return value is a tuple of strings
+    """
+    request = load_certificate_request(csr, datatype)
+    return tuple(nss.oid_dotted_decimal(ext.oid_tag)[4:]
+                 for ext in request.extensions)
+
+class _PrincipalName(univ.Sequence):
+    componentType = namedtype.NamedTypes(
+        namedtype.NamedType('name-type', univ.Integer().subtype(
+            explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
+        ),
+        namedtype.NamedType('name-string', univ.SequenceOf(char.GeneralString()).subtype(
+            explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
+        ),
+    )
+
+class _KRB5PrincipalName(univ.Sequence):
+    componentType = namedtype.NamedTypes(
+        namedtype.NamedType('realm', char.GeneralString().subtype(
+            explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
+        ),
+        namedtype.NamedType('principalName', _PrincipalName().subtype(
+            explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
+        ),
+    )
+
+def _decode_krb5principalname(data):
+    principal = decoder.decode(data, asn1Spec=_KRB5PrincipalName())[0]
+    realm = (str(principal['realm']).replace('\\', '\\\\')
+                                    .replace('@', '\\@'))
+    name = principal['principalName']['name-string']
+    name = '/'.join(str(n).replace('\\', '\\\\')
+                          .replace('/', '\\/')
+                          .replace('@', '\\@') for n in name)
+    name = '%s@%s' % (name, realm)
+    return name
+
+class _AnotherName(univ.Sequence):
+    componentType = namedtype.NamedTypes(
+        namedtype.NamedType('type-id', univ.ObjectIdentifier()),
+        namedtype.NamedType('value', univ.Any().subtype(
+            explicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
+        ),
+    )
+
+class _GeneralName(univ.Choice):
+    componentType = namedtype.NamedTypes(
+        namedtype.NamedType('otherName', _AnotherName().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0))
+        ),
+        namedtype.NamedType('rfc822Name', char.IA5String().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 1))
+        ),
+        namedtype.NamedType('dNSName', char.IA5String().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 2))
+        ),
+        namedtype.NamedType('x400Address', univ.Sequence().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 3))
+        ),
+        namedtype.NamedType('directoryName', univ.Choice().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 4))
+        ),
+        namedtype.NamedType('ediPartyName', univ.Sequence().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 5))
+        ),
+        namedtype.NamedType('uniformResourceIdentifier', char.IA5String().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 6))
+        ),
+        namedtype.NamedType('iPAddress', univ.OctetString().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 7))
+        ),
+        namedtype.NamedType('registeredID', univ.ObjectIdentifier().subtype(
+            implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 8))
+        ),
+    )
+
+class _SubjectAltName(univ.SequenceOf):
+    componentType = _GeneralName()
+
 def get_subjectaltname(csr, datatype=PEM):
     """
     Given a CSR return the subjectaltname value, if any.
@@ -48,13 +135,26 @@ def get_subjectaltname(csr, datatype=PEM):
     The return value is a tuple of strings or None
     """
     request = load_certificate_request(csr, datatype)
-    try:
-        for extension in request.extensions:
-            if extension.oid_tag  == nss.SEC_OID_X509_SUBJECT_ALT_NAME:
-                return nss.x509_alt_name(extension.value)
-    finally:
-        del request
-    return None
+    for extension in request.extensions:
+        if extension.oid_tag == nss.SEC_OID_X509_SUBJECT_ALT_NAME:
+            break
+    else:
+        return None
+    del request
+
+    nss_names = nss.x509_alt_name(extension.value, nss.AsObject)
+    asn1_names = decoder.decode(extension.value.data,
+                                asn1Spec=_SubjectAltName())[0]
+    names = []
+    for nss_name, asn1_name in zip(nss_names, asn1_names):
+        name_type = nss_name.type_string
+        if name_type == SAN_OTHERNAME_KRB5PRINCIPALNAME:
+            name = _decode_krb5principalname(asn1_name['otherName']['value'])
+        else:
+            name = nss_name.name
+        names.append((name_type, name))
+
+    return tuple(names)
 
 # Unfortunately, NSS can only parse the extension request attribute, so
 # we have to parse friendly name ourselves (see RFC 2986)
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index b1fa32b..d0f8bc6 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -42,6 +42,7 @@ from ipalib import output
 from ipalib.plugins.service import validate_principal
 import nss.nss as nss
 from nss.error import NSPRError
+from pyasn1.error import PyAsn1Error
 
 __doc__ = _("""
 IPA certificate operations
@@ -136,17 +137,6 @@ def validate_pkidate(ugettext, value):
 
     return None
 
-def get_csr_hostname(csr):
-    """
-    Return the value of CN in the subject of the request or None
-    """
-    try:
-        subject = pkcs10.get_subject(csr)
-        return subject.common_name  #pylint: disable=E1101
-    except NSPRError, nsprerr:
-        raise errors.CertificateOperationError(
-            error=_('Failure decoding Certificate Signing Request: %s') % nsprerr)
-
 def validate_csr(ugettext, csr):
     """
     Ensure the CSR is base64-encoded and can be decoded by our PKCS#10
@@ -290,6 +280,14 @@ class cert_request(VirtualCommand):
         ),
     )
 
+    _allowed_extensions = {
+        '2.5.29.14': None,      # Subject Key Identifier
+        '2.5.29.15': None,      # Key Usage
+        '2.5.29.17': 'request certificate with subjectaltname',
+        '2.5.29.19': None,      # Basic Constraints
+        '2.5.29.37': None,      # Extended Key Usage
+    }
+
     def execute(self, csr, **kw):
         ldap = self.api.Backend.ldap2
         principal = kw.get('principal')
@@ -313,10 +311,22 @@ class cert_request(VirtualCommand):
         if not bind_principal.startswith('host/'):
             self.check_access()
 
-        # FIXME: add support for subject alt name
+        try:
+            subject = pkcs10.get_subject(csr)
+            extensions = pkcs10.get_extensions(csr)
+            subjectaltname = pkcs10.get_subjectaltname(csr) or ()
+        except (NSPRError, PyAsn1Error), e:
+            raise errors.CertificateOperationError(
+                error=_("Failure decoding Certificate Signing Request: %s") % e)
+
+        if not bind_principal.startswith('host/'):
+            for ext in extensions:
+                operation = self._allowed_extensions.get(ext)
+                if operation:
+                    self.check_access(operation)
 
         # Ensure that the hostname in the CSR matches the principal
-        subject_host = get_csr_hostname(csr)
+        subject_host = subject.common_name  #pylint: disable=E1101
         if not subject_host:
             raise errors.ValidationError(name='csr',
                 error=_("No hostname was found in subject of request."))
@@ -328,28 +338,40 @@ class cert_request(VirtualCommand):
                     "does not match principal hostname '%(hostname)s'") % dict(
                         subject_host=subject_host, hostname=hostname))
 
+        for ext in extensions:
+            if ext not in self._allowed_extensions:
+                raise errors.ValidationError(
+                    name='csr', error=_("extension %s is forbidden") % ext)
+
+        for name_type, name in subjectaltname:
+            if name_type not in (pkcs10.SAN_DNSNAME,
+                                 pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME,
+                                 pkcs10.SAN_OTHERNAME_UPN):
+                raise errors.ValidationError(
+                    name='csr',
+                    error=_("subject alt name type %s is forbidden") %
+                          name_type)
+
         dn = None
         service = None
         # See if the service exists and punt if it doesn't and we aren't
         # going to add it
         try:
-            if not principal.startswith('host/'):
-                service = api.Command['service_show'](principal, all=True)['result']
-                dn = service['dn']
+            if servicename != 'host':
+                service = api.Command['service_show'](principal, all=True)
             else:
-                hostname = get_host_from_principal(principal)
-                service = api.Command['host_show'](hostname, all=True)['result']
-                dn = service['dn']
+                service = api.Command['host_show'](hostname, all=True)
         except errors.NotFound, e:
             if not add:
                 raise errors.NotFound(reason=_("The service principal for "
                     "this request doesn't exist."))
             try:
-                service = api.Command['service_add'](principal, **{'force': True})['result']
-                dn = service['dn']
+                service = api.Command['service_add'](principal, force=True)
             except errors.ACIError:
                 raise errors.ACIError(info=_('You need to be a member of '
                     'the serviceadmin role to add services'))
+        service = service['result']
+        dn = service['dn']
 
         # We got this far so the service entry exists, can we write it?
         if not ldap.can_write(dn, "usercertificate"):
@@ -357,25 +379,35 @@ class cert_request(VirtualCommand):
                 "to the 'userCertificate' attribute of entry '%s'.") % dn)
 
         # Validate the subject alt name, if any
-        subjectaltname = pkcs10.get_subjectaltname(csr)
-        if subjectaltname is not None:
-            for name in subjectaltname:
+        for name_type, name in subjectaltname:
+            if name_type == pkcs10.SAN_DNSNAME:
                 name = unicode(name)
                 try:
-                    hostentry = api.Command['host_show'](name, all=True)['result']
-                    hostdn = hostentry['dn']
+                    if servicename == 'host':
+                        altservice = api.Command['host_show'](name, all=True)
+                    else:
+                        altprincipal = '%s/%s@%s' % (servicename, name, realm)
+                        altservice = api.Command['service_show'](
+                            altprincipal, all=True)
                 except errors.NotFound:
                     # We don't want to issue any certificates referencing
                     # machines we don't know about. Nothing is stored in this
                     # host record related to this certificate.
-                    raise errors.NotFound(reason=_('no host record for '
-                        'subject alt name %s in certificate request') % name)
-                authprincipal = getattr(context, 'principal')
-                if authprincipal.startswith("host/"):
-                    if not hostdn in service.get('managedby_host', []):
-                        raise errors.ACIError(info=_(
-                            "Insufficient privilege to create a certificate "
-                            "with subject alt name '%s'.") % name)
+                    raise errors.NotFound(reason=_('The service principal for '
+                        'subject alt name %s in certificate request does not '
+                        'exist') % name)
+                altdn = altservice['result']['dn']
+                if not ldap.can_write(altdn, "usercertificate"):
+                    raise errors.ACIError(info=_(
+                        "Insufficient privilege to create a certificate with "
+                        "subject alt name '%s'.") % name)
+            elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME,
+                               pkcs10.SAN_OTHERNAME_UPN):
+                if name != principal:
+                    raise errors.ValidationError(
+                        name='csr',
+                        error=_("principal %s in subject alt name does not "
+                                "match requested service principal") % name)
 
         if 'usercertificate' in service:
             serial = x509.get_serial_number(service['usercertificate'][0], datatype=x509.DER)
-- 
1.9.0

From 66560360678234c65ebafc96e8528bff4a2484d0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 18 Jun 2014 09:02:22 +0200
Subject: [PATCH 3/4] Add virtual operations read permission.

---
 ACI.txt                                                 |  2 ++
 ipaserver/install/plugins/update_managed_permissions.py | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/ACI.txt b/ACI.txt
index 2ceaacc..a2978e5 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -112,3 +112,5 @@ dn: cn=System: Read Replication Agreements,cn=permissions,cn=pbac,dc=ipa,dc=exam
 aci: (targetattr = "cn || description || nsds50ruv || nsds5beginreplicarefresh || nsds5debugreplicatimeout || nsds5flags || nsds5replicaabortcleanruv || nsds5replicaautoreferral || nsds5replicabackoffmax || nsds5replicabackoffmin || nsds5replicabinddn || nsds5replicabindmethod || nsds5replicabusywaittime || nsds5replicachangecount || nsds5replicachangessentsincestartup || nsds5replicacleanruv || nsds5replicacleanruvnotified || nsds5replicacredentials || nsds5replicaenabled || nsds5replicahost || nsds5replicaid || nsds5replicalastinitend || nsds5replicalastinitstart || nsds5replicalastinitstatus || nsds5replicalastupdateend || nsds5replicalastupdatestart || nsds5replicalastupdatestatus || nsds5replicalegacyconsumer || nsds5replicaname || nsds5replicaport || nsds5replicaprotocoltimeout || nsds5replicapurgedelay || nsds5replicareferral || nsds5replicaroot || nsds5replicasessionpausetime || nsds5replicastripattrs || nsds5replicatedattributelist || nsds5replicatedattributelisttotal || nsds5replicatimeout || nsds5replicatombstonepurgeinterval || nsds5replicatransportinfo || nsds5replicatype || nsds5replicaupdateinprogress || nsds5replicaupdateschedule || nsds5task || nsds7directoryreplicasubtree || nsds7dirsynccookie || nsds7newwingroupsyncenabled || nsds7newwinusersyncenabled || nsds7windowsdomain || nsds7windowsreplicasubtree || nsruvreplicalastmodified || nsstate || objectclass || onewaysync || winsyncdirectoryfilter || winsyncinterval || winsyncmoveaction || winsyncsubtreepair || winsyncwindowsfilter")(targetfilter = "(|(objectclass=nsds5Replica)(objectclass=nsds5replicationagreement)(objectclass=nsDSWindowsReplicationAgreement)(objectClass=nsMappingTree))")(version 3.0;acl "permission:System: Read Replication Agreements";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Replication Agreements,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: cn=System: Read Replication Information,cn=permissions,cn=pbac,dc=ipa,dc=example
 aci: (targetattr = "cn || nsds5flags || nsds5replicaabortcleanruv || nsds5replicaautoreferral || nsds5replicabackoffmax || nsds5replicabackoffmin || nsds5replicabinddn || nsds5replicachangecount || nsds5replicacleanruv || nsds5replicaid || nsds5replicalegacyconsumer || nsds5replicaname || nsds5replicaprotocoltimeout || nsds5replicapurgedelay || nsds5replicareferral || nsds5replicaroot || nsds5replicatombstonepurgeinterval || nsds5replicatype || nsds5task || nsstate || objectclass")(targetfilter = "(objectclass=nsds5replica)")(version 3.0;acl "permission:System: Read Replication Information";allow (compare,read,search) userdn = "ldap:///all";;)
+dn: cn=System: Read Virtual Operations,cn=permissions,cn=pbac,dc=ipa,dc=example
+aci: (targetattr = "cn || objectclass")(targetfilter = "(objectclass=ipavirtualoperation)")(version 3.0;acl "permission:System: Read Virtual Operations";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Virtual Operations,cn=permissions,cn=pbac,dc=ipa,dc=example";)
diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py
index 7b1405a..ca773a9 100644
--- a/ipaserver/install/plugins/update_managed_permissions.py
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -228,7 +228,18 @@ NONOBJECT_PERMISSIONS = {
             'winsyncsubtreepair',
         },
         'default_privileges': {'Replication Administrators'},
-    }
+    },
+    'System: Read Virtual Operations': {
+        'replaces_global_anonymous_aci': True,
+        'ipapermlocation': DN(api.env.container_virtual, api.env.basedn),
+        'ipapermtargetfilter': {'(objectclass=ipavirtualoperation)'},
+        'ipapermbindruletype': 'permission',
+        'ipapermright': {'read', 'search', 'compare'},
+        'ipapermdefaultattr': {
+            'cn', 'objectclass',
+        },
+        'default_privileges': {'Certificate Administrators'},
+    },
 }
 
 
-- 
1.9.0

>From 36fbce12f1a339c0f8ddbb658d0c7da6a5aa9bea Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 18 Jun 2014 11:45:45 +0200
Subject: [PATCH 4/4] Remove GetEffectiveRights control when
 ldap2.get_effective_rights fails.

---
 ipaserver/plugins/ldap2.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index aa9a001..cfcec7c 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -333,9 +333,11 @@ class ldap2(LDAPClient, CrudBackend):
             "krbPrincipalAux", base_dn=api.env.basedn)
         sctrl = [GetEffectiveRightsControl(True, "dn: " + str(entry.dn))]
         self.conn.set_option(_ldap.OPT_SERVER_CONTROLS, sctrl)
-        entry = self.get_entry(dn, attrs_list)
-        # remove the control so subsequent operations don't include GER
-        self.conn.set_option(_ldap.OPT_SERVER_CONTROLS, [])
+        try:
+            entry = self.get_entry(dn, attrs_list)
+        finally:
+            # remove the control so subsequent operations don't include GER
+            self.conn.set_option(_ldap.OPT_SERVER_CONTROLS, [])
         return entry
 
     def can_write(self, dn, attr):
-- 
1.9.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to