Make DistributionPoint relative_name a set of NameAttribute (#3210)

* Add RelativeDistinguishedName class

* Make relative_name a RelativeDistinguishedName

DistributionPoint relative_name is currently a Name but RFC 5280
defines it as RelativeDistinguishedName, i.e. a non-empty SET OF
name attributes.  Change the DistributionPoint relative_name
attribute to be a RelativeDistinguishedName.

9 files changed, 187 insertions, 22 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1b81312f..40ea4a64 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -26,6 +26,12 @@ Changelog
   :meth:`~cryptography.x509.random_serial_number`.
 * Added support for encoding ``IPv4Network`` and ``IPv6Network`` in X.509
   certificates for use with :class:`~cryptography.x509.NameConstraints`.
+* Added :class:`~cryptography.x509.RelativeDistinguishedName`
+* :class:`~cryptography.x509.DistributionPoint` now accepts
+  :class:`~cryptography.x509.RelativeDistinguishedName` for
+  :attr:`~cryptography.x509.DistributionPoint.relative_name`.
+  Deprecated use of :class:`~cryptography.x509.Name` as
+  :attr:`~cryptography.x509.DistributionPoint.relative_name`.
 
 1.5.3 - 2016-11-05
 ~~~~~~~~~~~~~~~~~~
diff --git a/docs/x509/reference.rst b/docs/x509/reference.rst
index bee7c176..c5623315 100644
--- a/docs/x509/reference.rst
+++ b/docs/x509/reference.rst
@@ -1156,6 +1156,22 @@ X.509 CSR (Certificate Signing Request) Builder Object
 
         The value of the attribute.
 
+
+.. class:: RelativeDistinguishedName(attributes)
+
+    .. versionadded:: 1.6
+
+    A relative distinguished name is a non-empty set of name attributes.  The
+    object is iterable to get every attribute.
+
+    .. method:: get_attributes_for_oid(oid)
+
+        :param oid: An :class:`ObjectIdentifier` instance.
+
+        :returns: A list of :class:`NameAttribute` instances that match the OID
+            provided.  The list should contain zero or one values.
+
+
 .. class:: ObjectIdentifier
 
     .. versionadded:: 0.8
@@ -1851,12 +1867,15 @@ X.509 Extensions
 
     .. attribute:: relative_name
 
-        :type: :class:`Name` or None
+        :type: :class:`RelativeDistinguishedName` or None
 
         This field describes methods to retrieve the CRL relative to the CRL
         issuer. At most one of ``full_name`` or ``relative_name`` will be
         non-None.
 
+        .. versionchanged:: 1.6
+            Changed from :class:`Name` to :class:`RelativeDistinguishedName`.
+
     .. attribute:: crl_issuer
 
         :type: list of :class:`GeneralName` instances or None
diff --git a/src/cryptography/hazmat/backends/openssl/decode_asn1.py b/src/cryptography/hazmat/backends/openssl/decode_asn1.py
index af9d3920..f8e8c95c 100644
--- a/src/cryptography/hazmat/backends/openssl/decode_asn1.py
+++ b/src/cryptography/hazmat/backends/openssl/decode_asn1.py
@@ -544,7 +544,11 @@ def _decode_crl_distribution_points(backend, cdps):
                 )
             # OpenSSL code doesn't test for a specific type for
             # relativename, everything that isn't fullname is considered
-            # relativename.
+            # relativename.  Per RFC 5280:
+            #
+            # DistributionPointName ::= CHOICE {
+            #      fullName                [0]      GeneralNames,
+            #      nameRelativeToCRLIssuer [1]      RelativeDistinguishedName }
             else:
                 rns = cdp.distpoint.name.relativename
                 rnum = backend._lib.sk_X509_NAME_ENTRY_num(rns)
@@ -558,7 +562,7 @@ def _decode_crl_distribution_points(backend, cdps):
                         _decode_x509_name_entry(backend, rn)
                     )
 
