regenerate and copy CRL during reissue; run restorecon after every CRL copy

Reissue internally revokes the old cert (revoke_issued), which makes the published CRL immediately stale, but neither the CLI --reissue path nor the TUI renew flow ever regenerated/copied it — only standalone revoke and "Regenerate CRL" did. A cert revoked as part of a reissue could keep authenticating against OpenVPN until someone remembered to regenerate the CRL by hand. Both reissue paths now call gen_crl()+copy_crl() right after a successful revoke_issued(), same as standalone revoke. This isn't gated on the subsequent build_client_full() succeeding, since the CRL is already stale the moment the old cert is revoked. A CRL-regen failure here is reported as a warning and the reissue continues to build the replacement cert — a new cert is more urgent than a fresh CRL, and "Regenerate CRL"/--gen-crl remain available to retry. Also add a RESTORECON_BINARY setting (default "restorecon", "" to disable): copy_crl() now runs it on the destination file after every copy, everywhere copy_crl() is called. Best-effort like is_ca_key_encrypted() — a missing/misconfigured binary is swallowed rather than breaking CRL deployment on non-SELinux systems. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-09 10:35:17 +03:00
parent bb11702c4f
commit 0477392b5e
4 changed files with 105 additions and 10 deletions
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -26,7 +26,7 @@ Edit the `SETTINGS` block at the top of `openvpncertupdate.py` before first run.
 | Flag | Effect |
 |---|---|
 | `--create CN` | Issue new cert (requires `--email`) |
-| `--reissue CN` | Revoke + reissue cert (`--email` optional, falls back to stored) |
+| `--reissue CN` | Revoke + regenerate CRL + reissue cert (`--email` optional, falls back to stored) |
 | `--revoke CN` | Revoke cert and regenerate CRL |
 | `--gen-crl` | Regenerate and copy CRL only |
 | `--list` | List recently-expired/soon-to-expire CNs (per `DAYS_PAST`/`DAYS_AHEAD`) with email; read-only, no CA passphrase needed |
@@ -67,8 +67,8 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v  # single test
 ## Re-issue workflow

 1. `revoke-issued <CN>` — archives old key + CSR to `pki/revoked/`
-2. `build-client-full <CN> --passout=pass:<pw>` — generates new key + cert
-3. CRL **not** auto-updated during renewal; use "Regenerate CRL" menu item or `r` hotkey
+2. CRL regenerated and copied to `CRL_DEST_PATH` immediately after the revoke succeeds — the old cert is already revoked at this point, so the published CRL would otherwise be stale until a separate manual regen. Not fatal: a failure here is reported but the workflow continues to step 3 (a new cert is more urgent than a fresh CRL, and "Regenerate CRL" / `--gen-crl` remain available to retry)
+3. `build-client-full <CN> --passout=pass:<pw>` — generates new key + cert

 ## TUI key bindings

@@ -88,7 +88,7 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v  # single test
 - EasyRSA called with `--batch`; `--passin=pass:<passphrase>` omitted when the resolved passphrase is empty
 - CA passphrase resolution (`resolve_ca_passphrase()`, run once in `main()` right after arg parsing, before dispatch): `CA_PASSPHRASE=""` → auto-detect via `is_ca_key_encrypted()` (checks `<PKI_DIR>/private/ca.key` PEM header for `ENCRYPTED`) and prompt only if encrypted; `"!empty"` → never check/prompt, passphrase is `""`; `"!ask"` → always prompt, skip detection; any other value → used literally. `--list`/`--list-all` skip this resolution entirely since they only read `index.txt` and never touch the CA
 - Cryptgeon: matches the `occulto` browser client — `key=os.urandom(32)` used directly (no derivation) for AES-256-GCM; `contents` = `base64(b"AES-GCM") + "--" + base64(nonce) + "--" + base64(ciphertext)`; `meta` = JSON string `{"type": "text"}`; URL = `<base>/note/<id>#<key.hex()>`
