Thanks for reviews.  Rebased and updated patches attached (and one
new patch).  No substantive changes to 92..94.  Patch order is:

92-2, 93-2, 94-2, 98, 90-3

Other comments inline.

Thanks,
Fraser

On Fri, Aug 12, 2016 at 11:33:28AM +0200, Jan Cholasta wrote:
> Patch 0092: ACK
> 
> Patch 0093: ACK
> 
> Patch 0094: ACK
> 
> Patch 0090:
> 
> 1) Generic otherNames (san_other) do not work correctly. The OID is not
> included in the value and names with complex type other than
> KerberosPrincipal are not parsed correctly. The value should include the OID
> and DER blob of the name.
> 
Updated to use "OID:b64(DER)" as the attribute value.

> 2) With --all, san_other should be included in the result for all
> otherNames, even the known ones, to provide (limited) forward compatibility.
> 
Done; when --all given, known otherName kinds are included in
'san_other' attribute in addition to their own attribute.

> 3) Do we have to support *all* the name types? I mean we could, for the sake
> of completeness, but it might be easier to just keep the few ones we
> actually care about (email, DNS name, principal name, UPN and directory name
> in your patch 0095).
> 
Yeah, why not support them all?  See also Petr's comments in other
branch of thread.

> 4)
> 
> +            obj.setdefault(attr_name, []).append(unicode(name))
> 
> The value should not (always) be unicode, but of the type declared by the
> param (unicode or ipapython.kerberos.Principal or
> ipapython.dnsutil.DNSName).
> 
I now pass the value to the constructor of whatever type the
parameter uses:

        attr_value = self.params[attr_name].type(name_formatted)
        obj.setdefault(attr_name, []).append(attr_value)
From 17e5515ab0eeb92d87091eb00a26dcf358060dba Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Thu, 21 Jul 2016 16:27:49 +1000
Subject: [PATCH 92/94] Move GeneralName parsing code to ipalib.x509

GeneralName parsing code is primarily relevant to X.509.  An
upcoming change will add SAN parsing to the cert-show command, so
first move the GeneralName parsing code from ipalib.pkcs10 to
ipalib.x509.

Part of: https://fedorahosted.org/freeipa/ticket/6022
---
 ipalib/pkcs10.py          |  93 ++-----------------------------------
 ipalib/x509.py            | 114 +++++++++++++++++++++++++++++++++++++++++++++-
 ipaserver/plugins/cert.py |   8 ++--
 3 files changed, 120 insertions(+), 95 deletions(-)

diff --git a/ipalib/pkcs10.py b/ipalib/pkcs10.py
index 
e340c1a2005ad781542a32a0a76753e80364dacf..158ebb3a25be2bd292f3883545fe8afe49b7be8c
 100644
--- a/ipalib/pkcs10.py
+++ b/ipalib/pkcs10.py
@@ -22,9 +22,10 @@ from __future__ import print_function
 import sys
 import base64
 import nss.nss as nss
-from pyasn1.type import univ, char, namedtype, tag
+from pyasn1.type import univ, namedtype, tag
 from pyasn1.codec.der import decoder
 import six
+from ipalib import x509
 
 if six.PY3:
     unicode = str
@@ -32,11 +33,6 @@ if six.PY3:
 PEM = 0
 DER = 1
 
