Bump for review.

On Mon, Aug 15, 2016 at 05:15:16PM +1000, Fraser Tweedale wrote:
> 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