- `copy_crl()` does `chmod 644` after copy
+- `copy_crl()` does `chmod 644` after copy, then runs `RESTORECON_BINARY` (default `restorecon`) on the copied file — best-effort like `is_ca_key_encrypted()`: a missing/misconfigured binary is swallowed, not fatal. Set `RESTORECON_BINARY=""` to disable on non-SELinux systems
 - Password: pos 1=uppercase, pos 2=lowercase (no j), pos 3-27=alphanumeric, pos 28=lowercase (no j); `oO01lIQ5S2Z8B` banned everywhere
 - Inline file path: `<PKI_DIR>/inline/private/<CN>.inline`
 - User emails are not stored separately: `build_client_full()` sets `EASYRSA_REQ_EMAIL` whenever an email is known, which EasyRSA embeds as `emailAddress=` in the cert subject — so it round-trips through `<PKI_DIR>/index.txt` itself. `get_email()` reads it back from there; there is no `openvpncertupdate-metadata.json`
--- a/openvpncertupdate.py
+++ b/openvpncertupdate.py
@@ -54,6 +54,7 @@ CONFIG_NAME        = "client.ovpn"
 VPN_CONFIGS_DIR    = "./vpn-configs"

 CRL_DEST_PATH      = "/etc/openvpn/crl.pem"
+RESTORECON_BINARY  = "restorecon"  # run on CRL_DEST_PATH after each copy; "" disables (non-SELinux systems)

 CRYPTGEON_URL      = "https://cryptgeon.example.com"

@@ -76,7 +77,7 @@ DAYS_AHEAD  = 14
 _OVERRIDABLE_SETTINGS = frozenset({
    "EASYRSA_DIR", "EASYRSA_PKI_DIR", "CA_PASSPHRASE",
    "OVPN_TEMPLATE_PATH", "CONFIG_NAME", "VPN_CONFIGS_DIR",
-    "CRL_DEST_PATH",
+    "CRL_DEST_PATH", "RESTORECON_BINARY",
    "CRYPTGEON_URL",
    "MAIL_FROM", "MAIL_SUBJECT", "EMAIL_TEMPLATE_PATH", "MAIL_BINARY",
    "SMTP_HOST", "SMTP_PORT", "SMTP_USER", "SMTP_PASSWORD", "SMTP_TLS",
@@ -307,9 +308,16 @@ def gen_crl(easyrsa_dir: str, pki_dir: str, ca_passphrase: str) -> None:
    )


-def copy_crl(pki_dir: str, dest_path: str) -> None:
+def copy_crl(pki_dir: str, dest_path: str, restorecon_binary: str = "") -> None:
    shutil.copy2(f"{pki_dir}/crl.pem", dest_path)
    os.chmod(dest_path, 0o644)
+    if restorecon_binary:
+        # Best-effort, like is_ca_key_encrypted(): a missing/misconfigured
+        # restorecon must not break CRL deployment on non-SELinux systems.
+        try:
+            subprocess.run([restorecon_binary, dest_path], capture_output=True, text=True)
+        except OSError:
+            pass


 def is_ca_key_encrypted(pki_dir: str) -> bool:
@@ -1113,6 +1121,20 @@ class CursesApp:
            except EasyRSAError as exc:
                self._error(stdscr, f"EasyRSA error during revoke:\n{exc}")
                return True
+            # The old cert is now revoked, so the published CRL is stale
+            # until regenerated — do that now rather than leaving it to a
+            # separate manual "Regenerate CRL" step. Not fatal: the new
+            # cert is more urgent than a fresh CRL, so keep going either way.
+            try:
+                gen_crl(EASYRSA_DIR, EASYRSA_PKI_DIR, CA_PASSPHRASE)
+                copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH, RESTORECON_BINARY)
+            except EasyRSAError as exc:
+                self._error(
+                    stdscr,
+                    f"CRL update failed:\n{exc}\n\n"
+                    f"{final_cn} was revoked but the published CRL is stale.\n"
+                    f"Use \"Regenerate CRL\" once this finishes.",
+                )

        try:
            build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR,
