Thanks Ian! I'll try and review this in the next couple of days?
Do you use GitHub? If so, you could create a pull request there,
which will make it more visible, easier to review, and cause CI to
run on your patch. If not, that's OK. We are happy to receive your
contribution by any means!
Cheers,
Fraser
On Fri, Mar 09, 2018 at 05:10:52PM -0600, Ian Pilcher via FreeIPA-devel wrote:
On 03/01/2018 08:52 AM, Rob Crittenden wrote:
> Ian Pilcher via FreeIPA-devel wrote:
>
> You'd need to ensure that the IP address exists in IPA but that it is
> owned/managed by the user/host/service making the request.
Any hints on how to do that? I don't see any ownership information
associated with DNS records or IP addresses, but I don't really know
where I should be looking
> > FYI, I've been working on the logic for validating the IP addresses in
> > my not-copious-spare time, and I hope to have something worth discussing
> > in the next week or so.
>
> Thanks, I look forward to it.
First draft below. Looking forward to y'alls' comments.
From 9e64ec302ad5370a22437d53876ab31f2a237033 Mon Sep 17 00:00:00 2001
From: Ian Pilcher <arequipeno(a)gmail.com>
Date: Fri, 9 Mar 2018 12:33:55 -0600
Subject: [PATCH] Allow issuing certificates with IP addresses in
subjectAltName
Allow issuing certificates with IP addresses in the subject
alternative name (SAN), if all of the following are true.
* One of the DNS names in the SAN resolves to the IP address
(possibly through a CNAME).
* All of the DNS entries in the resolution chain are managed by
this IPA instance.
* The IP address has a (correct) reverse DNS entry that is managed
by this IPA instance
---
ipaserver/plugins/cert.py | 92
+++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 89 insertions(+), 3 deletions(-)
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index db624357a..452728d21 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -22,11 +22,13 @@
import base64
import collections
import datetime
+import itertools
import logging
from operator import attrgetter
import cryptography.x509
from cryptography.hazmat.primitives import hashes, serialization
+from dns import resolver, reversename
import six
from ipalib import Command, Str, Int, Flag
@@ -48,7 +50,7 @@ from .certprofile import validate_profile_id
from ipalib.text import _
from ipalib.request import context
from ipalib import output
-from ipapython import kerberos
+from ipapython import dnsutil, kerberos
from ipapython.dn import DN
from ipaserver.plugins.service import normalize_principal, validate_realm
@@ -773,9 +775,10 @@ class cert_request(Create, BaseCertMethod,
VirtualCommand):
"'userCertificate' attribute of entry
'%s'.") %
dn)
# Validate the subject alt name, if any
- generalnames = []
+ san_has_ipaddrs = False
+ generalnames = ()
if ext_san is not None:
- generalnames = x509.process_othernames(ext_san.value)
+ generalnames = tuple(x509.process_othernames(ext_san.value))
for gn in generalnames:
if isinstance(gn, cryptography.x509.general_name.DNSName):
if principal.is_user:
@@ -858,11 +861,16 @@ class cert_request(Create, BaseCertMethod,
VirtualCommand):
"subject alt name type %s is forbidden "
"for non-user principals") %
"RFC822Name"
)
+ elif isinstance(gn, cryptography.x509.general_name.IPAddress):
+ san_has_ipaddrs = True
else:
raise errors.ACIError(
info=_("Subject alt name type %s is forbidden")
% type(gn).__name__)
+ if san_has_ipaddrs:
+ _validate_san_ips(generalnames)
+
# Request the certificate
try:
# re-serialise to PEM, in case the user-supplied data has
@@ -1046,6 +1054,84 @@ def _principal_name_matches_principal(name,
principal_obj):
return principal in principal_obj.get('krbprincipalname', [])
+def _validate_san_ips(generalnames):
+ """
+ Raise a ValidationError if the subjectAltName in a CSR includes
+ any IP address(es) that do not match a DNS name in the SAN.
+
+ """
+ san_dns_ips = set()
+ for name in generalnames:
+ if isinstance(name, cryptography.x509.general_name.DNSName):
+ san_dns_ips.update(_san_dnsname_ips(name.value))
+ for name in generalnames:
+ if isinstance(name, cryptography.x509.general_name.IPAddress):
+ if unicode(name.value) not in san_dns_ips:
+ raise errors.ValidationError(
+ name='csr',
+ error=_(
+ "IP address in subjectAltName (%s) does not "
+ "match any DNS name"
+ ) % name.value
+ )
+
+
+def _san_dnsname_ips(dnsname, dnsname_is_cname=False):
+ """
+ Returns a set of IP addresses, managed by this IPa instance,
+ that correspond to the DNS name (from the subjectAltName).
+
+ """
+ fqdn = dnsutil.DNSName(dnsname).make_absolute()
+ if fqdn.__len__() < 4:
+ logger.debug("Skipping IPs for %s: hostname too short" % dnsname)
+ return ()
+ zone = dnsutil.DNSName(resolver.zone_for_name(fqdn))
+ name = fqdn.relativize(zone)
+ try:
+ result = api.Command['dnsrecord_show'](zone, name)['result']
+ except errors.NotFound as nf:
+ logger.debug("Skipping IPs for %s: %s" % (dnsname, nf))
+ return ()
+ ips = set()
+ for ip in itertools.chain(result.get('arecord', ()),
+ result.get('aaaarecord', ())):
+ if _ip_rdns_ok(ip, fqdn):
+ ips.add(ip)
+ cnames = result.get('cnamerecord', ())
+ if cnames:
+ if dnsname_is_cname:
+ logger.debug("Skipping IPs for %s: chained CNAME" % dnsname)
+ else:
+ for cname in cnames:
+ if not cname.endswith('.'):
+ cname = u'%s.%s' % (cname, zone)
+ ips.update(_san_dnsname_ips(cname, True))
+ return ips
+
+
+def _ip_rdns_ok(ip, fqdn):
+ """
+ Determines whether the IP address has a reverse DNS entry (managed
+ by this IPA instance) that points to the FQDN.
+
+ """
+ rname = dnsutil.DNSName(reversename.from_address(ip))
+ zone = dnsutil.DNSName(resolver.zone_for_name(rname))
+ name = rname.relativize(zone)
+ try:
+ result = api.Command['dnsrecord_show'](zone, name)['result']
+ except errors.NotFound as nf:
+ logger.debug("Skipping IP %s: reverse DNS record not found" % ip)
+ # This assumes that the IP only has 1 A/AAAA record pointing to it.
+ if result['ptrrecord'][0] != fqdn.to_unicode():
+ logger.debug("Skipping IP: %s: reverse DNS doesn't match FQDN %s"
+ % (ip, fqdn))
+ return False
+ else:
+ return True
+
+
@register()
class cert_status(Retrieve, BaseCertMethod, VirtualCommand):
__doc__ = _('Check the status of a certificate signing request.')
--
2.14.3
--
========================================================================
Ian Pilcher arequipeno(a)gmail.com
-------- "I grew up before Mark Zuckerberg invented friendship" --------
========================================================================
From 9e64ec302ad5370a22437d53876ab31f2a237033 Mon Sep 17 00:00:00
2001
From: Ian Pilcher <arequipeno(a)gmail.com>
Date: Fri, 9 Mar 2018 12:33:55 -0600
Subject: [PATCH] Allow issuing certificates with IP addresses in
subjectAltName
Allow issuing certificates with IP addresses in the subject
alternative name (SAN), if all of the following are true.
* One of the DNS names in the SAN resolves to the IP address
(possibly through a CNAME).
* All of the DNS entries in the resolution chain are managed by
this IPA instance.
* The IP address has a (correct) reverse DNS entry that is managed
by this IPA instance
---
ipaserver/plugins/cert.py | 92 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 89 insertions(+), 3 deletions(-)
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index db624357a..452728d21 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -22,11 +22,13 @@
import base64
import collections
import datetime
+import itertools
import logging
from operator import attrgetter
import cryptography.x509
from cryptography.hazmat.primitives import hashes, serialization
+from dns import resolver, reversename
import six
from ipalib import Command, Str, Int, Flag
@@ -48,7 +50,7 @@ from .certprofile import validate_profile_id
from ipalib.text import _
from ipalib.request import context
from ipalib import output
-from ipapython import kerberos
+from ipapython import dnsutil, kerberos
from ipapython.dn import DN
from ipaserver.plugins.service import normalize_principal, validate_realm
@@ -773,9 +775,10 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
"'userCertificate' attribute of entry
'%s'.") % dn)
# Validate the subject alt name, if any
- generalnames = []
+ san_has_ipaddrs = False
+ generalnames = ()
if ext_san is not None:
- generalnames = x509.process_othernames(ext_san.value)
+ generalnames = tuple(x509.process_othernames(ext_san.value))
for gn in generalnames:
if isinstance(gn, cryptography.x509.general_name.DNSName):
if principal.is_user:
@@ -858,11 +861,16 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
"subject alt name type %s is forbidden "
"for non-user principals") %
"RFC822Name"
)
+ elif isinstance(gn, cryptography.x509.general_name.IPAddress):
+ san_has_ipaddrs = True
else:
raise errors.ACIError(
info=_("Subject alt name type %s is forbidden")
% type(gn).__name__)
+ if san_has_ipaddrs:
+ _validate_san_ips(generalnames)
+
# Request the certificate
try:
# re-serialise to PEM, in case the user-supplied data has
@@ -1046,6 +1054,84 @@ def _principal_name_matches_principal(name, principal_obj):
return principal in principal_obj.get('krbprincipalname', [])
+def _validate_san_ips(generalnames):
+ """
+ Raise a ValidationError if the subjectAltName in a CSR includes
+ any IP address(es) that do not match a DNS name in the SAN.
+
+ """
+ san_dns_ips = set()
+ for name in generalnames:
+ if isinstance(name, cryptography.x509.general_name.DNSName):
+ san_dns_ips.update(_san_dnsname_ips(name.value))
+ for name in generalnames:
+ if isinstance(name, cryptography.x509.general_name.IPAddress):
+ if unicode(name.value) not in san_dns_ips:
+ raise errors.ValidationError(
+ name='csr',
+ error=_(
+ "IP address in subjectAltName (%s) does not "
+ "match any DNS name"
+ ) % name.value
+ )
+
+
+def _san_dnsname_ips(dnsname, dnsname_is_cname=False):
+ """
+ Returns a set of IP addresses, managed by this IPa instance,
+ that correspond to the DNS name (from the subjectAltName).
+
+ """
+ fqdn = dnsutil.DNSName(dnsname).make_absolute()
+ if fqdn.__len__() < 4:
+ logger.debug("Skipping IPs for %s: hostname too short" % dnsname)
+ return ()
+ zone = dnsutil.DNSName(resolver.zone_for_name(fqdn))
+ name = fqdn.relativize(zone)
+ try:
+ result = api.Command['dnsrecord_show'](zone, name)['result']
+ except errors.NotFound as nf:
+ logger.debug("Skipping IPs for %s: %s" % (dnsname, nf))
+ return ()
+ ips = set()
+ for ip in itertools.chain(result.get('arecord', ()),
+ result.get('aaaarecord', ())):
+ if _ip_rdns_ok(ip, fqdn):
+ ips.add(ip)
+ cnames = result.get('cnamerecord', ())
+ if cnames:
+ if dnsname_is_cname:
+ logger.debug("Skipping IPs for %s: chained CNAME" % dnsname)
+ else:
+ for cname in cnames:
+ if not cname.endswith('.'):
+ cname = u'%s.%s' % (cname, zone)
+ ips.update(_san_dnsname_ips(cname, True))
+ return ips
+
+
+def _ip_rdns_ok(ip, fqdn):
+ """
+ Determines whether the IP address has a reverse DNS entry (managed
+ by this IPA instance) that points to the FQDN.
+
+ """
+ rname = dnsutil.DNSName(reversename.from_address(ip))
+ zone = dnsutil.DNSName(resolver.zone_for_name(rname))
+ name = rname.relativize(zone)
+ try:
+ result = api.Command['dnsrecord_show'](zone, name)['result']
+ except errors.NotFound as nf:
+ logger.debug("Skipping IP %s: reverse DNS record not found" % ip)
+ # This assumes that the IP only has 1 A/AAAA record pointing to it.
+ if result['ptrrecord'][0] != fqdn.to_unicode():
+ logger.debug("Skipping IP: %s: reverse DNS doesn't match FQDN %s"
+ % (ip, fqdn))
+ return False
+ else:
+ return True
+
+
@register()
class cert_status(Retrieve, BaseCertMethod, VirtualCommand):
__doc__ = _('Check the status of a certificate signing request.')
--
2.14.3
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel(a)lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-leave(a)lists.fedorahosted.org