-SAN_DNSNAME = 'DNS name'
-SAN_RFC822NAME = 'RFC822 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.
@@ -72,78 +68,6 @@ def get_extensions(csr, datatype=PEM):
     return tuple(get_prefixed_oid_str(ext)[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):
     """
@@ -159,19 +83,8 @@ def get_subjectaltname(csr, datatype=PEM):
         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 x509.decode_generalnames(extension.value)
 
-    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/x509.py b/ipalib/x509.py
index 
82194922d151a1b0f2df03df3578ad45b43b71c9..15168de08240a84794efef409d022eaa983291c9
 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -40,7 +40,7 @@ import re
 
 import nss.nss as nss
 from nss.error import NSPRError
-from pyasn1.type import univ, namedtype, tag
+from pyasn1.type import univ, char, namedtype, tag
 from pyasn1.codec.der import decoder, encoder
 import six
 
@@ -63,6 +63,11 @@ EKU_EMAIL_PROTECTION = '1.3.6.1.5.5.7.3.4'
 EKU_ANY = '2.5.29.37.0'
 EKU_PLACEHOLDER = '1.3.6.1.4.1.3319.6.10.16'
 
+SAN_DNSNAME = 'DNS name'
+SAN_RFC822NAME = 'RFC822 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)'
+
 _subject_base = None
 
 def subject_base():
@@ -374,6 +379,113 @@ def encode_ext_key_usage(ext_key_usage):
     eku = encoder.encode(eku)
     return _encode_extension('2.5.29.37', EKU_ANY not in ext_key_usage, eku)
 
+
+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()
+
+
+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
+
+
+def decode_generalnames(secitem):
+    """
+    Decode a GeneralNames object (this the data for the Subject
+    Alt Name and Issuer Alt Name extensions, among others).
+
+    ``secitem``
+      The input is the DER-encoded extension data, without the
+      OCTET STRING header, as an nss SecItem object.
+
+    Return a list of tuples of name types (as string, suitable for
+    presentation) and names (as string, suitable for presentation).
+
+    """
+    nss_names = nss.x509_alt_name(secitem, repr_kind=nss.AsObject)
+    asn1_names = decoder.decode(secitem.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 names
+
+
 if __name__ == '__main__':
     # this can be run with:
     # python ipalib/x509.py < /etc/ipa/ca.crt
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
06041d3083565e8d093b610473d6083111d406d2..85be2cf4daeb282b2c2ba866017c8e5745abda6d
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -535,7 +535,7 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
 
         # Validate the subject alt name, if any
         for name_type, name in subjectaltname:
-            if name_type == pkcs10.SAN_DNSNAME:
+            if name_type == x509.SAN_DNSNAME:
                 name = unicode(name)
                 alt_principal_obj = None
                 alt_principal_string = unicode(principal)
@@ -566,13 +566,13 @@ class cert_request(Create, BaseCertMethod, 
VirtualCommand):
                             "with subject alt name '%s'.") % name)
                 if alt_principal_string is not None and not bypass_caacl:
                     caacl_check(principal_type, principal, ca, profile_id)
-            elif name_type in (pkcs10.SAN_OTHERNAME_KRB5PRINCIPALNAME,
-                               pkcs10.SAN_OTHERNAME_UPN):
+            elif name_type in (x509.SAN_OTHERNAME_KRB5PRINCIPALNAME,
+                               x509.SAN_OTHERNAME_UPN):
                 if name != principal_string:
                     raise errors.ACIError(
                         info=_("Principal '%s' in subject alt name does not "
                                "match requested principal") % name)
-            elif name_type == pkcs10.SAN_RFC822NAME:
+            elif name_type == x509.SAN_RFC822NAME:
                 if principal_type == USER:
                     if name not in principal_obj.get('mail', []):
                         raise errors.ValidationError(
-- 
2.5.5

From 27a31d28a0af4a84545678f72ae86946dc9ebeaf Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Fri, 22 Jul 2016 12:05:13 +1000
Subject: [PATCH 93/94] x509: fix SAN directoryName parsing

The subjectAltName extension parsing code in ipalib.x509 fails on
directoryName values because the Choice structure is not endowed
with an inner type.  Implement the Name structure, whose inner type
is a CHOICE { SEQUENCE OF RelativeDistinguishedName }, to resolve.

Note that the structure still does not get fully parsed; only enough
to recognise the SequenceOf tag and not fail.

Part of: https://fedorahosted.org/freeipa/ticket/6022
---
 ipalib/x509.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ipalib/x509.py b/ipalib/x509.py
index 
15168de08240a84794efef409d022eaa983291c9..2dc67441c92686826dd24f00a5ad30566cd032da
 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -196,6 +196,12 @@ def is_self_signed(certificate, datatype=PEM, dbdir=None):
     del nsscert
     return self_signed
 
+class _Name(univ.Choice):
+    componentType = namedtype.NamedTypes(
+        namedtype.NamedType('rdnSequence',
+            univ.SequenceOf()),
+    )
+
 class _TBSCertificate(univ.Sequence):
     componentType = namedtype.NamedTypes(
         namedtype.NamedType(
@@ -204,9 +210,9 @@ class _TBSCertificate(univ.Sequence):
                 tag.tagClassContext, tag.tagFormatSimple, 0))),
         namedtype.NamedType('serialNumber', univ.Integer()),
         namedtype.NamedType('signature', univ.Sequence()),
-        namedtype.NamedType('issuer', univ.Sequence()),
+        namedtype.NamedType('issuer', _Name()),
         namedtype.NamedType('validity', univ.Sequence()),
-        namedtype.NamedType('subject', univ.Sequence()),
+        namedtype.NamedType('subject', _Name()),
         namedtype.NamedType('subjectPublicKeyInfo', univ.Sequence()),
         namedtype.OptionalNamedType(
             'issuerUniquedID',
@@ -403,7 +409,7 @@ class _GeneralName(univ.Choice):
         namedtype.NamedType('x400Address', univ.Sequence().subtype(
             implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 3))
         ),
-        namedtype.NamedType('directoryName', univ.Choice().subtype(
+        namedtype.NamedType('directoryName', _Name().subtype(
             implicitTag=tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 4))
         ),
         namedtype.NamedType('ediPartyName', univ.Sequence().subtype(
-- 
2.5.5

From 6e8dac22d4a985ce344c0d8583260cf5ceccbc1b Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Fri, 22 Jul 2016 12:11:59 +1000
Subject: [PATCH 94/94] x509: use NSS enums and OIDs to identify SAN types

GeneralName parsing currently relies heavily on strings from NSS.
Make the code hopefully less brittle by identifying GeneralName
types by NSS enums and, for otherName, the name-type OID also.

Part of: https://fedorahosted.org/freeipa/ticket/6022
---
 ipalib/x509.py            | 30 +++++++++++++++++++++++-------
 ipaserver/plugins/cert.py | 19 ++++++++++---------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/ipalib/x509.py b/ipalib/x509.py
index 
2dc67441c92686826dd24f00a5ad30566cd032da..541609fbc1a53a73eafcff2327e53a292c2d9a3c
 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -33,6 +33,7 @@
 
 from __future__ import print_function
 
+import collections
 import os
 import sys
 import base64
@@ -63,10 +64,8 @@ EKU_EMAIL_PROTECTION = '1.3.6.1.5.5.7.3.4'
 EKU_ANY = '2.5.29.37.0'
 EKU_PLACEHOLDER = '1.3.6.1.4.1.3319.6.10.16'
 
-SAN_DNSNAME = 'DNS name'
-SAN_RFC822NAME = 'RFC822 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)'
+SAN_UPN = '1.3.6.1.4.1.311.20.2.3'
+SAN_KRB5PRINCIPALNAME = '1.3.6.1.5.2.2'
 
 _subject_base = None
 
@@ -465,6 +464,10 @@ def _decode_krb5principalname(data):
     return name
 
 
+GeneralNameInfo = collections.namedtuple(
+        'GeneralNameInfo', ('type', 'desc', 'value'))
+
+
 def decode_generalnames(secitem):
     """
     Decode a GeneralNames object (this the data for the Subject
@@ -482,12 +485,25 @@ def decode_generalnames(secitem):
     asn1_names = decoder.decode(secitem.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:
+        # NOTE: we use the NSS enum to identify the name type.
+        # (For otherName we also tuple it up with the type-id OID).
+        # The enum does not correspond exactly to the ASN.1 tags.
+        # If we ever want to switch to using the true tag numbers,
+        # the expression to get the tag is:
+        #
+        #   asn1_name.getComponent().getTagSet()[0].asTuple()[2]
+        #
+        if nss_name.type_enum == nss.certOtherName:
+            oid = str(asn1_name['otherName']['type-id'])
+            nametype = (nss_name.type_enum, oid)
+        else:
+            nametype = nss_name.type_enum
+
+        if nametype == (nss.certOtherName, SAN_KRB5PRINCIPALNAME):
             name = _decode_krb5principalname(asn1_name['otherName']['value'])
         else:
             name = nss_name.name
-        names.append((name_type, name))
+        names.append(GeneralNameInfo(nametype, nss_name.type_string, name))
 
     return names
 
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
85be2cf4daeb282b2c2ba866017c8e5745abda6d..207f6964645254ebc417cab80634a68911ae0a08
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -534,8 +534,8 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
                 "to the 'userCertificate' attribute of entry '%s'.") % dn)
 
         # Validate the subject alt name, if any
-        for name_type, name in subjectaltname:
-            if name_type == x509.SAN_DNSNAME:
+        for name_type, desc, name in subjectaltname:
+            if name_type == nss.certDNSName:
                 name = unicode(name)
                 alt_principal_obj = None
                 alt_principal_string = unicode(principal)
@@ -549,7 +549,7 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
                         raise errors.ValidationError(
                             name='csr',
                             error=_("subject alt name type %s is forbidden "
-                                "for user principals") % name_type
+                                "for user principals") % desc
                         )
                 except errors.NotFound:
                     # We don't want to issue any certificates referencing