@@ -1189,7 +1211,7 @@ class CursesApp:
        try:
            revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, CA_PASSPHRASE)
            gen_crl(EASYRSA_DIR, EASYRSA_PKI_DIR, CA_PASSPHRASE)
-            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH)
+            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH, RESTORECON_BINARY)
        except EasyRSAError as exc:
            self._error(stdscr, f"Revoke failed:\n{exc}")
            return
@@ -1200,7 +1222,7 @@ class CursesApp:
        self._msg(stdscr, "Regenerating CRL…")
        try:
            gen_crl(EASYRSA_DIR, EASYRSA_PKI_DIR, CA_PASSPHRASE)
-            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH)
+            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH, RESTORECON_BINARY)
        except EasyRSAError as exc:
            self._error(stdscr, f"gen-crl failed:\n{exc}")
            return
@@ -1312,7 +1334,7 @@ class CliRunner:
        try:
            revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, CA_PASSPHRASE)
            gen_crl(EASYRSA_DIR, EASYRSA_PKI_DIR, CA_PASSPHRASE)
-            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH)
+            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH, RESTORECON_BINARY)
        except EasyRSAError as exc:
            print(f"error: {exc}", file=sys.stderr)
            sys.exit(1)
@@ -1322,7 +1344,7 @@ class CliRunner:
        print("Regenerating CRL…", file=sys.stderr)
        try:
            gen_crl(EASYRSA_DIR, EASYRSA_PKI_DIR, CA_PASSPHRASE)
-            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH)
+            copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH, RESTORECON_BINARY)
        except EasyRSAError as exc:
            print(f"error: {exc}", file=sys.stderr)
            sys.exit(1)
@@ -1344,6 +1366,21 @@ class CliRunner:
            except EasyRSAError as exc:
                print(f"error during revoke: {exc}", file=sys.stderr)
                sys.exit(1)
+            # The old cert is now revoked, so the published CRL is stale
+            # until regenerated — do that now rather than leaving it to a
+            # separate --gen-crl run. Not fatal: the new cert is more
+            # urgent than a fresh CRL, so keep going either way.
+            print(f"Regenerating CRL…", file=sys.stderr)
+            try:
+                gen_crl(EASYRSA_DIR, EASYRSA_PKI_DIR, CA_PASSPHRASE)
+                copy_crl(EASYRSA_PKI_DIR, CRL_DEST_PATH, RESTORECON_BINARY)
+            except EasyRSAError as exc:
+                print(
+                    f"warning: CRL update failed: {exc}\n"
+                    f"warning: {cn} was revoked but the published CRL is stale; "
+                    f"run --gen-crl once this finishes.",
+                    file=sys.stderr,
+                )

        print(f"Building certificate for {cn}…", file=sys.stderr)
        try:
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -14,6 +14,8 @@ def _patch_issue(monkeypatch, ovpn_path="/out/cn_2026-01-01/client.ovpn",
                 one_time_url="https://cg.example.com/#/note/abc/xyz"):
    """Patch all side-effectful callables used by CliRunner._issue."""
    monkeypatch.setattr("openvpncertupdate.revoke_issued", MagicMock())
+    monkeypatch.setattr("openvpncertupdate.gen_crl", MagicMock())
+    monkeypatch.setattr("openvpncertupdate.copy_crl", MagicMock())
    monkeypatch.setattr("openvpncertupdate.build_client_full", MagicMock())
    monkeypatch.setattr("openvpncertupdate.build_ovpn", MagicMock(return_value=ovpn_path))
    monkeypatch.setattr("openvpncertupdate.create_note", MagicMock(return_value=one_time_url))
