First pass at enforcing certificates be requested from same host

We want to only allow a machine to request a certificate for itself, not for
other machines. I've added a new taksgroup which will allow this.

The requesting IP is resolved and compared to the subject of the CSR to
determine if they are the same host. The same is done with the service
principal. Subject alt names are not queried yet.

This does not yet grant machines actual permission to request certificates
yet, that is still limited to the taskgroup request_certs.

6 files changed, 131 insertions, 36 deletions

diff --git a/install/updates/40-delegation.update b/install/updates/40-delegation.update
index b07dfc756..1be178933 100644
--- a/install/updates/40-delegation.update
+++ b/install/updates/40-delegation.update
@@ -292,6 +292,13 @@ add:cn: removeservices
 add:description: Remove Services
 add:member:'cn=serviceadmin,cn=rolegroups,cn=accounts,$SUFFIX'
 
+dn: cn=modifyservices,cn=taskgroups,cn=accounts,$SUFFIX
+add:objectClass: top
+add:objectClass: nestedgroup
+add:cn: modifyservices
+add:description: Modify Services
+add:member:'cn=serviceadmin,cn=rolegroups,cn=accounts,$SUFFIX'
+
 # Add the ACIs that grant these permissions for service administration
 
 dn: $SUFFIX
@@ -301,6 +308,10 @@ add:aci: '(target = "ldap:///krbprincipalname=*,cn=services,cn=accounts,
 add:aci: '(target = "ldap:///krbprincipalname=*,cn=services,cn=accounts,
  $SUFFIX")(version 3.0;acl "Remove Services";allow (delete) groupdn = "ldap
  :///cn=removeservices,cn=taskgroups,cn=accounts,$SUFFIX";)'
+add:aci: '(targetattr = "userCertificate")(target = "ldap:///krbprincipal
+ name=*,cn=services,cn=accounts,$SUFFIX")(version 3.0;acl "Modify Services"
+ ;allow (write) groupdn = "ldap:///cn=modifyservices,cn=taskgroups,cn=acco
+ unts,$SUFFIX";)'
 
 # Add the taskgroups referenced by the ACIs for delegation administration
 # This just lets one manage taskgroup membership and create and delete roles
@@ -522,7 +533,7 @@ add:cn: request certificate
 dn: cn=request_certs,cn=taskgroups,cn=accounts,$SUFFIX
 add:objectClass: top
 add:objectClass: nestedgroup
-add:cn: reqeust_certs
+add:cn: request_certs
 add:description: Request a SSL Certificate
 add:member:'cn=certadmin,cn=rolegroups,cn=accounts,$SUFFIX'
 
@@ -533,6 +544,27 @@ add: aci: '(targetattr = "objectClass")(target =
   CA" ; allow (write) groupdn = "ldap:///cn=request_certs,cn=taskgroups,
  cn=accounts,$SUFFIX";)'
 
+# Request Certificate from different host virtual op
+dn: cn=request certificate different host,cn=virtual operations,$SUFFIX
+add:objectClass: top
+add:objectClass: nsContainer
+add:cn: request certificate different host
+
+# Taskgroup for requesting certs from a different host
+dn: cn=request_cert_different_host,cn=taskgroups,cn=accounts,$SUFFIX
+add:objectClass: top
+add:objectClass: nestedgroup
+add:cn: request_cert_different_host
+add:description: Request a SSL Certificate from a different host
+add:member:'cn=certadmin,cn=rolegroups,cn=accounts,$SUFFIX'
+
+dn: $SUFFIX
+add: aci: '(targetattr = "objectClass")(target =
+ "ldap:///cn=request certificate different host,cn=virtual operations,
+ $SUFFIX" )(version 3.0 ; acl "Request Certificates from a
+  different host" ; allow (write) groupdn = "ldap:///cn=request_cert
+ _different_host,cn=taskgroups,cn=accounts,$SUFFIX";)'
+
 # Certificate Status virtual op
 dn: cn=certificate status,cn=virtual operations,$SUFFIX
 add:objectClass: top
