Cert: Fix role certificate parameter (#886)

* Cert: Fix role certificate parameter

The `create_ca_certificate_role` currently accepts a `certificate` parameter that can either be a PEM-formatted certificate or a path to a PEM-fromatted certificate file. The parameter is differentiated by trying to open a file with what's passed to `certificate`. For some systems, the lengh of a certificate is long enough that it will cause a OSError because of the path length.

The conditional was updated to actually inspect what is passed to contents. To avoid these issues moving forward, `certificate` will only accept a certificate string. Paths should be passed to the new `certificate_file` parameter. For now, passing a certificate file to `certificate` will generate a warning.

Updated tests to use a RSA 4096 key as the certificate file's length would cause the OSError regression solved by this fix. Tests additionally test the certificate and certificate_file parameters.

Signed-off-by: Colin McAllister <colinmca242@gmail.com>

* Update hvac/api/auth_methods/cert.py

Signed-off-by: Colin McAllister <colinmca242@gmail.com>
Co-authored-by: Brian Scholer <1260690+briantist@users.noreply.github.com>

hvac/api/auth_methods/cert.py

@@ -1,9 +1,11 @@
 #!/usr/bin/env python
 """Cert methods module."""
+import os
+import warnings
+
 from hvac.api.vault_api_base import VaultApiBase
 from hvac.utils import validate_pem_format
 from hvac import exceptions, utils
-import os
 
 
 class Cert(VaultApiBase):
@@ -15,7 +17,8 @@ class Cert(VaultApiBase):
     def create_ca_certificate_role(
         self,
         name,
-        certificate,
+        certificate="",
+        certificate_file="",
         allowed_common_names="",
         allowed_dns_sans="",
         allowed_email_sans="",
@@ -44,8 +47,13 @@ def create_ca_certificate_role(
         »Parameters
         :param name: The name of the certificate role.
         :type name: str
-        :param certificate: The PEM-format CA certificate.
+        :param certificate: The PEM-format CA certificate. Either certificate or certificate_file is required.
+            NOTE: Passing a certificate file path with the certificate argument is deprecated and will be dropped in
+            version 3.0.0
         :type certificate: str
+        :param certificate_file: File path to the PEM-format CA certificate.  Either certificate_file or certificate is
+            required.
+        :type certificate_file: str
         :param allowed_common_names: Constrain the Common Names in the client certificate with a globbed pattern. Value
             is a comma-separated list of patterns. Authentication requires at least one Name matching at least one
             pattern. If not set, defaults to allowing all names.
@@ -104,11 +112,24 @@ def create_ca_certificate_role(
         :param mount_point:
         :type mount_point:
         """
-        try:
-            with open(certificate) as f_cert:
+        if certificate:
+            try:
+                utils.validate_pem_format("", certificate)
+                cert = certificate
+            except exceptions.ParamValidationError:
+                with open(certificate) as f_cert:
+                    warnings.warn(
+                        "Passing a certificate file path to `certificate` is deprecated and will be removed in v3.0.0;"
+                        "use `certificate_file` instead. (See https://github.com/hvac/hvac/issues/914)"
+                    )
+                    cert = f_cert.read()
+        elif certificate_file:
+            with open(certificate_file) as f_cert:
                 cert = f_cert.read()
-        except FileNotFoundError:
-            cert = certificate
+        else:
+            raise exceptions.ParamValidationError(
+                "`certificate` or `certificate_file` must be provided"
+            )
 
         params = utils.remove_nones(
             {

tests/config_files/client-cert.pem

@@ -1,17 +1,31 @@
 -----BEGIN CERTIFICATE-----
-MIICtTCCAZ2gAwIBAgIJAMnxx3he29bBMA0GCSqGSIb3DQEBCwUAMBExDzANBgNV
-BAMMBmNsaWVudDAeFw0xNTA1MTcyMjQ0MTNaFw0yNTA1MTQyMjQ0MTNaMBExDzAN
-BgNVBAMMBmNsaWVudDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAM/K
-F7QA9SeohbyadyJLg3osWEkATjyUKSuhuS/jDftcvfXjpZswj9dbP6kBIJVSCIAd
-jA9EhNLrkuh+NG5ASOFAsKU34Eh/6A1ZH2XdoFxDJatwMEUPkAYUoxba0UHE/UN6
-nAHCmsaebiUHOJSSDpOc24u2FBGYU9LdBtvUPjexZMsgkGVjCr9odT7Ero+SKfXg
-yjmg9cuFrjWaSpSBFEUHO0t5hkeuGI4sgIgel/zQYNsV+FfnBNwgLcwWAAc7h/m1
-qBOoox4O8c49lHih6TifygOAkFzZx5gBFXUDVxuiBVRJCPphQ7h4tGJnbJwQWa+w
-JDA+19KSTAhx7K2Qr0sCAwEAAaMQMA4wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0B
-AQsFAAOCAQEAUlGtVgQFQFaZQbcH438SqTr5frHxHpEsYfw1R3C3TV14t/UetILb
-dRqTHHNw7gW2acWTXzvyygAaJq1gawymqHS+wSifcMRuugc8TOzeTEMose6KmgVo
-pla0A/MzrQ5Tk2hSRSXzYQO7tPW8k3XFQqAul4gOE4WWbOQFW0S3L+nZF9xr9npg
-T55qVLbG2UHZFsRcbdZgRZzUMcIxar//2jGYBFCTB3hPR3HJD0GaHjrGJxuYpU/T
-3BA+4xxfj3ZJiPPYpTWB53Cx7h8kUpNVc8rs7lUY9u7KKmhQIEo5NxKeacRtCUYi
-7R9iymoHRHTPhiyb4kqRfsVBEezMPWTK2w==
+MIIFVDCCAzwCCQCzkOYQ7N7fdDANBgkqhkiG9w0BAQsFADBsMQswCQYDVQQGEwJV
+UzERMA8GA1UECAwITWlzc291cmkxFDASBgNVBAcMC0thbnNhcyBDaXR5MQ0wCwYD
+VQQKDARIVkFDMRQwEgYDVQQLDAtNYWludGFpbmVyczEPMA0GA1UEAwwGY2xpZW50
+MB4XDTIyMTExOTE5NDAyMloXDTI2MTExODE5NDAyMlowbDELMAkGA1UEBhMCVVMx
+ETAPBgNVBAgMCE1pc3NvdXJpMRQwEgYDVQQHDAtLYW5zYXMgQ2l0eTENMAsGA1UE
+CgwESFZBQzEUMBIGA1UECwwLTWFpbnRhaW5lcnMxDzANBgNVBAMMBmNsaWVudDCC
+AiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBAJUCx4nFH98FQSTryV81Vx9M
+Uvxhpygnw5QOsdK06VLOra8vstut5dZQXTQA0I44xCdcG37K6t3pIbVZT2j1sv2C
+PCaz/X7fwAMXUQJJhydywx4Y902jKo8zYFhoOx+ZdB2jJKkTs5hrL0kDIdNj3x1h
+phx+WZsPryqrpa0B4mUNYbvzP7JJSOEyyIIc+qwZ2EYF36z+adKA/HOP7g56bfpi
+nTWUJGBMfeNNe6Y4KUVZZsM6T6S1pXsMlBmg26gHkKrmiFJNgewbtb5m7HcILtdg
+DWLN5HNvGmodO6N0ebGJ25g8pYfaXFJu/PWMSR549IOcc8JG8ikcs3II2HJoOb+R
+v7bcc9qX+0NHfgXiUCqu18AN08V3JEIOFzfg3JeT5Qqv+l1b8eoQR5HAaJQgVfLW
+ibKCaMWLoWdDqxWo7LXEB+wT/k4D0afYDmL1jFOp8A193zivdr0pzXo6p2TIU7vJ
+um49O7Gta2fk2FPMS9WrhFmfvK/fkxoPbUEviMwHolYGZF8UKbDVvE6x8Pq5urnM
+LLOmzNGvJMCOsiVfqY6cDoINdu7CXQB+mmDO3KR1jDr6tUEV0NiWqEjFGxHcxy8L
+DFOqHzZlGMGCX3YhE1rroCEthTdj7IT9dzD+62ExoWGNdiCFugY5HJ1CJbYDdxNk
+2Xd9u0YK2FuW23BWmgC5AgMBAAEwDQYJKoZIhvcNAQELBQADggIBACMDPV6iuHjw
+ipd+Vc228UR60jRwQ7RwFif90LapYtLT5wWRXe2c/rXdXV6k9jE33UmB6Kv1Q5mb
+aqPwv2p8fWLvzl20TP+UDMKSGyXoY9hteuMkr8/MA0MRfuQ4r//ZAphwW2lj3vzZ
+6XvwmQfNF7FF0OwsRKCa1e5LsqnEFNTyQLPUya1FjpnArylMEz2bMrfzEY8O8/9U
+B+InkzUZxkJ7S7Xm6oKsjqTl/6HHhYSumSnAccqJ7Qi+tMrWDqMLLj6o2RV1vSi6
+3eYMctp3r3rDGB2TERtJ3R5pmW9VLhvlERh99r1VMjSX2nW1oZCA+7iBlHD0CuNg
+6LMcrA+Cocul+b9vO3ibzF/eh1K+ROfPir+FtJUYwhvZ/nW/1RYysI9aZptwHXHX
+163BLSXRqvCIZELI9qu0bdHdUd6cRy4Ff5ugLWV8mU3wWM+RrFUPSWttRN+gnXmr
+BnMIfHB+3KbvzbrQiPYwGuvM5opV4afMdphzOPkFPXV6Lo/hAwZHzFzyY/UsQkgg
+flNX0yiEtNXHLG8IMFlpDUvTpnNxnrFeY5INk5tc54d1oRhUKJofRa01OA4Y5feE
+iVvCnouQh0PHKZTBju4S479QntCFzjPRBfdH7JfFR69MY2iGzXwwCqEIQI6XUiUa
+WMI4R7772R4OTMUBoclmQDDxRAzdPK3y
 -----END CERTIFICATE-----

tests/config_files/client-key.pem

@@ -1,28 +1,52 @@
 -----BEGIN PRIVATE KEY-----
-MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDPyhe0APUnqIW8
-mnciS4N6LFhJAE48lCkrobkv4w37XL3146WbMI/XWz+pASCVUgiAHYwPRITS65Lo
-fjRuQEjhQLClN+BIf+gNWR9l3aBcQyWrcDBFD5AGFKMW2tFBxP1DepwBwprGnm4l
-BziUkg6TnNuLthQRmFPS3Qbb1D43sWTLIJBlYwq/aHU+xK6Pkin14Mo5oPXLha41
-mkqUgRRFBztLeYZHrhiOLICIHpf80GDbFfhX5wTcIC3MFgAHO4f5tagTqKMeDvHO
-PZR4oek4n8oDgJBc2ceYARV1A1cbogVUSQj6YUO4eLRiZ2ycEFmvsCQwPtfSkkwI
-ceytkK9LAgMBAAECggEBAMu7KUxrV/N/b0kB5mcwxQOH8TP6YbtRxaArHAiQxge7
-4eDySFCF57LOm/dXzJu7Z+w9jDL4CodHRK5B96RV0V9R7yC/TV+oUVytEnTtPSrZ
-B1k1UY6U+LCNkW56M4wZ2fxMfiPXq6uscPBLSTWak0I2HKi6aKjrwtxPrEbjrfEp
-b/4XvAam2XUzhOiyioG466RA55/EuG9h9rb0vVNwuDrtaxnnTLdquUZRbSWcpEsJ
-rnEFLCJFxhqIOaM+WB46KItozaHkmZIue41vNgaIAcPwLMHR1rjK8wwM5TwycHiu
-xawMwJYz6rolVfLeoTTe8a4/cwUxLEevNujX2K2jt1ECgYEA60DSVTEjMWv98gNg
-j85DvvaBPgQ7Uf9DmG/4Y7erly6D/Ks/WGnj8dt01Gn/lmHmHa0EO0VyCPHLY1Nm
-XCBN98CunX7aGuTWJv3y184+b6f0bvsUs4w79gfSJaCOhonTE1ua4Od0+PBvi2VJ
-KyT2LoEATm7V3ydt8ybvpKlosmkCgYEA4h08le0emhyKX0uhz8RpgfoBLdtVVBn8
-A5fj6mRJLGOwLA8k7hVolpKckTvEVK1E/PmeEgd3KWteJAOUlmHPNtLQBKNrTgKZ
-2PojCNlDozTEQsQwXwymEvf0clJP7ZzJja3Jj1jujy2w+lPJr+A5Nrw2ybmqFHxC
-oJGcWLgetZMCgYEAqtg2Z5v5vOh6UYlWVNkspcAK9+jG07OXeVrHflNij7Y+L/6F
-UEYGPhxr6d7YuHN8VEzT5991Az3lwMpOadUkCiqqJMJ8zk8lnseJ1mN3QEsu3nIF
-BKdw3Cmt2ZmSJ+5rlYFPJsmwTyD9QLRbwLNk/Ty95jfjpd3SoWTDYEePOxECgYEA
-nTUQk3iKB0ICnqDZ/rcjV//24+gT2mY6hlELYEKbcWZZ7TgWSAXQcqe6fWtdZDYX
-uf7e/M/99Gk2yblHjIqyY/6MLD1mdJVwW6AFzbgow4cDURxQOTV1HckRRo6eDakt
-pPiII8cEDb4JxvGMT2CIxtpzCSux6Y4/UJ4MYs6YY0sCgYAG99K/2WSaC3oROmp6
-7z5JJrtFqIU2j3nMvuDy14rgxnmiaP/YMM2oQdhYsLElfSHEmRdSB//kmPe8njoU
-JzMcmdOZ3BCLpiV6F2f8I9tsZDOli362BVzpvabU5tfH4SUNWLOquFyJG7jHBMON
-3z2xcVp6G6r+3yVLG8KCZDOM/A==
+MIIJQQIBADANBgkqhkiG9w0BAQEFAASCCSswggknAgEAAoICAQCVAseJxR/fBUEk
+68lfNVcfTFL8YacoJ8OUDrHStOlSzq2vL7LbreXWUF00ANCOOMQnXBt+yurd6SG1
+WU9o9bL9gjwms/1+38ADF1ECSYcncsMeGPdNoyqPM2BYaDsfmXQdoySpE7OYay9J
+AyHTY98dYaYcflmbD68qq6WtAeJlDWG78z+ySUjhMsiCHPqsGdhGBd+s/mnSgPxz
+j+4Oem36Yp01lCRgTH3jTXumOClFWWbDOk+ktaV7DJQZoNuoB5Cq5ohSTYHsG7W+
+Zux3CC7XYA1izeRzbxpqHTujdHmxiduYPKWH2lxSbvz1jEkeePSDnHPCRvIpHLNy
+CNhyaDm/kb+23HPal/tDR34F4lAqrtfADdPFdyRCDhc34NyXk+UKr/pdW/HqEEeR
+wGiUIFXy1omygmjFi6FnQ6sVqOy1xAfsE/5OA9Gn2A5i9YxTqfANfd84r3a9Kc16
+OqdkyFO7ybpuPTuxrWtn5NhTzEvVq4RZn7yv35MaD21BL4jMB6JWBmRfFCmw1bxO
+sfD6ubq5zCyzpszRryTAjrIlX6mOnA6CDXbuwl0AfppgztykdYw6+rVBFdDYlqhI
+xRsR3McvCwxTqh82ZRjBgl92IRNa66AhLYU3Y+yE/Xcw/uthMaFhjXYghboGORyd
+QiW2A3cTZNl3fbtGCthblttwVpoAuQIDAQABAoICADnmR5RPxS+KQiLPf4KAHmcp
+6ADrDOF8efE4uH3QD+0+I5yzowLbEjhxJ5YCTwdEJWvJzWE6BYLOiGuH1WRKPA9n
+IGpFe9VlqZ1J3Q9PdLWuUQSL3OQM54eXBfO0JG0F2ml9r8wGvL0XwNrU3brezp4D
+CGF/8t9ee8jPw2NzHP348iXuwaxL74aZjfGIwxpGPJQQyjXU8Vd7CRz6KxzzE3K8
+CKv98Rh+AFhnDgmrPtYUg+qN6M5uSmuCmrVOnKswrVTKmOOp/Da7GaZvJaimxFSp
+nOnsaL01yYS78SV1J6gFkg8YWZVtEnxxZdcfeHXft9q/2vb9TDEPVuaTa08hEna2
+pzBksFa6RCqbNhGs9BxnVyADT+uby56kzeUUsL4BoaNAd+9eJaQSczpZuJKPru7m
+B0L+lAbrgXe8ogCr+On42uMR+1JIufnoytIdOebK0WQ+FFh3q5Z3nkpnL8LHlcmn
+9MBKZSTn52NTZEgKudx/9tnruqM8UzJE33idP0tJ/BXuhnGYIZ6beiBizNSpnXyp
+yD3F2sVmQMIBhZRQTE5WjPzv0EK4CL7+rxeLV+7IauA8caSEJblr9BpWozjLVBRZ
++uq9ePl6Aj/3/R0JAWqjhwtyWsbVmYScZjchX+iQuINPH+TBxBNb7i82WHaDV+F7
+OSBA2oghxWrta8VM/iuBAoIBAQDFoss/u03MLJnOjpi9t0BM8HTuIAXQfgVesjsz
+8LYrp/kEv0xwfMQ5lXM6bTZflK2/nCf+Qkv5XhxKT3onyE8xEXmzDGXSZWU5kt1s
+dV2oAedosLEr4yRet6kWnmrgZ+pzt/dVPz8cLDB7hr/L8COX1F+kr9ZhPeHLqBDm
+TWwEkFppFArxzFiX0dDL6YmuqMz8s86765VjC/WCRATRfkZpeLJqQ1t7qj35KW8T
+WhJw5Z4FT1lYYBoRLBZQGUyPpoN3/Ef9ELim5yYjiuNRUUylQt/LADBhKMCw+mjB
+zmt1e6YqeHZ8Pti27LZ9Xgp3RYwJeqxHvYoVUvg/EqoVQn5xAoIBAQDBA/K9so3g
+MiNKgO9PJNHbzQ/ENdhxzvUX5nNtBCQ3O9LYtWxH3WU2Qbfu0MTNx1iaVEhb40P2
+m6/DukYLRlcYQct9a9PI8wjbVguAYjUkroM4HC1WmUunikp8Y6zBcUyZOuO6haNS
+8gmC9TJWF1zt/C3y6/duhnQW6pS378fyglijkrKL3m2SyUSY3CYNpvK4CiUGTIny
+TT9HWFLm7lL++EFehv/5+BCmIfm9hqFqdgztA9vMk+yOss/rhrzSMMnVXA8rqDZb
+ThZbSncGGZ7/pP51Od6A4FCzM2jzSIzlUuE4Rgl72I8X1LBi96eRjs1nv973HJjc
+ZzEcD/ZugVrJAoIBABMnxCtZLCEt8Xpjt8oriL/sTmkGEYozrLGQ9MwD+KQik7Ay
+GmaMsTgjlBeFnk+FSZHYwMeUBWkWBgftzBkvAP3wg6sm+Hd7/GOoKG3qsjkmdhdf
+iMyIJ25GvszCbflyGPc+TWqlxqgkYE6YsN5DWC/PFbYQlOa36Lqkf37S4jVPCHRG
+zQjClZOhcmtRJ1cWZUfY5aCxJLmE3zbRAOaNJqXyXNiPzr4dsgQEUA/AGyl2F+rL
+1g0Kw3wGmeMJkLIboYJvoUqV5E/I95laTM4E4zjhUm+KhSXlnUeJ/b1LFH8+jMxx
+AIrqTHbuWoGsK7eDth71FN2fMVL/x6/VT0bgOwECggEAec7TDxIy5QzfJrKN0+WJ
+puwXihztFo2kMUwXBfJ6JH1Vh7uetQ+sQvxgZo9L1A19BhAwhgS7rFL+LkhM0eq3
+JjDHicRVqFhv+3Om2IOhxB954hwuJJujmfz1GuEQchfdXkpC00BpHPxwMjwpYxll
+ZS8jlE2EHjUt6BRsfFQPXCUUaU437XsWrW7lIo60WSNFxA1q5jnBSK+lQpIj52b5
+67yiDFSoEImTgDSq1gezsGsFp0IrIUUtIbUI8DtcyE1P4p0xWbzTaPWzuDMbQDdF
+F1J532MA31Ywt5a3IelkOYj/ZzMT4xt82m44Toy6LGrxiAsXW92pPlbFME+GlqB9
+uQKCAQAjLbha9VhxXzmFI4nfePNXZXJpTxfQczCxIaddwKBMKgM7x+Bqr28E2cos
+iq/RvA4xvkrGQhtM1A822U6BjBYJMPIZJDBPtKrZxRpiklOf2mrKaKb6q5OPA/IP
+wOwn+ByXp+hZr7te0VdvWZfburMBiim8sJ4M2wz+SGKMcs6cJ8/Y9L5KpDW2cjdu
+c/vDpHS+Sgfjy5k3mYDCNHzKFcldM8m4bVwNwGbMycj6P4LT00m5gQpYb3vJ45Sj
+UH6otU9878bMJrMmmCa0vVdnimAKcqai/j1mkBOgFajNSeSsyFcV7Hn0gRhyOHqg
+mfb+SR45Dwco8Uqfxuua2KsK5RsZ
 -----END PRIVATE KEY-----

tests/integration_tests/api/auth_methods/test_cert.py

@@ -9,8 +9,9 @@
 class TestCert(HvacIntegrationTestCase, TestCase):
     TEST_MOUNT_POINT = "cert-test"
     TEST_ROLE_NAME = "testrole"
+    TEST_CLIENT_CERTIFICATE_FILE = utils.get_config_file_path("client-cert.pem")
     cert = utils.create_client()._adapter._kwargs.get("cert")
-    with open(utils.get_config_file_path("client-cert.pem")) as fp:
+    with open(TEST_CLIENT_CERTIFICATE_FILE, "r") as fp:
         TEST_CERTIFICATE = fp.read()
 
     def setUp(self):
@@ -38,6 +39,25 @@ def test_create_ca_certificate_role(self):
 
         self.assertEqual(first=204, second=response.status_code)
 
+    def test_create_ca_certificate_with_filename(self):
+        response = self.client.auth.cert.create_ca_certificate_role(
+            name="testrole2",
+            certificate_file=self.TEST_CLIENT_CERTIFICATE_FILE,
+            mount_point=self.TEST_MOUNT_POINT,
+        )
+
+        self.assertEqual(first=204, second=response.status_code)
+
+    def test_create_ca_certificate_with_filename_deprecated(self):
+        """This tests the deprecated feature of passing a certificate file via the certificate argument"""
+        response = self.client.auth.cert.create_ca_certificate_role(
+            name="testrole2",
+            certificate=self.TEST_CLIENT_CERTIFICATE_FILE,
+            mount_point=self.TEST_MOUNT_POINT,
+        )
+
+        self.assertEqual(first=204, second=response.status_code)
+
     def test_read_ca_certificate_role(self):
         response = self.client.auth.cert.read_ca_certificate_role(
             name=self.TEST_ROLE_NAME,
@@ -96,14 +116,6 @@ def test_login(self, name, cacert, cert_pem, key_pem, mount_point):
                     cert_pem=cert_pem,
                     mount_point=mount_point,
                 )
-        # elif cacert:
-        #     with self.assertRaises(OSError):
-        #         self.client.auth.cert.login(
-        #             name=name,
-        #             cacert=cacert,
-        #             cert_pem=cert_pem,
-        #             mount_point=mount_point,
-        #         )
         elif (
             name != ""
             and name