@@ -566,13 +566,15 @@ class cert_request(Create, BaseCertMethod, 
VirtualCommand):
                             "with subject alt name '%s'.") % name)
                 if alt_principal_string is not None and not bypass_caacl:
                     caacl_check(principal_type, principal, ca, profile_id)
-            elif name_type in (x509.SAN_OTHERNAME_KRB5PRINCIPALNAME,
-                               x509.SAN_OTHERNAME_UPN):
+            elif name_type in [
+                (nss.certOtherName, x509.SAN_UPN),
+                (nss.certOtherName, x509.SAN_KRB5PRINCIPALNAME),
+            ]:
                 if name != principal_string:
                     raise errors.ACIError(
                         info=_("Principal '%s' in subject alt name does not "
                                "match requested principal") % name)
-            elif name_type == x509.SAN_RFC822NAME:
+            elif name_type == nss.certRFC822Name:
                 if principal_type == USER:
                     if name not in principal_obj.get('mail', []):
                         raise errors.ValidationError(
@@ -585,12 +587,11 @@ class cert_request(Create, BaseCertMethod, 
VirtualCommand):
                     raise errors.ValidationError(
                         name='csr',
                         error=_("subject alt name type %s is forbidden "
-                            "for non-user principals") % name_type
+                            "for non-user principals") % desc
                     )
             else:
                 raise errors.ACIError(
-                    info=_("Subject alt name type %s is forbidden") %
-                         name_type)
+                    info=_("Subject alt name type %s is forbidden") % desc)
 
         # Request the certificate
         result = self.Backend.ra.request_certificate(
-- 
2.5.5

From 9481e0436dc46b4668f1a45bd21f97c2096da142 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Mon, 15 Aug 2016 15:39:49 +1000
Subject: [PATCH] x509: include otherName DER value in GeneralNameInfo

We want to include the whole DER value when we pretty-print
unrecognised otherNames, so add a field to the GeneralNameInfo
namedtuple and populate it for otherNames.

Part of: https://fedorahosted.org/freeipa/ticket/6022
---
 ipalib/x509.py            | 13 +++++++++----
 ipaserver/plugins/cert.py |  2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/ipalib/x509.py b/ipalib/x509.py
index 
541609fbc1a53a73eafcff2327e53a292c2d9a3c..e986a97a58aafd3aeab08765a397edbf67c7841a
 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -465,7 +465,7 @@ def _decode_krb5principalname(data):
 
 
 GeneralNameInfo = collections.namedtuple(
-        'GeneralNameInfo', ('type', 'desc', 'value'))
+        'GeneralNameInfo', ('type', 'desc', 'value', 'der_value'))
 
 
 def decode_generalnames(secitem):