@@ -543,7 +575,7 @@ add:cn: certificate status
 dn: cn=certificate_status,cn=taskgroups,cn=accounts,$SUFFIX
 add:objectClass: top
 add:objectClass: nestedgroup
-add:cn: reqeust_certs
+add:cn: certificate_status
 add:description: Status of cert request
 add:member:'cn=certadmin,cn=rolegroups,cn=accounts,$SUFFIX'
 
@@ -564,7 +596,7 @@ add:cn: revoke certificate
 dn: cn=revoke_certificate,cn=taskgroups,cn=accounts,$SUFFIX
 add:objectClass: top
 add:objectClass: nestedgroup
-add:cn: reqeust_certs
+add:cn: revoke_certificate
 add:description: Revoke Certificate
 add:member:'cn=certadmin,cn=rolegroups,cn=accounts,$SUFFIX'
 
@@ -585,7 +617,7 @@ add:cn: revoke certificate
 dn: cn=revoke_certificate,cn=taskgroups,cn=accounts,$SUFFIX
 add:objectClass: top
 add:objectClass: nestedgroup
-add:cn: reqeust_certs
+add:cn: revoke_certificate
 add:description: Revoke Certificate
 add:member:'cn=certadmin,cn=rolegroups,cn=accounts,$SUFFIX'
 
@@ -606,7 +638,7 @@ add:cn: certificate remove hold
 dn: cn=certificate_remove_hold,cn=taskgroups,cn=accounts,$SUFFIX
 add:objectClass: top
 add:objectClass: nestedgroup
-add:cn: reqeust_certs
+add:cn: certificate_remove_hold
 add:description: Certificate Remove Hold
 add:member:'cn=certadmin,cn=rolegroups,cn=accounts,$SUFFIX'
 
diff --git a/ipalib/backend.py b/ipalib/backend.py
index b123ed140..7c964b799 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -97,10 +97,15 @@ class Executioner(Backend):
 
 
     def create_context(self, ccache=None, client_ip=None):
+        """
+        client_ip: The IP address of the remote client.
+        """
         if self.env.in_server:
             self.Backend.ldap2.connect(ccache=ccache)
         else:
             self.Backend.xmlclient.connect()
+        if client_ip is not None:
+            setattr(context, "client_ip", client_ip)
 
     def destroy_context(self):
         destroy_context()
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index 1681a22f6..f446b36b5 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -29,8 +29,11 @@ if api.env.enable_ra is not True:
 from ipalib import Command, Str, Int, Bytes, Flag
 from ipalib import errors
 from ipalib.plugins.virtual import *
+from ipalib.plugins.service import split_principal
 import base64
 from OpenSSL import crypto
+from ipalib.request import context
+from ipapython import dnsclient
 
 def get_serial(certificate):
     """
@@ -49,6 +52,22 @@ def get_serial(certificate):
 
     return serial
 
+def get_csr_hostname(csr):
+    """
+    Return the value of CN in the subject of the request
+    """
+    try:
+        der = base64.b64decode(csr)
+        request = crypto.load_certificate_request(crypto.FILETYPE_ASN1, der)
+        sub = request.get_subject().get_components()
+        for s in sub:
+            if s[0].lower() == "cn":
+                return s[1]
+    except crypto.Error, e:
+        raise errors.GenericError(format='Unable to decode CSR: %s' % str(e))
+
+    return None
+
 def validate_csr(ugettext, csr):
     """
     For now just verify that it is properly base64-encoded.
@@ -61,7 +80,7 @@ def validate_csr(ugettext, csr):
 
 class cert_request(VirtualCommand):
     """
-    Submit a certificate singing request.
+    Submit a certificate signing request.
     """
 
     takes_args = (Str('csr', validate_csr),)
@@ -83,7 +102,6 @@ class cert_request(VirtualCommand):
     )
 
     def execute(self, csr, **kw):
-        super(cert_request, self).execute()
         skw = {"all": True}
         principal = kw.get('principal')
         add = kw.get('add')