@@ -21,6 +23,8 @@ def _patch_issue(monkeypatch, ovpn_path="/out/cn_2026-01-01/client.ovpn",
    monkeypatch.setattr("openvpncertupdate.generate_password", MagicMock(return_value="Testpass1234567890abcdefgh"))
    return {
        "revoke_issued":    sys.modules["openvpncertupdate"].revoke_issued,
+        "gen_crl":          sys.modules["openvpncertupdate"].gen_crl,
+        "copy_crl":         sys.modules["openvpncertupdate"].copy_crl,
        "build_client_full": sys.modules["openvpncertupdate"].build_client_full,
        "build_ovpn":       sys.modules["openvpncertupdate"].build_ovpn,
        "create_note":      sys.modules["openvpncertupdate"].create_note,
@@ -87,6 +91,33 @@ def test_reissue_calls_revoke_then_build(monkeypatch, capsys):
    assert mocks["build_client_full"].call_args.args[2] == "bob"


+def test_reissue_regenerates_and_copies_crl(monkeypatch, capsys):
+    # The old cert is revoked as part of reissue, so the published CRL is
+    # stale until regenerated — reissue must do that itself now.
+    mocks = _patch_issue(monkeypatch)
+    CliRunner().reissue("bob", "bob@example.com")
+    mocks["gen_crl"].assert_called_once()
+    mocks["copy_crl"].assert_called_once()
+
+
+def test_create_does_not_touch_crl(monkeypatch, capsys):
+    # --create never revokes anything, so there's nothing stale to fix.
+    mocks = _patch_issue(monkeypatch)
+    CliRunner().create("alice", "alice@example.com")
+    mocks["gen_crl"].assert_not_called()
+    mocks["copy_crl"].assert_not_called()
+
+
+def test_reissue_continues_building_when_crl_regen_fails(monkeypatch, capsys):
+    import openvpncertupdate
+    mocks = _patch_issue(monkeypatch)
+    monkeypatch.setattr("openvpncertupdate.gen_crl",
+                        MagicMock(side_effect=openvpncertupdate.EasyRSAError("gen-crl failed")))
+    CliRunner().reissue("bob", "bob@example.com")
+    mocks["build_client_full"].assert_called_once()
+    assert "CRL update failed" in capsys.readouterr().err
+
+
 def test_reissue_falls_back_to_index_txt_email(monkeypatch, capsys):
    # get_email() reads straight from index.txt; this stubs that lookup
    # rather than the file itself (index.txt parsing is covered in test_pki.py).
--- a/tests/test_easyrsa.py
+++ b/tests/test_easyrsa.py
@@ -68,6 +68,33 @@ def test_copy_crl_copies_and_chmods(mock_chmod, mock_copy):
    mock_chmod.assert_called_once_with("/etc/openvpn/crl.pem", 0o644)


+@patch("openvpncertupdate.subprocess.run")
+@patch("openvpncertupdate.shutil.copy2")
+@patch("openvpncertupdate.os.chmod")
+def test_copy_crl_skips_restorecon_when_binary_empty(mock_chmod, mock_copy, mock_run):
+    copy_crl("/pki", "/etc/openvpn/crl.pem", restorecon_binary="")
+    mock_run.assert_not_called()
+
+
+@patch("openvpncertupdate.subprocess.run")
+@patch("openvpncertupdate.shutil.copy2")
+@patch("openvpncertupdate.os.chmod")
+def test_copy_crl_runs_restorecon_when_binary_set(mock_chmod, mock_copy, mock_run):
+    copy_crl("/pki", "/etc/openvpn/crl.pem", restorecon_binary="restorecon")
+    mock_run.assert_called_once_with(
+        ["restorecon", "/etc/openvpn/crl.pem"], capture_output=True, text=True)
+
+
+@patch("openvpncertupdate.subprocess.run", side_effect=FileNotFoundError("no restorecon"))
+@patch("openvpncertupdate.shutil.copy2")
+@patch("openvpncertupdate.os.chmod")
+def test_copy_crl_restorecon_failure_is_non_fatal(mock_chmod, mock_copy, mock_run):
+    # Missing/misconfigured restorecon must not break CRL deployment.
+    copy_crl("/pki", "/etc/openvpn/crl.pem", restorecon_binary="restorecon")  # must not raise
+    mock_copy.assert_called_once()
+    mock_chmod.assert_called_once()
+
+
 # ---------------------------------------------------------------------------
 # is_ca_key_encrypted
 # ---------------------------------------------------------------------------