-                relative_name = x509.Name(attributes)
+                relative_name = x509.RelativeDistinguishedName(attributes)
 
         dist_points.append(
             x509.DistributionPoint(
diff --git a/src/cryptography/utils.py b/src/cryptography/utils.py
index 48ed449a..159d1ded 100644
--- a/src/cryptography/utils.py
+++ b/src/cryptography/utils.py
@@ -17,6 +17,7 @@ import warnings
 # ends.
 DeprecatedIn10 = DeprecationWarning
 DeprecatedIn14 = DeprecationWarning
+DeprecatedIn16 = DeprecationWarning
 
 
 def read_only_property(name):
diff --git a/src/cryptography/x509/__init__.py b/src/cryptography/x509/__init__.py
index feab4497..51914e1e 100644
--- a/src/cryptography/x509/__init__.py
+++ b/src/cryptography/x509/__init__.py
@@ -30,7 +30,9 @@ from cryptography.x509.general_name import (
     RegisteredID, UniformResourceIdentifier, UnsupportedGeneralNameType,
     _GENERAL_NAMES
 )
-from cryptography.x509.name import Name, NameAttribute
+from cryptography.x509.name import (
+    Name, NameAttribute, RelativeDistinguishedName
+)
 from cryptography.x509.oid import (
     AuthorityInformationAccessOID, CRLEntryExtensionOID,
     CertificatePoliciesOID, ExtendedKeyUsageOID, ExtensionOID, NameOID,
@@ -122,6 +124,7 @@ __all__ = [
     "UnsupportedGeneralNameType",
     "NameAttribute",
     "Name",
+    "RelativeDistinguishedName",
     "ObjectIdentifier",
     "ExtensionType",
     "Extensions",
diff --git a/src/cryptography/x509/extensions.py b/src/cryptography/x509/extensions.py
index c0705a3a..f7f6fcd3 100644
--- a/src/cryptography/x509/extensions.py
+++ b/src/cryptography/x509/extensions.py
@@ -8,6 +8,7 @@ import abc
 import datetime
 import hashlib
 import ipaddress
+import warnings
 from enum import Enum
 
 from pyasn1.codec.der import decoder
@@ -20,7 +21,7 @@ from cryptography.hazmat.primitives import constant_time, serialization
 from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePublicKey
 from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicKey
 from cryptography.x509.general_name import GeneralName, IPAddress, OtherName
-from cryptography.x509.name import Name
+from cryptography.x509.name import Name, RelativeDistinguishedName
 from cryptography.x509.oid import (
     CRLEntryExtensionOID, ExtensionOID, ObjectIdentifier
 )
@@ -437,8 +438,20 @@ class DistributionPoint(object):
                     "full_name must be a list of GeneralName objects"
                 )
 
-        if relative_name and not isinstance(relative_name, Name):
-            raise TypeError("relative_name must be a Name")
+        if relative_name:
+            if isinstance(relative_name, Name):
+                warnings.warn(
+                    "relative_name=<Name> is deprecated and will "
+                    "be removed in a future version; use "
+                    "<RelativeDistinguishedName> instead.",
+                    utils.DeprecatedIn16,
+                    stacklevel=2
+                )
+                relative_name = RelativeDistinguishedName(relative_name)
+            elif not isinstance(relative_name, RelativeDistinguishedName):
+                raise TypeError(
+                    "relative_name must be a RelativeDistinguishedName"
+                )
 
         if crl_issuer:
             crl_issuer = list(crl_issuer)
diff --git a/src/cryptography/x509/name.py b/src/cryptography/x509/name.py
index 7e55f6e3..7dc80646 100644
--- a/src/cryptography/x509/name.py
+++ b/src/cryptography/x509/name.py
@@ -52,6 +52,41 @@ class NameAttribute(object):
         return "<NameAttribute(oid={0.oid}, value={0.value!r})>".format(self)
 
 
+class RelativeDistinguishedName(object):
+    def __init__(self, attributes):
+        attributes = frozenset(attributes)
+        if not attributes:
+            raise ValueError("a relative distinguished name cannot be empty")
+        if not all(isinstance(x, NameAttribute) for x in attributes):
+            raise TypeError("attributes must be an iterable of NameAttribute")
+
+        self._attributes = attributes
+
+    def get_attributes_for_oid(self, oid):
+        return [i for i in self if i.oid == oid]
+
+    def __eq__(self, other):
+        if not isinstance(other, RelativeDistinguishedName):
+            return NotImplemented
+
+        return self._attributes == other._attributes
+
+    def __ne__(self, other):
+        return not self == other
+
+    def __hash__(self):
+        return hash(self._attributes)
+
+    def __iter__(self):
+        return iter(self._attributes)
+
+    def __len__(self):
+        return len(self._attributes)
+
+    def __repr__(self):
+        return "<RelativeDistinguishedName({0!r})>".format(list(self))
+
+
 class Name(object):
     def __init__(self, attributes):
         attributes = list(attributes)
diff --git a/tests/test_x509.py b/tests/test_x509.py
index d3b24ecc..67df30c0 100644
--- a/tests/test_x509.py
+++ b/tests/test_x509.py
@@ -1895,7 +1895,7 @@ class TestCertificateBuilder(object):
             x509.CRLDistributionPoints([
                 x509.DistributionPoint(
                     full_name=None,
-                    relative_name=x509.Name([
+                    relative_name=x509.RelativeDistinguishedName([
                         x509.NameAttribute(
                             NameOID.COMMON_NAME,
                             u"indirect CRL for indirectCRL CA3"
@@ -3604,6 +3604,77 @@ class TestNameAttribute(object):
             )
 
 
+class TestRelativeDistinguishedName(object):
+    def test_init_empty(self):
+        with pytest.raises(ValueError):
+            x509.RelativeDistinguishedName([])
+
+    def test_init_not_nameattribute(self):
+        with pytest.raises(TypeError):
+            x509.RelativeDistinguishedName(["not-a-NameAttribute"])
+
+    def test_init_duplicate_attribute(self):
+        rdn = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+        ])
+        assert len(rdn) == 1
+
+    def test_hash(self):
+        rdn1 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value2'),
+        ])
+        rdn2 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value2'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+        ])
+        rdn3 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value3'),
+        ])
+        assert hash(rdn1) == hash(rdn2)
+        assert hash(rdn1) != hash(rdn3)
+
+    def test_eq(self):
+        rdn1 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value2'),
+        ])
+        rdn2 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value2'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+        ])
+        assert rdn1 == rdn2
+
+    def test_ne(self):
+        rdn1 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value2'),
+        ])
+        rdn2 = x509.RelativeDistinguishedName([
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1'),
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.2'), u'value3'),
+        ])
+        assert rdn1 != rdn2
+        assert rdn1 != object()
+
+    def test_iter_input(self):
+        attrs = [
+            x509.NameAttribute(x509.ObjectIdentifier('2.999.1'), u'value1')
+        ]
+        rdn = x509.RelativeDistinguishedName(iter(attrs))
+        assert list(rdn) == attrs
+        assert list(rdn) == attrs
+
+    def test_get_attributes_for_oid(self):
+        oid = x509.ObjectIdentifier('2.999.1')
+        attr = x509.NameAttribute(oid, u'value1')
+        rdn = x509.RelativeDistinguishedName([attr])
+        assert rdn.get_attributes_for_oid(oid) == [attr]
+        assert rdn.get_attributes_for_oid(x509.ObjectIdentifier('1.2.3')) == []
+
+
 class TestObjectIdentifier(object):
     def test_eq(self):
         oid1 = x509.ObjectIdentifier('2.999.1')