@@ -91,6 +109,47 @@ class cert_request(VirtualCommand):
         del kw['add']
         service = None
 
+        # Can this user request certs?
+        self.check_access()
+
+        # FIXME: add support for subject alt name
+        # Is this cert for this principal?
+        subject_host = get_csr_hostname(csr)
+
+        # Ensure that the hostname in the CSR matches the principal
+        (servicename, hostname, realm) = split_principal(principal)
+        if subject_host.lower() != hostname.lower():
+            raise errors.ACIError(info="hostname in subject of request '%s' does not match principal hostname '%s'" % (subject_host, hostname))
+
+        # Get the IP address of the machine that submitted the request. We
+        # will compare this to the subjectname of the CSR.
+        client_ip = getattr(context, 'client_ip')
+        rhost = None
+        if client_ip not in (None, ''):
+            rev = client_ip.split('.')
+            if len(rev) == 0:
+                rev = client_ip.split(':')
+                rev.reverse()
+                addr = "%s.in-addr.arpa." % ".".join(rev)
+            else:
+                rev.reverse()
+                addr = "%s.in-addr.arpa." % ".".join(rev)
+            rs = dnsclient.query(addr, dnsclient.DNS_C_IN, dnsclient.DNS_T_PTR)
+            if len(rs) == 0:
+                raise errors.ACIError(info='DNS lookup on client failed for IP %s' % client_ip)
+            for rsn in rs:
+                if rsn.dns_type == dnsclient.DNS_T_PTR:
+                    rhost = rsn
+                    break
+
+        if rhost is None:
+            raise errors.ACIError(info='DNS lookup on client failed for IP %s' % client_ip)
+
+        client_hostname = rhost.rdata.ptrdname
+        if subject_host.lower() != client_hostname.lower():
+             self.log.debug("IPA: hostname in subject of request '%s' does not match requesting hostname '%s'" % (subject_host, client_hostname))
+             self.check_access(operation="request certificate different host")
+
         # See if the service exists and punt if it doesn't and we aren't
         # going to add it
         try:
@@ -98,6 +157,8 @@ class cert_request(VirtualCommand):
             if 'usercertificate' in service:
                 # FIXME, what to do here? Do we revoke the old cert?
                 raise errors.GenericError(format='entry already has a certificate, serial number %s' % get_serial(service['usercertificate']))
+            if not can_write(dn, "usercertificate"):
+                raise errors.ACIError(info='You need to be a member of the serviceadmin role to update services')
 
         except errors.NotFound, e:
             if not add:
@@ -110,7 +171,10 @@ class cert_request(VirtualCommand):
         # either exists or we should add it.
         if result.get('status') == '0':
             if service is None:
-                service = api.Command['service_add'](principal, **{})
+                try:
+                    service = api.Command['service_add'](principal, **{})
+                except errors.ACIError:
+                    raise errors.ACIError(info='You need to be a member of the serviceadmin role to add services')
             skw = {"usercertificate": str(result.get('certificate'))}
             api.Command['service_mod'](principal, **skw)
 
@@ -162,7 +226,7 @@ class cert_status(VirtualCommand):
 
 
     def execute(self, request_id, **kw):
-        super(cert_status, self).execute()
+        self.check_access()
         return self.Backend.ra.check_request_status(request_id)
 
     def output_for_cli(self, textui, result, *args, **kw):
@@ -183,7 +247,7 @@ class cert_get(VirtualCommand):
     operation="retrieve certificate"
 
     def execute(self, serial_number):
-        super(cert_get, self).execute()
+        self.check_access()
         return self.Backend.ra.get_certificate(serial_number)
 
     def output_for_cli(self, textui, result, *args, **kw):
@@ -215,7 +279,7 @@ class cert_revoke(VirtualCommand):
 
 
     def execute(self, serial_number, **kw):
-        super(cert_revoke, self).execute()
+        self.check_access()
         return self.Backend.ra.revoke_certificate(serial_number, **kw)
 
     def output_for_cli(self, textui, result, *args, **kw):
