Fix several x509_v2 issues

changelog/66929.fixed.md

@@ -0,0 +1 @@
+Fixed x509_v2.certificate_managed state fails if another state.apply is queued

changelog/66942.fixed.md

@@ -0,0 +1 @@
+Fixed x509_v2 private_key_managed failing on Windows due to default `mode` argument

changelog/68828.fixed.md

@@ -0,0 +1 @@
+Made x509_v2 certificate_managed respect `copypath` and `prepend_cn` parameters

salt/modules/x509_v2.py

@@ -118,6 +118,14 @@
 
 Breaking changes versus the previous ``x509`` modules
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* The ``public_key`` parameter to ``x509.certificate_managed`` (and corresponding
+  ``x509.create_certificate``) used to accept a private key.
+  The new modules require an actual public key if this parameter is specified.
+  You can pass a private key in the ``private_key`` parameter instead.
+
+  Failing to ensure it really is a public key you are passing as ``public_key`` fails
+  with ``Could not load PEM-encoded public key.``.
+
 * The output format has changed for all ``read_*`` functions as well as the state return dict.
 * The formatting of some extension definitions might have changed, but should
   be stable for most basic use cases.
@@ -267,33 +275,48 @@ def create_certificate(
         The hashing algorithm to use for the signature. Valid values are:
         sha1, sha224, sha256, sha384, sha512, sha512_224, sha512_256, sha3_224,
         sha3_256, sha3_384, sha3_512. Defaults to ``sha256``.
-        This will be ignored for ``ed25519`` and ``ed448`` key types.
+        Ignored for ``ed25519`` and ``ed448`` key types.
 
     private_key
-        The private key corresponding to the public key the certificate should
-        be issued for. This is one way of specifying the public key that will
-        be included in the certificate, the other ones being ``public_key`` and ``csr``.
+        A **private key**, which is used to derive the public key the certificate
+        is issued for. If unset, checks ``public_key`` or ``csr`` to derive it.
+
+        Ignored when creating self-signed certificates (missing ``signing_cert``).
+
+        .. hint::
+            When ``encoding`` is ``pkcs12``, this private key is embedded into
+            the resulting container.
 
     private_key_passphrase
         If ``private_key`` is specified and encrypted, the passphrase to decrypt it.
 
     public_key
-        The public key the certificate should be issued for. Other ways of passing
-        the required information are ``private_key`` and ``csr``. If neither are set,
-        the public key of the ``signing_private_key`` will be included, i.e.
-        a self-signed certificate is generated.
+        A **public key**, which is used as the public key the certificate is issued for,
+        but only if ``private_key`` is **not** specified.
+
+        If this is unset, checks ``csr`` to derive it.
+
+        Ignored when creating self-signed certificates (missing ``signing_cert``).
 
     csr
-        A certificate signing request to use as a base for generating the certificate.
-        The following information will be respected, depending on configuration:
-        * public key
-        * extensions, if not otherwise specified (arguments, signing_policy)
+        A **certificate signing request** to use as a base for generating the certificate:
+
+        - Extensions not otherwise specified (arguments, signing_policy) are copied.
+        - If ``private_key`` and ``public_key`` are both unspecified, copies the embedded
+          public key into the certificate. This step is skipped when creating self-signed
+          certificates (missing ``signing_cert``).
 
     signing_cert
         The CA certificate to be used for signing the issued certificate.
 
+        Leave empty to create a self-signed certificate.
+
     signing_private_key
-        The private key corresponding to the public key in ``signing_cert``. Required.
+        The private key to be used for signing the new certificate. Required.
+
+        Usually, this is the private key corresponding to the public key in ``signing_cert``.
+        When creating self-signed certificates (missing ``signing_cert``), derives
+        the new certificate's embedded public key from this private key.
 
     signing_private_key_passphrase
         If ``signing_private_key`` is encrypted, the passphrase to decrypt it.
@@ -385,10 +408,9 @@ def create_certificate(
 
             .. code-block:: yaml
 
-                # mind this being a list, not a dict
                 - subjectAltName:
-                    - email:me@example.com
-                    - DNS:example.com
+                    - email:me@example.com  # list items can be strings
+                    - dns: example.com      # or single-key dicts
 
         issuerAltName
             The syntax is the same as for ``subjectAltName``, except that the additional
@@ -846,7 +868,7 @@ def create_crl(
         The hashing algorithm to use for the signature. Valid values are:
         sha1, sha224, sha256, sha384, sha512, sha512_224, sha512_256, sha3_224,
         sha3_256, sha3_384, sha3_512. Defaults to ``sha256``.
-        This will be ignored for ``ed25519`` and ``ed448`` key types.
+        Ignored for ``ed25519`` and ``ed448`` key types.
 
     encoding
         Specify the encoding of the resulting certificate revocation list.
@@ -1059,7 +1081,7 @@ def create_csr(
         The hashing algorithm to use for the signature. Valid values are:
         sha1, sha224, sha256, sha384, sha512, sha512_224, sha512_256, sha3_224,
         sha3_256, sha3_384, sha3_512. Defaults to ``sha256``.
-        This will be ignored for ``ed25519`` and ``ed448`` key types.
+        Ignored for ``ed25519`` and ``ed448`` key types.
 
     encoding
         Specify the encoding of the resulting certificate signing request.

salt/states/x509_v2.py

@@ -188,6 +188,7 @@
 from datetime import datetime, timedelta, timezone
 
 import salt.utils.files
+import salt.utils.platform
 from salt.exceptions import CommandExecutionError, SaltInvocationError
 from salt.state import STATE_INTERNAL_KEYWORDS as _STATE_INTERNAL_KEYWORDS
 
@@ -227,8 +228,6 @@ def certificate_managed(
     signing_policy=None,
     encoding="pem",
     append_certs=None,
-    copypath=None,
-    prepend_cn=False,
     digest="sha256",
     signing_private_key=None,
     signing_private_key_passphrase=None,
@@ -251,7 +250,7 @@ def certificate_managed(
     Ensure an X.509 certificate is present as specified.
 
     This function accepts the same arguments as :py:func:`x509.create_certificate <salt.modules.x509_v2.create_certificate>`,
-    as well as most ones for `:py:func:`file.managed <salt.states.file.managed>`.
+    as well as most ones for :py:func:`file.managed <salt.states.file.managed>`.
 
     name
         The path the certificate should be present at.
@@ -297,37 +296,49 @@ def certificate_managed(
         The hashing algorithm to use for the signature. Valid values are:
         sha1, sha224, sha256, sha384, sha512, sha512_224, sha512_256, sha3_224,
         sha3_256, sha3_384, sha3_512. Defaults to ``sha256``.
-        This will be ignored for ``ed25519`` and ``ed448`` key types.
+        Ignored for ``ed25519`` and ``ed448`` key types.
+
+    signing_cert
+        The CA certificate to be used for signing the issued certificate.
+
+        Leave empty to create a self-signed certificate.
 
     signing_private_key
-        The private key corresponding to the public key in ``signing_cert``. Required.
+        The private key to be used for signing the new certificate. Required.
+
+        Usually, this is the private key corresponding to the public key in ``signing_cert``.
+        When creating self-signed certificates (missing ``signing_cert``), derives
+        the new certificate's embedded public key from this private key.
 
     signing_private_key_passphrase
         If ``signing_private_key`` is encrypted, the passphrase to decrypt it.
 
-    signing_cert
-        The CA certificate to be used for signing the issued certificate.
+    private_key
+        A **private key**, which is used to derive the public key the certificate
+        is issued for. If this is unset, checks ``public_key`` or ``csr`` to derive it.
 
-    public_key
-        The public key the certificate should be issued for. Other ways of passing
-        the required information are ``private_key`` and ``csr``. If neither are set,
-        the public key of the ``signing_private_key`` will be included, i.e.
-        a self-signed certificate is generated.
+        Ignored when creating self-signed certificates (missing ``signing_cert``).
 
-    private_key
-        The private key corresponding to the public key the certificate should
-        be issued for. This is one way of specifying the public key that will
-        be included in the certificate, the other ones being ``public_key`` and ``csr``.
+        .. hint::
+            When ``encoding`` is ``pkcs12``, this private key is embedded into
+            the resulting container.
 
     private_key_passphrase
         If ``private_key`` is specified and encrypted, the passphrase to decrypt it.
 
+    public_key
+        A **public key**, which is used as the public key the certificate is issued for,
+        but only if ``private_key`` is **not** specified. If this is unset, checks ``csr`` to derive it.
+
+        Ignored when creating self-signed certificates (missing ``signing_cert``).
+
     csr
-        A certificate signing request to use as a base for generating the certificate.
-        The following information will be respected, depending on configuration:
+        A **certificate signing request** to use as a base for generating the certificate:
 
-        * public key
-        * extensions, if not otherwise specified (arguments, signing_policy)
+        - Extensions not otherwise specified (arguments, signing_policy) are copied.
+        - If ``private_key`` and ``public_key`` are both unspecified, copies the embedded
+          public key into the certificate. This step is skipped when creating self-signed
+          certificates (missing ``signing_cert``).
 
     subject
         The subject's distinguished name embedded in the certificate. This is one way of
@@ -742,7 +753,7 @@ def crl_managed(
         The hashing algorithm to use for the signature. Valid values are:
         sha1, sha224, sha256, sha384, sha512, sha512_224, sha512_256, sha3_224,
         sha3_256, sha3_384, sha3_512. Defaults to ``sha256``.
-        This will be ignored for ``ed25519`` and ``ed448`` key types.
+        Ignored for ``ed25519`` and ``ed448`` key types.
 
     encoding
         Specify the encoding of the resulting certificate revocation list.
@@ -1047,7 +1058,7 @@ def csr_managed(
         The hashing algorithm to use for the signature. Valid values are:
         sha1, sha224, sha256, sha384, sha512, sha512_224, sha512_256, sha3_224,
         sha3_256, sha3_384, sha3_512. Defaults to ``sha256``.
-        This will be ignored for ``ed25519`` and ``ed448`` key types.
+        Ignored for ``ed25519`` and ``ed448`` key types.
 
     encoding
         Specify the encoding of the resulting certificate revocation list.
@@ -1359,7 +1370,7 @@ def private_key_managed(
     if extra_args:
         raise SaltInvocationError(f"Unrecognized keyword arguments: {list(extra_args)}")
 
-    if not file_args.get("mode"):
+    if not file_args.get("mode") and not salt.utils.platform.is_windows():
         # ensure secure defaults
         file_args["mode"] = "0400"
 
@@ -1591,7 +1602,13 @@ def _file_managed(name, test=None, **kwargs):
         raise SaltInvocationError("test param can only be None or True")
     # work around https://github.com/saltstack/salt/issues/62590
     test = test or __opts__["test"]
-    res = __salt__["state.single"]("file.managed", name, test=test, **kwargs)
+    res = __salt__["state.single"](
+        "file.managed", name, test=test, concurrent=True, **kwargs
+    )
+    if not isinstance(res, dict):
+        raise CommandExecutionError(
+            f"Failed running file.managed in x509_v2 state: {res}"
+        )
     return res[next(iter(res))]
 
 

tests/pytests/functional/states/test_x509_v2.py

@@ -28,6 +28,7 @@
     pytest.mark.slow_test,
     pytest.mark.skipif(HAS_LIBS is False, reason="Needs cryptography library"),
     pytest.mark.skip_on_fips_enabled_platform,
+    pytest.mark.windows_whitelisted,
 ]
 
 
@@ -1362,6 +1363,7 @@ def test_certificate_managed_extension_removed(x509, cert_args, rsa_privkey, ca_
     }
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.parametrize("mode", ["0400", "0640", "0644"])
 def test_certificate_managed_mode(x509, cert_args, rsa_privkey, ca_key, mode, modules):
     """
@@ -1388,6 +1390,7 @@ def test_certificate_managed_file_managed_create_false(
     assert not pathlib.Path(cert_args["name"]).exists()
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_cert")
 @pytest.mark.parametrize("existing_cert", [{"mode": "0644"}], indirect=True)
 def test_certificate_managed_mode_change_only(
@@ -1409,6 +1412,7 @@ def test_certificate_managed_mode_change_only(
     assert cert_new.serial_number == cert.serial_number
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_cert")
 def test_certificate_managed_mode_test_true(x509, cert_args, modules):
     """
@@ -1521,6 +1525,21 @@ def test_certificate_managed_pkcs12_embedded_pk_kept(
     assert new_pk.public_key().public_numbers() == cur_pk.public_key().public_numbers()
 
 
+@pytest.mark.parametrize("prepend_cn", [False, True])
+def test_certificate_managed_copypath(
+    x509, cert_args, rsa_privkey, ca_key, prepend_cn, tmp_path
+):
+    cert_args["private_key"] = rsa_privkey
+    cert_args["copypath"] = str(tmp_path)
+    cert_args["prepend_cn"] = prepend_cn
+    ret = x509.certificate_managed(**cert_args)
+    cert = _assert_cert_basic(ret, cert_args["name"], rsa_privkey, ca_key)
+    prefix = ""
+    if prepend_cn:
+        prefix = "success-"
+    assert (tmp_path / f"{prefix}{cert.serial_number:x}.crt").exists()
+
+
 def test_crl_managed_empty(x509, crl_args, ca_key):
     ret = x509.crl_managed(**crl_args)
     crl = _assert_crl_basic(ret, ca_key)
@@ -1750,6 +1769,7 @@ def test_crl_managed_existing_encoding_change_only(x509, crl_args, ca_key):
     assert new.extensions[0].value.crl_number == 1
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.parametrize("mode", ["0400", "0640", "0644"])
 def test_crl_managed_mode(x509, crl_args, ca_key, mode, modules):
     """
@@ -1772,6 +1792,7 @@ def test_crl_managed_file_managed_create_false(x509, crl_args):
     assert not pathlib.Path(crl_args["name"]).exists()
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_crl")
 @pytest.mark.parametrize(
     "existing_crl",
@@ -1797,6 +1818,7 @@ def test_crl_managed_mode_change_only(x509, crl_args, ca_key, modules):
     )
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_crl")
 def test_crl_managed_mode_test_true(x509, crl_args, modules):
     """
@@ -2044,6 +2066,7 @@ def test_csr_managed_extension_removed(x509, csr_args, csr_args_exts, rsa_privke
     }
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.parametrize("mode", ["0400", "0640", "0644"])
 def test_csr_managed_mode(x509, csr_args, rsa_privkey, mode, modules):
     """
@@ -2066,6 +2089,7 @@ def test_csr_managed_file_managed_create_false(x509, csr_args):
     assert not pathlib.Path(csr_args["name"]).exists()
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_csr")
 @pytest.mark.parametrize("existing_csr", [{"mode": "0644"}], indirect=True)
 def test_csr_managed_mode_change_only(x509, csr_args, ca_key, modules):
@@ -2082,6 +2106,7 @@ def test_csr_managed_mode_change_only(x509, csr_args, ca_key, modules):
     assert modules.file.get_mode(csr_args["name"]) == "0640"
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_csr")
 def test_csr_managed_mode_test_true(x509, csr_args, modules):
     """
@@ -2364,6 +2389,7 @@ def test_private_key_managed_passphrase_changed_overwrite(x509, pk_args):
     _assert_pk_basic(ret, "rsa", passphrase="hunter1")
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.parametrize("encoding", ["pem", "der"])
 @pytest.mark.parametrize("mode", [None, "0600", "0644"])
 def test_private_key_managed_mode(x509, pk_args, mode, encoding, modules):
@@ -2388,6 +2414,7 @@ def test_private_key_managed_file_managed_create_false(x509, pk_args):
     assert not pathlib.Path(pk_args["name"]).exists()
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_pk")
 def test_private_key_managed_mode_test_true(x509, pk_args, modules):
     """
@@ -2463,6 +2490,7 @@ def test_private_key_managed_follow_symlinks_changes(
     assert pathlib.Path(ret.name).is_symlink() == follow
 
 
+@pytest.mark.skip_on_windows
 @pytest.mark.usefixtures("existing_pk")
 @pytest.mark.parametrize("existing_pk", [{"mode": "0400"}], indirect=True)
 def test_private_key_managed_mode_change_only(x509, pk_args, modules):