@@ -477,8 +477,9 @@ def decode_generalnames(secitem):
       The input is the DER-encoded extension data, without the
       OCTET STRING header, as an nss SecItem object.
 
-    Return a list of tuples of name types (as string, suitable for
-    presentation) and names (as string, suitable for presentation).
+    Return a list of ``GeneralNameInfo`` namedtuples.  The
+    ``der_value`` field is set for otherNames, otherwise it is
+    ``None``.
 
     """
     nss_names = nss.x509_alt_name(secitem, repr_kind=nss.AsObject)
@@ -496,14 +497,18 @@ def decode_generalnames(secitem):
         if nss_name.type_enum == nss.certOtherName:
             oid = str(asn1_name['otherName']['type-id'])
             nametype = (nss_name.type_enum, oid)
+            der_value = asn1_name['otherName']['value'].asOctets()
         else:
             nametype = nss_name.type_enum
+            der_value = None
 
         if nametype == (nss.certOtherName, SAN_KRB5PRINCIPALNAME):
             name = _decode_krb5principalname(asn1_name['otherName']['value'])
         else:
             name = nss_name.name
-        names.append(GeneralNameInfo(nametype, nss_name.type_string, name))
+
+        gni = GeneralNameInfo(nametype, nss_name.type_string, name, der_value)
+        names.append(gni)
 
     return names
 
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
207f6964645254ebc417cab80634a68911ae0a08..30c708113942fca4d1f11aa1219367110e518309
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -534,7 +534,7 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
                 "to the 'userCertificate' attribute of entry '%s'.") % dn)
 
         # Validate the subject alt name, if any
-        for name_type, desc, name in subjectaltname:
+        for name_type, desc, name, der_name in subjectaltname:
             if name_type == nss.certDNSName:
                 name = unicode(name)
                 alt_principal_obj = None
-- 
2.5.5

From 0706141a0457a90e58c94527c65d7f1ead87c719 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftwee...@redhat.com>
Date: Thu, 14 Jul 2016 21:36:33 +1000
Subject: [PATCH] cert-show: show subject alternative names

Enhance the cert-show command to return subject alternative name
values.

Fixes: https://fedorahosted.org/freeipa/ticket/6022
---
 ipaserver/plugins/cert.py | 125 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 120 insertions(+), 5 deletions(-)

diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 
30c708113942fca4d1f11aa1219367110e518309..c42ba2de06df5d6204275fb9d31694268814a269
 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -38,7 +38,7 @@ from ipalib import ngettext
 from ipalib.constants import IPA_CA_CN
 from ipalib.crud import Create, PKQuery, Retrieve, Search
 from ipalib.frontend import Method, Object
-from ipalib.parameters import Bytes, DateTime, DNParam, Principal
+from ipalib.parameters import Bytes, DateTime, DNParam, DNSNameParam, Principal
 from ipalib.plugable import Registry
 from .virtual import VirtualCommand
 from .baseldap import pkey_to_value
@@ -49,6 +49,7 @@ from ipalib.request import context
 from ipalib import output
 from ipapython import kerberos
 from ipapython.dn import DN
+from ipapython.ipa_log_manager import root_logger
 from ipaserver.plugins.service import normalize_principal, validate_realm
 
 if six.PY3:
@@ -293,9 +294,74 @@ class BaseCertObject(Object):
             label=_('Serial number (hex)'),
             flags={'no_create', 'no_update', 'no_search'},
         ),
+        Str(
+            'san_rfc822name*',
+            label=_('Subject Alternative Name (Email)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        DNSNameParam(
+            'san_dnsname*',
+            label=_('Subject Alternative Name (DNS)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_x400address*',
+            label=_('Subject Alternative Name (X.400 address)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_directoryname*',
+            label=_('Subject Alternative Name (Directory name)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_edipartyname*',
+            label=_('Subject Alternative Name (EDI Party name)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_uri*',
+            label=_('Subject Alternative Name (URI)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_ipaddress*',
+            label=_('Subject Alternative Name (IP Address)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_oid*',
+            label=_('Subject Alternative Name (OID)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Principal(
+            'san_other_upn*',
+            label=_('Subject Alternative Name (UPN)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Principal(
+            'san_other_kpn*',
+            label=_('Subject Alternative Name (Kerberos Principal)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
+        Str(
+            'san_other*',
+            label=_('Subject Alternative Name (Other Name)'),
+            flags={'no_create', 'no_update', 'no_search'},
+        ),
     )
 
-    def _parse(self, obj):
+    def _parse(self, obj, generic_othernames):
+        """Extract certificate-specific data into a result object.
+
+        ``obj``
+            Result object containing certificate, into which extracted
+            data will be inserted.
+        ``generic_othernames``
+            If ``True`` add recognised otherNames to the generic
+            ``san_other`` attribute as well as their own attribute.
+
+        """
         cert = x509.load_certificate(obj['certificate'])
         obj['subject'] = DN(unicode(cert.subject))
         obj['issuer'] = DN(unicode(cert.issuer))
@@ -308,6 +374,55 @@ class BaseCertObject(Object):
         obj['serial_number'] = cert.serial_number
         obj['serial_number_hex'] = u'0x%X' % cert.serial_number
 
+        try:
+            ext_san = cert.get_extension(nss.SEC_OID_X509_SUBJECT_ALT_NAME)
+            general_names = x509.decode_generalnames(ext_san.value)
+        except KeyError:
+            general_names = []
+
+        for name_type, desc, name, der_name in general_names:
+            try:
+                self._add_san_attribute(
+                    obj, generic_othernames, name_type, name, der_name)
+            except Exception as e:
+                # Invalid GeneralName (i.e. not a valid X.509 cert);
+                # don't fail but log something about it
+                root_logger.warning(
+                    "Encountered bad GeneralName; skipping", exc_info=True)
+
+
+    def _add_san_attribute(
+            self, obj, generic_othernames, name_type, name, der_name):
+        name_type_map = {
+            nss.certRFC822Name: 'san_rfc822name',
+            nss.certDNSName: 'san_dnsname',
+            nss.certX400Address: 'san_x400address',
+            nss.certDirectoryName: 'san_directoryname',
+            nss.certEDIPartyName: 'san_edipartyname',
+            nss.certURI: 'san_uri',
+            nss.certIPAddress: 'san_ipaddress',
+            nss.certRegisterID: 'san_oid',
+            (nss.certOtherName, x509.SAN_UPN): 'san_other_upn',
+            (nss.certOtherName, x509.SAN_KRB5PRINCIPALNAME): 'san_other_kpn',
+        }
+
+        attr_name = name_type_map.get(name_type, 'san_other')
+
+        if attr_name != 'san_other':
+            name_formatted = name
+        else:
+            # display as "OID : b64(DER)"
+            name_formatted = u'{}:{}'.format(
+                name_type[1], base64.b64encode(der_name))
+        attr_value = self.params[attr_name].type(name_formatted)
+        obj.setdefault(attr_name, []).append(attr_value)
+
+        if generic_othernames and attr_name.startswith('san_other_'):
+            # recurse, faking the name type so that OID is still conveyed,
+            # but it won't be found in name_type_map
+            self._add_san_attribute(
+                obj, False, (None, name_type[1]), name, der_name)
+
 
 class BaseCertMethod(Method):
     def get_options(self):
@@ -597,7 +712,7 @@ class cert_request(Create, BaseCertMethod, VirtualCommand):
         result = self.Backend.ra.request_certificate(
             csr, profile_id, ca_id, request_type=request_type)
         if not raw:
-            self.obj._parse(result)
+            self.obj._parse(result, all)
             result['request_id'] = int(result['request_id'])
 
         # Success? Then add it to the principal's entry
@@ -775,7 +890,7 @@ class cert_show(Retrieve, CertMethod, VirtualCommand):
 
         if not raw:
             result['certificate'] = result['certificate'].replace('\r\n', '')
-            self.obj._parse(result)
+            self.obj._parse(result, all)
             result['revoked'] = ('revocation_reason' in result)
             if 'owner' in result:
                 self.obj._fill_owners(result)
@@ -1143,7 +1258,7 @@ class cert_find(Search, CertMethod):
                     if 'certificate' in obj:
                         obj['certificate'] = (
                             obj['certificate'].replace('\r\n', ''))
-                        self.obj._parse(obj)
+                        self.obj._parse(obj, all)
                         if not all:
                             del obj['certificate']
                             del obj['valid_not_before']
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to