@@ -236,7 +300,7 @@ class cert_remove_hold(VirtualCommand):
     operation = "certificate remove hold"
 
     def execute(self, serial_number, **kw):
-        super(cert_remove_hold, self).execute()
+        self.check_access()
         return self.Backend.ra.take_certificate_off_hold(serial_number)
 
     def output_for_cli(self, textui, result, *args, **kw):
diff --git a/ipalib/plugins/virtual.py b/ipalib/plugins/virtual.py
index d21a58f12..3ac96301e 100644
--- a/ipalib/plugins/virtual.py
+++ b/ipalib/plugins/virtual.py
@@ -40,34 +40,27 @@ class VirtualCommand(Command):
     """
     operation = None
 
-    def execute(self, *args, **kw):
+    def check_access(self, operation=None):
         """
-        Perform the LDAP query to determine authorization.
+        Perform an LDAP query to determine authorization.
 
-        This should be executed via super() before any actual work is done.
+        This should be executed before any actual work is done.
         """
-        if self.operation is None:
+        if self.operation is None and operation is None:
             raise errors.ACIError(info='operation not defined')
 
+        if operation is None:
+            operation = self.operation
+
         ldap = self.api.Backend.ldap2
-        self.log.info("IPA: virtual verify %s" % self.operation)
+        self.log.info("IPA: virtual verify %s" % operation)
 
-        operationdn = "cn=%s,%s,%s" % (self.operation, self.api.env.container_virtual, self.api.env.basedn)
+        operationdn = "cn=%s,%s,%s" % (operation, self.api.env.container_virtual, self.api.env.basedn)
 
-        # By adding this unknown objectclass we do several things.
-        # DS checks ACIs before the objectclass so we can test for ACI
-        # errors to know if we have rights. If we do have rights then the
-        # update will fail anyway with a Database error because of an
-        # unknown objectclass, so we can catch that gracefully as well.
         try:
-            updatekw = {'objectclass': ['somerandomunknownclass']}
-            ldap.update(operationdn, **updatekw)
-        except errors.ACIError, e:
-            self.log.debug("%s" % str(e))
-            raise errors.ACIError(info='not allowed to perform this command')
-        except errors.ObjectclassViolation:
-            return
-        except Exception, e:
-            # Something unexpected happened. Log it and deny access to be safe.
-            self.log.info("Virtual verify failed: %s %s" % (type(e), str(e)))
-            raise errors.ACIError(info='not allowed to perform this command')
+            if not ldap.can_write(operationdn, "objectclass"):
+                raise errors.ACIError(info='not allowed to perform this command')
+        except errors.NotFound:
+            raise errors.ACIError(info='No such virtual command')
+
+        return True
diff --git a/ipaserver/__init__.py b/ipaserver/__init__.py
index ec2a53649..7fdba6246 100644
--- a/ipaserver/__init__.py
+++ b/ipaserver/__init__.py
@@ -54,6 +54,7 @@ def xmlrpc(req):
         response = api.Backend.xmlserver.marshaled_dispatch(
             req.read(),
             req.subprocess_env.get('KRB5CCNAME'),
+            req.connection.remote_ip
         )
 
     req.content_type = 'text/xml'
diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py
index c98ca7ab7..ab8b41720 100644
--- a/ipaserver/rpcserver.py
+++ b/ipaserver/rpcserver.py
@@ -181,12 +181,12 @@ class xmlserver(WSGIExecutioner):
     def methodHelp(self, *params):
         return u'methodHelp not implemented'
 
-    def marshaled_dispatch(self, data, ccache):
+    def marshaled_dispatch(self, data, ccache, client_ip):
         """
         Execute the XML-RPC request contained in ``data``.
         """
         try:
-            self.create_context(ccache=ccache)
+            self.create_context(ccache=ccache, client_ip=client_ip)
             (params, name) = xml_loads(data)
             if name in self.__system:
                 response = (self.__system[name](*params),)