separate full_name/relative_name and change reasons to an enumeration
diff --git a/docs/x509.rst b/docs/x509.rst
index 9ef8e14..3cf4f90 100644
--- a/docs/x509.rst
+++ b/docs/x509.rst
@@ -812,11 +812,19 @@
.. versionadded:: 0.9
- .. attribute:: distribution_point
+ .. attribute:: full_name
- :type: list of :class:`GeneralName` instances, :class:`Name`, or None
+ :type: list of :class:`GeneralName` instances or None
- This field describes methods to retrieve the CRL.
+ This field describes methods to retrieve the CRL. If this is not None
+ then ``relative_name`` must be None.
+
+ .. attribute:: relative_name
+
+ :type: :class:`Name` or None
+
+ This field describes methods to retrieve the CRL relative to the CRL
+ issuer. If this is not None then ``full_name`` must be None.
.. attribute:: crl_issuer
@@ -826,7 +834,7 @@
.. attribute:: reasons
- :type: :class:`ReasonFlags` or None
+ :type: list of :class:`ReasonFlags` or None
The reasons a given distribution point may be used for when performing
revocation checks.
@@ -835,40 +843,53 @@
.. versionadded:: 0.9
- This class holds reasons a distribution point may be used for when
- performing revocation checks.
+ An enumeration for CRL reasons.
+
+ .. attribute:: unspecified
+
+ It is unspecified why the certificate was revoked. This reason cannot
+ be used as a reason flag in a :class:`DistributionPoint`.
.. attribute:: key_compromise
- :type: bool
+ This reason indicates that the private key was compromised.
.. attribute:: ca_compromise
- :type: bool
+ This reason indicates that the CA issuing the certificate was
+ compromised.
.. attribute:: affiliation_changed
- :type: bool
+ This reason indicates that the subject's name or other information has
+ changed.
.. attribute:: superseded
- :type: bool
+ This reason indicates that a certificate has been superseded.
.. attribute:: cessation_of_operation
- :type: bool
+ This reason indicates that the certificate is no longer required.
.. attribute:: certificate_hold
- :type: bool
+ This reason indicates that the certificate is on hold.
.. attribute:: privilege_withdrawn
- :type: bool
+ This reason indicates that the privilege granted by this certificate
+ have been withdrawn.
.. attribute:: aa_compromise
- :type: bool
+ When an attribute authority has been compromised.
+
+ .. attribute:: remove_from_crl
+
+ This reason indicates that the certificate was on hold and should be
+ removed from the CRL. This reason cannot be used as a reason flag
+ in a :class:`DistributionPoint`.
Object Identifiers
~~~~~~~~~~~~~~~~~~
diff --git a/src/cryptography/x509.py b/src/cryptography/x509.py
index 671294e..cee0cc3 100644
--- a/src/cryptography/x509.py
+++ b/src/cryptography/x509.py
@@ -513,20 +513,21 @@
class DistributionPoint(object):
- def __init__(self, distribution_point, reasons, crl_issuer):
- if distribution_point:
- if (
- (
- isinstance(distribution_point, list) and
- not all(
- isinstance(x, GeneralName) for x in distribution_point
- )
- ) or not isinstance(distribution_point, (list, Name))
- ):
- raise TypeError(
- "distribution_point must be None, a list of general names"
- ", or a Name"
- )
+ def __init__(self, full_name, relative_name, reasons, crl_issuer):
+ if full_name and relative_name:
+ raise ValueError(
+ "At least one of full_name and relative_name must be None"
+ )
+
+ if full_name and not all(
+ isinstance(x, GeneralName) for x in full_name
+ ):
+ raise TypeError(
+ "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 crl_issuer and not all(
isinstance(x, GeneralName) for x in crl_issuer
@@ -535,23 +536,36 @@
"crl_issuer must be None or a list of general names"
)
- if reasons and not isinstance(reasons, ReasonFlags):
- raise TypeError("reasons must be None or ReasonFlags")
+ if reasons and not all(
+ isinstance(x, ReasonFlags) for x in reasons
+ ):
+ raise TypeError("reasons must be None or list of ReasonFlags")
- if reasons and not crl_issuer and not distribution_point:
+ if reasons and (
+ ReasonFlags.unspecified in reasons or
+ ReasonFlags.remove_from_crl in reasons
+ ):
raise ValueError(
- "You must supply crl_issuer or distribution_point when "
+ "unspecified and remove_from_crl are not valid reasons in a "
+ "DistributionPoint"
+ )
+
+ if reasons and not crl_issuer and not (full_name or relative_name):
+ raise ValueError(
+ "You must supply crl_issuer, full_name, or relative_name when "
"reasons is not None"
)
- self._distribution_point = distribution_point
+ self._full_name = full_name
+ self._relative_name = relative_name
self._reasons = reasons
self._crl_issuer = crl_issuer
def __repr__(self):
return (
- "<DistributionPoint(distribution_point={0.distribution_point}, rea"
- "sons={0.reasons}, crl_issuer={0.crl_issuer})>".format(self)
+ "<DistributionPoint(full_name={0.full_name}, relative_name={0.rela"
+ "tive_name}, reasons={0.reasons}, crl_issuer={0.crl_is"
+ "suer})>".format(self)
)
def __eq__(self, other):
@@ -559,7 +573,8 @@
return NotImplemented
return (
- self.distribution_point == other.distribution_point and
+ self.full_name == other.full_name and
+ self.relative_name == other.relative_name and
self.reasons == other.reasons and
self.crl_issuer == other.crl_issuer
)
@@ -567,62 +582,23 @@
def __ne__(self, other):
return not self == other
- distribution_point = utils.read_only_property("_distribution_point")
+ full_name = utils.read_only_property("_full_name")
+ relative_name = utils.read_only_property("_relative_name")
reasons = utils.read_only_property("_reasons")
crl_issuer = utils.read_only_property("_crl_issuer")
-class ReasonFlags(object):
- def __init__(self, key_compromise, ca_compromise, affiliation_changed,
- superseded, cessation_of_operation, certificate_hold,
- privilege_withdrawn, aa_compromise):
- self._key_compromise = key_compromise
- self._ca_compromise = ca_compromise
- self._affiliation_changed = affiliation_changed
- self._superseded = superseded
- self._cessation_of_operation = cessation_of_operation
- self._certificate_hold = certificate_hold
- self._privilege_withdrawn = privilege_withdrawn
- self._aa_compromise = aa_compromise
-
- def __repr__(self):
- return (
- "<ReasonFlags(key_compromise={0.key_compromise}, ca_compromise"
- "={0.ca_compromise}, affiliation_changed={0.affiliation_changed},"
- "superseded={0.superseded}, cessation_of_operation={0.cessation_o"
- "f_operation}, certificate_hold={0.certificate_hold}, privilege_w"
- "ithdrawn={0.privilege_withdrawn}, aa_compromise={0.aa_compromise"
- "})>".format(self)
- )
-
- def __eq__(self, other):
- if not isinstance(other, ReasonFlags):
- return NotImplemented
-
- return (
- self.key_compromise == other.key_compromise and
- self.ca_compromise == other.ca_compromise and
- self.affiliation_changed == other.affiliation_changed and
- self.superseded == other.superseded and
- self.cessation_of_operation == other.cessation_of_operation and
- self.certificate_hold == other.certificate_hold and
- self.privilege_withdrawn == other.privilege_withdrawn and
- self.aa_compromise == other.aa_compromise
- )
-
- def __ne__(self, other):
- return not self == other
-
- key_compromise = utils.read_only_property("_key_compromise")
- ca_compromise = utils.read_only_property("_ca_compromise")
- affiliation_changed = utils.read_only_property("_affiliation_changed")
- superseded = utils.read_only_property("_superseded")
- cessation_of_operation = utils.read_only_property(
- "_cessation_of_operation"
- )
- certificate_hold = utils.read_only_property("_certificate_hold")
- privilege_withdrawn = utils.read_only_property("_privilege_withdrawn")
- aa_compromise = utils.read_only_property("_aa_compromise")
+class ReasonFlags(Enum):
+ unspecified = "unspecified"
+ key_compromise = "keyCompromise"
+ ca_compromise = "cACompromise"
+ affiliation_changed = "affiliationChanged"
+ superseded = "superseded"
+ cessation_of_operation = "cessationOfOperation"
+ certificate_hold = "certificateHold"
+ privilege_withdrawn = "privilegeWithdrawn"
+ aa_compromise = "aACompromise"
+ remove_from_crl = "removeFromCRL"
@six.add_metaclass(abc.ABCMeta)
diff --git a/tests/test_x509_ext.py b/tests/test_x509_ext.py
index 1ccb361..e0858c7 100644
--- a/tests/test_x509_ext.py
+++ b/tests/test_x509_ext.py
@@ -1320,69 +1320,46 @@
assert ext.value.authority_cert_serial_number == 3
-class TestReasonFlags(object):
- def test_flags(self):
- flags = x509.ReasonFlags(
- True, True, False, False, True, True, False, False
- )
- assert flags.key_compromise is True
- assert flags.ca_compromise is True
- assert flags.affiliation_changed is False
- assert flags.superseded is False
- assert flags.cessation_of_operation is True
- assert flags.certificate_hold is True
- assert flags.privilege_withdrawn is False
- assert flags.aa_compromise is False
-
- def test_eq(self):
- flags = x509.ReasonFlags(
- True, True, False, False, True, True, False, False
- )
- flags2 = x509.ReasonFlags(
- True, True, False, False, True, True, False, False
- )
- assert flags == flags2
-
- def test_ne(self):
- flags = x509.ReasonFlags(
- True, True, False, False, True, True, False, False
- )
- flags2 = x509.ReasonFlags(
- True, True, False, False, True, True, False, True
- )
- assert flags != flags2
- assert flags != object()
-
- def test_repr(self):
- flags = x509.ReasonFlags(
- True, True, False, False, True, True, False, False
- )
- assert repr(flags) == (
- "<ReasonFlags(key_compromise=True, ca_compromise=True, affiliatio"
- "n_changed=False,superseded=False, cessation_of_operation=True, c"
- "ertificate_hold=True, privilege_withdrawn=False, aa_compromise=F"
- "alse)>"
- )
-
-
class TestDistributionPoint(object):
- def test_distribution_point_list_not_general_names(self):
+ def test_distribution_point_full_name_not_general_names(self):
with pytest.raises(TypeError):
- x509.DistributionPoint(["notgn"], None, None)
+ x509.DistributionPoint(["notgn"], None, None, None)
- def test_distribution_point_not_name(self):
+ def test_distribution_point_relative_name_not_name(self):
with pytest.raises(TypeError):
- x509.DistributionPoint("notname", None, None)
+ x509.DistributionPoint(None, "notname", None, None)
+
+ def test_distribution_point_full_and_relative_not_none(self):
+ with pytest.raises(ValueError):
+ x509.DistributionPoint("data", "notname", None, None)
def test_crl_issuer_not_general_names(self):
with pytest.raises(TypeError):
- x509.DistributionPoint(None, None, ["notgn"])
+ x509.DistributionPoint(None, None, None, ["notgn"])
def test_reason_not_reasonflags(self):
with pytest.raises(TypeError):
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
- "notreasonflags",
+ None,
+ ["notreasonflags"],
+ None
+ )
+
+ def test_disallowed_reasons(self):
+ with pytest.raises(ValueError):
+ x509.DistributionPoint(
+ [x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
+ None,
+ [x509.ReasonFlags.unspecified],
+ None
+ )
+
+ with pytest.raises(ValueError):
+ x509.DistributionPoint(
+ [x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
+ None,
+ [x509.ReasonFlags.remove_from_crl],
None
)
@@ -1390,18 +1367,16 @@
with pytest.raises(ValueError):
x509.DistributionPoint(
None,
- x509.ReasonFlags(
- True, True, False, False, True, True, False, False
- ),
+ None,
+ [x509.ReasonFlags.aa_compromise],
None
)
def test_eq(self):
dp = x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
- x509.ReasonFlags(
- False, False, False, False, False, True, False, False
- ),
+ None,
+ [x509.ReasonFlags.superseded],
[
x509.DirectoryName(
x509.Name([
@@ -1414,9 +1389,8 @@
)
dp2 = x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
- x509.ReasonFlags(
- False, False, False, False, False, True, False, False
- ),
+ None,
+ [x509.ReasonFlags.superseded],
[
x509.DirectoryName(
x509.Name([
@@ -1432,9 +1406,10 @@
def test_ne(self):
dp = x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
- x509.ReasonFlags(
- False, False, False, False, False, True, False, False
- ),
+ None,
+ [
+ x509.ReasonFlags.superseded,
+ ],
[
x509.DirectoryName(
x509.Name([
@@ -1448,6 +1423,7 @@
dp2 = x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://crypt.og/crl")],
None,
+ None,
None
)
assert dp != dp2
@@ -1455,12 +1431,14 @@
def test_repr(self):
dp = x509.DistributionPoint(
+ None,
x509.Name([
x509.NameAttribute(x509.OID_COMMON_NAME, "myCN")
]),
- x509.ReasonFlags(
- False, False, False, False, False, True, False, False
- ),
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
[
x509.DirectoryName(
x509.Name([
@@ -1472,14 +1450,12 @@
],
)
assert repr(dp) == (
- "<DistributionPoint(distribution_point=<Name([<NameAttribute(oid="
- "<ObjectIdentifier(oid=2.5.4.3, name=commonName)>, value='myCN')>"
- "])>, reasons=<ReasonFlags(key_compromise=False, ca_compromise=Fa"
- "lse, affiliation_changed=False,superseded=False, cessation_of_op"
- "eration=False, certificate_hold=True, privilege_withdrawn=False,"
- " aa_compromise=False)>, crl_issuer=[<DirectoryName(value=<Name(["
- "<NameAttribute(oid=<ObjectIdentifier(oid=2.5.4.3, name=commonNam"
- "e)>, value='Important CA')>])>)>])>"
+ "<DistributionPoint(full_name=None, relative_name=<Name([<NameAtt"
+ "ribute(oid=<ObjectIdentifier(oid=2.5.4.3, name=commonName)>, val"
+ "ue='myCN')>])>, reasons=[<ReasonFlags.key_compromise: 'keyCompro"
+ "mise'>, <ReasonFlags.ca_compromise: 'cACompromise'>], crl_issuer"
+ "=[<DirectoryName(value=<Name([<NameAttribute(oid=<ObjectIdentifi"
+ "er(oid=2.5.4.3, name=commonName)>, value='Important CA')>])>)>])>"
)
@@ -1493,13 +1469,16 @@
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://domain")],
None,
+ None,
None
),
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
None
),
])
@@ -1508,13 +1487,16 @@
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"http://domain")],
None,
+ None,
None
),
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
None
),
]
@@ -1523,37 +1505,41 @@
cdp = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
None
),
])
assert repr(cdp) == (
- "<CRLDistributionPoints([<DistributionPoint(distribution_point=[<"
- "UniformResourceIdentifier(value=ftp://domain)>], reasons=<Reason"
- "Flags(key_compromise=True, ca_compromise=True, affiliation_chang"
- "ed=True,superseded=True, cessation_of_operation=True, certificat"
- "e_hold=True, privilege_withdrawn=True, aa_compromise=True)>, crl"
- "_issuer=None)>])>"
+ "<CRLDistributionPoints([<DistributionPoint(full_name=[<UniformRes"
+ "ourceIdentifier(value=ftp://domain)>], relative_name=None, reason"
+ "s=[<ReasonFlags.key_compromise: 'keyCompromise'>, <ReasonFlags.ca"
+ "_compromise: 'cACompromise'>], crl_issuer=None)>])>"
)
def test_eq(self):
cdp = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
[x509.UniformResourceIdentifier(u"uri://thing")],
),
])
cdp2 = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
[x509.UniformResourceIdentifier(u"uri://thing")],
),
])
@@ -1563,36 +1549,43 @@
cdp = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
[x509.UniformResourceIdentifier(u"uri://thing")],
),
])
cdp2 = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain2")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
[x509.UniformResourceIdentifier(u"uri://thing")],
),
])
cdp3 = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, False
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ ],
[x509.UniformResourceIdentifier(u"uri://thing")],
),
])
cdp4 = x509.CRLDistributionPoints([
x509.DistributionPoint(
[x509.UniformResourceIdentifier(u"ftp://domain")],
- x509.ReasonFlags(
- True, True, True, True, True, True, True, True
- ),
+ None,
+ [
+ x509.ReasonFlags.key_compromise,
+ x509.ReasonFlags.ca_compromise,
+ ],
[x509.UniformResourceIdentifier(u"uri://thing2")],
),
])