test exceptions and properly reject duplicate attributes in csrbuilder (#5319)

src/cryptography/x509/base.py

@@ -36,6 +36,13 @@ def _reject_duplicate_extension(extension, extensions):
             raise ValueError('This extension has already been set.')
 
 
+def _reject_duplicate_attribute(oid, attributes):
+    # This is quadratic in the number of attributes
+    for attr_oid, _ in attributes:
+        if attr_oid == oid:
+            raise ValueError('This attribute has already been set.')
+
+
 def _convert_to_naive_utc_time(time):
     """Normalizes a datetime to a naive datetime in UTC.
 
@@ -448,6 +455,8 @@ class CertificateSigningRequestBuilder(object):
         if not isinstance(value, bytes):
             raise TypeError("value must be bytes")
 
+        _reject_duplicate_attribute(oid, self._attributes)
+
         return CertificateSigningRequestBuilder(
             self._subject_name, self._extensions,
             self._attributes + [(oid, value)]

tests/x509/test_x509.py

@@ -3653,6 +3653,27 @@ class TestCertificateSigningRequestBuilder(object):
             x509.oid.NameOID.LOCALITY_NAME
         ) == locality
 
+    def test_add_attribute_bad_types(self, backend):
+        request = x509.CertificateSigningRequestBuilder()
+        with pytest.raises(TypeError):
+            request.add_attribute(
+                b"not an oid", b"val"
+            )
+
+        with pytest.raises(TypeError):
+            request.add_attribute(
+                x509.oid.AttributeOID.CHALLENGE_PASSWORD, 383
+            )
+
+    def test_duplicate_attribute(self, backend):
+        request = x509.CertificateSigningRequestBuilder().add_attribute(
+            x509.oid.AttributeOID.CHALLENGE_PASSWORD, b"val"
+        )
+        with pytest.raises(ValueError):
+            request.add_attribute(
+                x509.oid.AttributeOID.CHALLENGE_PASSWORD, b"val2"
+            )
+
     def test_set_subject_twice(self):
         builder = x509.CertificateSigningRequestBuilder()
         builder = builder.subject_name(