diff --git a/tests/test_x509_ext.py b/tests/test_x509_ext.py
index 749e52f1..7104121d 100644
--- a/tests/test_x509_ext.py
+++ b/tests/test_x509_ext.py
@@ -3003,6 +3003,17 @@ class TestDistributionPoint(object):
         with pytest.raises(ValueError):
             x509.DistributionPoint("data", "notname", None, None)
 
+    def test_relative_name_name_value_deprecated(self):
+        with pytest.deprecated_call():
+            x509.DistributionPoint(
+                None,
+                x509.Name([
+                    x509.NameAttribute(NameOID.COMMON_NAME, u"myCN")
+                ]),
+                None,
+                None
+            )
+
     def test_crl_issuer_not_general_names(self):
         with pytest.raises(TypeError):
             x509.DistributionPoint(None, None, None, ["notgn"])
@@ -3127,7 +3138,7 @@ class TestDistributionPoint(object):
     def test_repr(self):
         dp = x509.DistributionPoint(
             None,
-            x509.Name([
+            x509.RelativeDistinguishedName([
                 x509.NameAttribute(NameOID.COMMON_NAME, u"myCN")
             ]),
             frozenset([x509.ReasonFlags.ca_compromise]),
@@ -3143,21 +3154,23 @@ class TestDistributionPoint(object):
         )
         if six.PY3:
             assert repr(dp) == (
-                "<DistributionPoint(full_name=None, relative_name=<Name([<Name"
-                "Attribute(oid=<ObjectIdentifier(oid=2.5.4.3, name=commonName)"
-                ">, value='myCN')>])>, reasons=frozenset({<ReasonFlags.ca_comp"
-                "romise: 'cACompromise'>}), crl_issuer=[<DirectoryName(value=<"
-                "Name([<NameAttribute(oid=<ObjectIdentifier(oid=2.5.4.3, name="
-                "commonName)>, value='Important CA')>])>)>])>"
+                "<DistributionPoint(full_name=None, relative_name=<RelativeDis"
+                "tinguishedName([<NameAttribute(oid=<ObjectIdentifier(oid=2.5."
+                "4.3, name=commonName)>, value='myCN')>])>, reasons=frozenset("
+                "{<ReasonFlags.ca_compromise: 'cACompromise'>}), crl_issuer=[<"
+                "DirectoryName(value=<Name([<NameAttribute(oid=<ObjectIdentifi"
+                "er(oid=2.5.4.3, name=commonName)>, value='Important CA')>])>)"
+                ">])>"
             )
         else:
             assert repr(dp) == (
-                "<DistributionPoint(full_name=None, relative_name=<Name([<Name"
-                "Attribute(oid=<ObjectIdentifier(oid=2.5.4.3, name=commonName)"
-                ">, value=u'myCN')>])>, reasons=frozenset([<ReasonFlags.ca_com"
-                "promise: 'cACompromise'>]), crl_issuer=[<DirectoryName(value="
-                "<Name([<NameAttribute(oid=<ObjectIdentifier(oid=2.5.4.3, name"
-                "=commonName)>, value=u'Important CA')>])>)>])>"
+                "<DistributionPoint(full_name=None, relative_name=<RelativeDis"
+                "tinguishedName([<NameAttribute(oid=<ObjectIdentifier(oid=2.5."
+                "4.3, name=commonName)>, value=u'myCN')>])>, reasons=frozenset"
+                "([<ReasonFlags.ca_compromise: 'cACompromise'>]), crl_issuer=["
+                "<DirectoryName(value=<Name([<NameAttribute(oid=<ObjectIdentif"
+                "ier(oid=2.5.4.3, name=commonName)>, value=u'Important CA')>])"
+                ">)>])>"
             )
 
 
@@ -3407,7 +3420,7 @@ class TestCRLDistributionPointsExtension(object):
         assert cdps == x509.CRLDistributionPoints([
             x509.DistributionPoint(
                 full_name=None,
-                relative_name=x509.Name([
+                relative_name=x509.RelativeDistinguishedName([
                     x509.NameAttribute(
                         NameOID.COMMON_NAME,
                         u"indirect CRL for indirectCRL CA3"