Fix re-issue for CNs whose issued cert file is missing
Renewal failed with an empty error box for any CN listed in index.txt without a corresponding pki/issued/<CN>.crt — the state you get when an index.txt is carried over from an older EasyRSA install but the issued/ files are not. Two defects: 1. EasyRSA writes its diagnostics to stdout, not stderr: print() is `printf '%s\n'`, and both die() and user_error() route through it. stderr only carries output from the tools EasyRSA shells out to, and even that is silenced under -S/--silent-ssl. _run_easyrsa built its message from stderr alone, so every EasyRSA failure reported blank. _easyrsa_diagnostics() now merges both streams (stderr first, so the specific openssl message is not what the dialog clips) and drops the version banner and blank padding. 2. EasyRSA reads the serial out of the .crt itself, so revoke-issued cannot revoke a CN whose cert file is gone — and there is nothing to add to the CRL either. has_issued_cert() now gates the revoke and CRL steps in both CliRunner._issue() and CursesApp._process_cert(); the workflow warns and goes straight to build-client-full. An explicit --revoke / TUI `r` still fails loudly rather than silently no-op. The post-build failure message now keys off whether a revoke actually happened, not off is_renewal, so it no longer claims "has been revoked" when nothing was. Adds tests/test_app_reissue.py: the TUI re-issue path had no coverage at all, and it is the path this bug was reported from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -53,7 +53,7 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v # single test
|
|||||||
| SETTINGS OVERRIDE | `ConfigError`, `load_settings_overrides()`, `_OVERRIDABLE_SETTINGS` |
|
| SETTINGS OVERRIDE | `ConfigError`, `load_settings_overrides()`, `_OVERRIDABLE_SETTINGS` |
|
||||||
| PKI | `CertInfo`, `_load_current_certs()`, `load_expiring_certs()`, `load_all_certs()`, `_parse_index_line()`, `get_email()` |
|
| PKI | `CertInfo`, `_load_current_certs()`, `load_expiring_certs()`, `load_all_certs()`, `_parse_index_line()`, `get_email()` |
|
||||||
| PASSWORD | `generate_password()` |
|
| PASSWORD | `generate_password()` |
|
||||||
| EASYRSA | `EasyRSAError`, `revoke_issued()`, `build_client_full()`, `gen_crl()`, `copy_crl()`, `is_ca_key_encrypted()`, `resolve_ca_passphrase()` |
|
| EASYRSA | `EasyRSAError`, `_easyrsa_diagnostics()`, `issued_cert_path()`, `has_issued_cert()`, `revoke_issued()`, `build_client_full()`, `gen_crl()`, `copy_crl()`, `is_ca_key_encrypted()`, `resolve_ca_passphrase()` |
|
||||||
| CONFIG | `build_ovpn()` → `vpn-configs/<CN>_<YYYY-MM-DD>_<NN>/CONFIG_NAME` |
|
| CONFIG | `build_ovpn()` → `vpn-configs/<CN>_<YYYY-MM-DD>_<NN>/CONFIG_NAME` |
|
||||||
| CRYPTGEON | `CryptgeonError`, `create_note()` |
|
| CRYPTGEON | `CryptgeonError`, `create_note()` |
|
||||||
| MAILER | `build_mime_message()`, `send_email()` |
|
| MAILER | `build_mime_message()`, `send_email()` |
|
||||||
@@ -66,6 +66,7 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v # single test
|
|||||||
|
|
||||||
## Re-issue workflow
|
## Re-issue workflow
|
||||||
|
|
||||||
|
0. `has_issued_cert()` gates steps 1–2: if `<PKI_DIR>/issued/<CN>.crt` is absent, both are **skipped** with a warning and the workflow goes straight to step 3. EasyRSA reads the serial out of the `.crt` itself, so `revoke-issued` can only fail on such a CN — and there is nothing to add to the CRL either. This happens when an `index.txt` is carried over from an older EasyRSA install without the `issued/` files: the index still lists V-status certs whose `.crt` never came along. `--revoke` / the TUI `r` key deliberately do *not* skip — an explicit revoke request should fail loudly rather than silently no-op
|
||||||
1. `revoke-issued <CN>` — archives old key + CSR to `pki/revoked/`
|
1. `revoke-issued <CN>` — archives old key + CSR to `pki/revoked/`
|
||||||
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)
|
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
|
3. `build-client-full <CN> --passout=pass:<pw>` — generates new key + cert
|
||||||
@@ -86,6 +87,7 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v # single test
|
|||||||
- External config file (`load_settings_overrides()`, run once in `main()` right after arg parsing, before dispatch): resolution order is `--config PATH` > `CONFIG_PATH` setting > `<this-script-path>.conf` next to the script. The CLI flag or `CONFIG_PATH` make the path explicit — a missing file there is a fatal `ConfigError`; the default `<script>.conf` path is optional and silently skipped if absent. The file is executed as Python (same syntax as the `SETTINGS` block, so only run trusted files) and only names listed in `_OVERRIDABLE_SETTINGS` are applied — `CONFIG_PATH` itself is deliberately not overridable this way
|
- External config file (`load_settings_overrides()`, run once in `main()` right after arg parsing, before dispatch): resolution order is `--config PATH` > `CONFIG_PATH` setting > `<this-script-path>.conf` next to the script. The CLI flag or `CONFIG_PATH` make the path explicit — a missing file there is a fatal `ConfigError`; the default `<script>.conf` path is optional and silently skipped if absent. The file is executed as Python (same syntax as the `SETTINGS` block, so only run trusted files) and only names listed in `_OVERRIDABLE_SETTINGS` are applied — `CONFIG_PATH` itself is deliberately not overridable this way
|
||||||
- Email: set `SMTP_HOST` to use smtplib (SMTP_TLS: `"starttls"`/`"ssl"`/`""`); leave empty to use `MAIL_BINARY`. Auth skipped when `SMTP_USER=""`
|
- Email: set `SMTP_HOST` to use smtplib (SMTP_TLS: `"starttls"`/`"ssl"`/`""`); leave empty to use `MAIL_BINARY`. Auth skipped when `SMTP_USER=""`
|
||||||
- EasyRSA called with `--batch`; `--passin=pass:<passphrase>` omitted when the resolved passphrase is empty
|
- EasyRSA called with `--batch`; `--passin=pass:<passphrase>` omitted when the resolved passphrase is empty
|
||||||
|
- EasyRSA error text arrives on **stdout**, not stderr: its `print()` is `printf '%s\n'`, and both `die()` and `user_error()` route through it. stderr only carries output from the tools EasyRSA shells out to (openssl), and even that is silenced under `-S/--silent-ssl` (not passed here). `_easyrsa_diagnostics()` therefore merges both streams — building an error from stderr alone reports failures as blank
|
||||||
- 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
|
- 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()>`
|
- 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, 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
|
- `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
|
||||||
|
|||||||
@@ -261,6 +261,33 @@ class EasyRSAError(Exception):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def _easyrsa_diagnostics(stdout: str, stderr: str) -> str:
|
||||||
|
"""Merge both streams of a failed EasyRSA run into displayable error text.
|
||||||
|
|
||||||
|
EasyRSA's print() is `printf '%s\\n'`, so die() and user_error() write their
|
||||||
|
diagnostics to *stdout*; stderr only carries output from the tools EasyRSA
|
||||||
|
shells out to (openssl). Reporting stderr alone therefore leaves most
|
||||||
|
failures with an empty message. Blank padding and the version banner around
|
||||||
|
EasyRSA's error block are dropped so the text fits the TUI error dialog.
|
||||||
|
|
||||||
|
stderr comes first: when EasyRSA dies at the openssl step its stdout also
|
||||||
|
carries the warn() banner and show_host() footer, so the specific cause
|
||||||
|
(openssl's own message) must lead or it is what the dialog clips.
|
||||||
|
"""
|
||||||
|
lines = []
|
||||||
|
for stream in (stderr, stdout):
|
||||||
|
for raw in (stream or "").splitlines():
|
||||||
|
line = raw.rstrip()
|
||||||
|
if not line.strip():
|
||||||
|
continue
|
||||||
|
if line.startswith("EasyRSA version ") or set(line.strip()) in (
|
||||||
|
{"-"}, {"="},
|
||||||
|
):
|
||||||
|
continue
|
||||||
|
lines.append(line)
|
||||||
|
return "\n".join(lines)
|
||||||
|
|
||||||
|
|
||||||
def _run_easyrsa(cmd: List[str], cwd: str,
|
def _run_easyrsa(cmd: List[str], cwd: str,
|
||||||
extra_env: Optional[Dict[str, str]] = None) -> None:
|
extra_env: Optional[Dict[str, str]] = None) -> None:
|
||||||
env = None
|
env = None
|
||||||
@@ -274,8 +301,9 @@ def _run_easyrsa(cmd: List[str], cwd: str,
|
|||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
# Find the first non-flag argument after the binary (the actual easyrsa verb)
|
# Find the first non-flag argument after the binary (the actual easyrsa verb)
|
||||||
verb = next((c for c in cmd[1:] if not c.startswith("-")), "unknown")
|
verb = next((c for c in cmd[1:] if not c.startswith("-")), "unknown")
|
||||||
|
detail = _easyrsa_diagnostics(result.stdout, result.stderr)
|
||||||
raise EasyRSAError(
|
raise EasyRSAError(
|
||||||
f"{verb} failed (exit {result.returncode}): {result.stderr.strip()}"
|
f"{verb} failed (exit {result.returncode}):\n{detail}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -286,6 +314,21 @@ def _base_cmd(easyrsa_dir: str, pki_dir: str, ca_passphrase: str) -> List[str]:
|
|||||||
return cmd
|
return cmd
|
||||||
|
|
||||||
|
|
||||||
|
def issued_cert_path(pki_dir: str, cn: str) -> str:
|
||||||
|
return f"{pki_dir}/issued/{cn}.crt"
|
||||||
|
|
||||||
|
|
||||||
|
def has_issued_cert(pki_dir: str, cn: str) -> bool:
|
||||||
|
"""Whether EasyRSA still holds the signed certificate for this CN.
|
||||||
|
|
||||||
|
index.txt can carry a V-status line whose .crt is gone — e.g. an index
|
||||||
|
copied over from an older EasyRSA install without the issued/ files.
|
||||||
|
EasyRSA reads the serial out of the .crt itself, so `revoke-issued` fails
|
||||||
|
outright on those CNs; the re-issue workflow skips the revoke step instead.
|
||||||
|
"""
|
||||||
|
return os.path.isfile(issued_cert_path(pki_dir, cn))
|
||||||
|
|
||||||
|
|
||||||
def revoke_issued(
|
def revoke_issued(
|
||||||
easyrsa_dir: str, pki_dir: str, cn: str, ca_passphrase: str
|
easyrsa_dir: str, pki_dir: str, cn: str, ca_passphrase: str
|
||||||
) -> None:
|
) -> None:
|
||||||
@@ -1126,12 +1169,25 @@ class CursesApp:
|
|||||||
final_cn = form.cn
|
final_cn = form.cn
|
||||||
self._msg(stdscr, f"Generating certificate for {final_cn}…")
|
self._msg(stdscr, f"Generating certificate for {final_cn}…")
|
||||||
|
|
||||||
if is_renewal:
|
revoked = False
|
||||||
|
if is_renewal and not has_issued_cert(EASYRSA_PKI_DIR, final_cn):
|
||||||
|
# index.txt lists the cert but the .crt is gone, so there is
|
||||||
|
# nothing EasyRSA can revoke. Issue the replacement anyway.
|
||||||
|
warning = (
|
||||||
|
f"No certificate file for {final_cn} at\n"
|
||||||
|
f"{issued_cert_path(EASYRSA_PKI_DIR, final_cn)}\n\n"
|
||||||
|
f"Nothing to revoke — skipping revoke and CRL update.\n"
|
||||||
|
f"Issuing a new certificate only."
|
||||||
|
)
|
||||||
|
self._log(f"{final_cn}: no issued cert file, revoke skipped")
|
||||||
|
self._msg(stdscr, warning, wait=True)
|
||||||
|
elif is_renewal:
|
||||||
try:
|
try:
|
||||||
revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, final_cn, CA_PASSPHRASE)
|
revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, final_cn, CA_PASSPHRASE)
|
||||||
except EasyRSAError as exc:
|
except EasyRSAError as exc:
|
||||||
self._error(stdscr, f"EasyRSA error during revoke:\n{exc}")
|
self._error(stdscr, f"EasyRSA error during revoke:\n{exc}")
|
||||||
return True
|
return True
|
||||||
|
revoked = True
|
||||||
# The old cert is now revoked, so the published CRL is stale
|
# The old cert is now revoked, so the published CRL is stale
|
||||||
# until regenerated — do that now rather than leaving it to a
|
# until regenerated — do that now rather than leaving it to a
|
||||||
# separate manual "Regenerate CRL" step. Not fatal: the new
|
# separate manual "Regenerate CRL" step. Not fatal: the new
|
||||||
@@ -1152,7 +1208,7 @@ class CursesApp:
|
|||||||
final_cn, form.password, CA_PASSPHRASE,
|
final_cn, form.password, CA_PASSPHRASE,
|
||||||
email=form.email)
|
email=form.email)
|
||||||
except EasyRSAError as exc:
|
except EasyRSAError as exc:
|
||||||
if is_renewal:
|
if revoked:
|
||||||
self._error(
|
self._error(
|
||||||
stdscr,
|
stdscr,
|
||||||
f"EasyRSA error during build-client-full:\n{exc}\n\n"
|
f"EasyRSA error during build-client-full:\n{exc}\n\n"
|
||||||
@@ -1370,13 +1426,25 @@ class CliRunner:
|
|||||||
send_email_flag: bool = True, show_eml: bool = False) -> None:
|
send_email_flag: bool = True, show_eml: bool = False) -> None:
|
||||||
password = generate_password()
|
password = generate_password()
|
||||||
|
|
||||||
if is_renewal:
|
revoked = False
|
||||||
|
if is_renewal and not has_issued_cert(EASYRSA_PKI_DIR, cn):
|
||||||
|
# index.txt lists the cert but the .crt is gone, so there is
|
||||||
|
# nothing EasyRSA can revoke. Issue the replacement anyway.
|
||||||
|
print(
|
||||||
|
f"warning: no certificate file at "
|
||||||
|
f"{issued_cert_path(EASYRSA_PKI_DIR, cn)}\n"
|
||||||
|
f"warning: nothing to revoke for {cn} — skipping revoke and CRL "
|
||||||
|
f"update, issuing a new certificate only.",
|
||||||
|
file=sys.stderr,
|
||||||
|
)
|
||||||
|
elif is_renewal:
|
||||||
print(f"Revoking existing cert for {cn}…", file=sys.stderr)
|
print(f"Revoking existing cert for {cn}…", file=sys.stderr)
|
||||||
try:
|
try:
|
||||||
revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, CA_PASSPHRASE)
|
revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, CA_PASSPHRASE)
|
||||||
except EasyRSAError as exc:
|
except EasyRSAError as exc:
|
||||||
print(f"error during revoke: {exc}", file=sys.stderr)
|
print(f"error during revoke: {exc}", file=sys.stderr)
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
revoked = True
|
||||||
# The old cert is now revoked, so the published CRL is stale
|
# The old cert is now revoked, so the published CRL is stale
|
||||||
# until regenerated — do that now rather than leaving it to a
|
# until regenerated — do that now rather than leaving it to a
|
||||||
# separate --gen-crl run. Not fatal: the new cert is more
|
# separate --gen-crl run. Not fatal: the new cert is more
|
||||||
@@ -1398,7 +1466,7 @@ class CliRunner:
|
|||||||
build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, password, CA_PASSPHRASE,
|
build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, password, CA_PASSPHRASE,
|
||||||
email=email_addr)
|
email=email_addr)
|
||||||
except EasyRSAError as exc:
|
except EasyRSAError as exc:
|
||||||
if is_renewal:
|
if revoked:
|
||||||
print(
|
print(
|
||||||
f"error during build-client-full: {exc}\n"
|
f"error during build-client-full: {exc}\n"
|
||||||
f"WARNING: {cn} has been revoked but no new cert was built.\n"
|
f"WARNING: {cn} has been revoked but no new cert was built.\n"
|
||||||
|
|||||||
92
tests/test_app_reissue.py
Normal file
92
tests/test_app_reissue.py
Normal file
@@ -0,0 +1,92 @@
|
|||||||
|
"""Tests for the TUI re-issue workflow (CursesApp._process_cert).
|
||||||
|
|
||||||
|
Uses the same mock-curses approach as test_dialogs.py — no real terminal
|
||||||
|
needed. _msg()/_error() swallow curses.error, so a MagicMock stdscr is enough
|
||||||
|
as long as getmaxyx() returns real ints.
|
||||||
|
"""
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
import curses as _curses
|
||||||
|
|
||||||
|
# Stub curses constants/callables before importing the module under test.
|
||||||
|
_curses.color_pair = lambda x: 0
|
||||||
|
_curses.A_BOLD = 0
|
||||||
|
_curses.A_UNDERLINE = 0
|
||||||
|
_curses.curs_set = lambda x: None
|
||||||
|
|
||||||
|
from openvpncertupdate import CursesApp, CertFormResult
|
||||||
|
|
||||||
|
|
||||||
|
def _stdscr():
|
||||||
|
s = MagicMock()
|
||||||
|
s.getmaxyx.return_value = (24, 80) # `sh - 2` needs a real int
|
||||||
|
s.getch.return_value = ord("q")
|
||||||
|
return s
|
||||||
|
|
||||||
|
|
||||||
|
def _patch_workflow(monkeypatch, cert_file_present):
|
||||||
|
"""Patch everything _process_cert touches after the form is confirmed."""
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"openvpncertupdate.show_cert_form",
|
||||||
|
MagicMock(return_value=CertFormResult(
|
||||||
|
cn="y.kuts", email="", password="Testpass1234567890abcdefgh",
|
||||||
|
confirmed=True)))
|
||||||
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert",
|
||||||
|
MagicMock(return_value=cert_file_present))
|
||||||
|
for name, retval in (
|
||||||
|
("revoke_issued", None),
|
||||||
|
("gen_crl", None),
|
||||||
|
("copy_crl", None),
|
||||||
|
("build_client_full", None),
|
||||||
|
("build_ovpn", "/out/y.kuts_2026-08-15_01/client.ovpn"),
|
||||||
|
("create_note", "https://cg.example.com/note/abc#deadbeef"),
|
||||||
|
):
|
||||||
|
monkeypatch.setattr(f"openvpncertupdate.{name}",
|
||||||
|
MagicMock(return_value=retval))
|
||||||
|
import openvpncertupdate as m
|
||||||
|
return {n: getattr(m, n) for n in (
|
||||||
|
"revoke_issued", "gen_crl", "copy_crl", "build_client_full")}
|
||||||
|
|
||||||
|
|
||||||
|
def test_tui_reissue_skips_revoke_when_cert_file_missing(monkeypatch):
|
||||||
|
# The bug as reported: the TUI offered renewal for a CN whose .crt was
|
||||||
|
# never carried over from the older EasyRSA install, and revoke-issued
|
||||||
|
# failed. There is nothing to revoke — build the replacement instead.
|
||||||
|
mocks = _patch_workflow(monkeypatch, cert_file_present=False)
|
||||||
|
app = CursesApp()
|
||||||
|
assert app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True) is True
|
||||||
|
mocks["revoke_issued"].assert_not_called()
|
||||||
|
mocks["gen_crl"].assert_not_called()
|
||||||
|
mocks["copy_crl"].assert_not_called()
|
||||||
|
mocks["build_client_full"].assert_called_once()
|
||||||
|
assert mocks["build_client_full"].call_args.args[2] == "y.kuts"
|
||||||
|
|
||||||
|
|
||||||
|
def test_tui_reissue_logs_the_skip(monkeypatch):
|
||||||
|
_patch_workflow(monkeypatch, cert_file_present=False)
|
||||||
|
app = CursesApp()
|
||||||
|
app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
||||||
|
assert any("revoke skipped" in e for e in app._session_log)
|
||||||
|
|
||||||
|
|
||||||
|
def test_tui_reissue_revokes_when_cert_file_present(monkeypatch):
|
||||||
|
mocks = _patch_workflow(monkeypatch, cert_file_present=True)
|
||||||
|
app = CursesApp()
|
||||||
|
assert app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True) is True
|
||||||
|
mocks["revoke_issued"].assert_called_once()
|
||||||
|
mocks["gen_crl"].assert_called_once()
|
||||||
|
mocks["copy_crl"].assert_called_once()
|
||||||
|
mocks["build_client_full"].assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_tui_skipped_revoke_build_failure_does_not_claim_revocation(monkeypatch):
|
||||||
|
# Nothing was revoked, so the "has been revoked" warning would be a lie.
|
||||||
|
import openvpncertupdate as m
|
||||||
|
_patch_workflow(monkeypatch, cert_file_present=False)
|
||||||
|
monkeypatch.setattr("openvpncertupdate.build_client_full",
|
||||||
|
MagicMock(side_effect=m.EasyRSAError("boom")))
|
||||||
|
app = CursesApp()
|
||||||
|
shown = []
|
||||||
|
monkeypatch.setattr(CursesApp, "_error",
|
||||||
|
lambda self, stdscr, text: shown.append(text))
|
||||||
|
app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
||||||
|
assert shown and "has been revoked" not in shown[0]
|
||||||
@@ -4,6 +4,9 @@ from unittest.mock import MagicMock, patch, call
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from openvpncertupdate import CliRunner, _build_parser, main
|
from openvpncertupdate import CliRunner, _build_parser, main
|
||||||
|
# Bound before _patch_issue() rebinds the module attribute, so tests can put
|
||||||
|
# the real filesystem check back and exercise the wiring end to end.
|
||||||
|
from openvpncertupdate import has_issued_cert as real_has_issued_cert
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
@@ -13,6 +16,9 @@ from openvpncertupdate import CliRunner, _build_parser, main
|
|||||||
def _patch_issue(monkeypatch, ovpn_path="/out/cn_2026-01-01/client.ovpn",
|
def _patch_issue(monkeypatch, ovpn_path="/out/cn_2026-01-01/client.ovpn",
|
||||||
one_time_url="https://cg.example.com/#/note/abc/xyz"):
|
one_time_url="https://cg.example.com/#/note/abc/xyz"):
|
||||||
"""Patch all side-effectful callables used by CliRunner._issue."""
|
"""Patch all side-effectful callables used by CliRunner._issue."""
|
||||||
|
# Default to "the issued .crt is there", the normal case; the tests that
|
||||||
|
# care about a missing cert file override this.
|
||||||
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=True))
|
||||||
monkeypatch.setattr("openvpncertupdate.revoke_issued", MagicMock())
|
monkeypatch.setattr("openvpncertupdate.revoke_issued", MagicMock())
|
||||||
monkeypatch.setattr("openvpncertupdate.gen_crl", MagicMock())
|
monkeypatch.setattr("openvpncertupdate.gen_crl", MagicMock())
|
||||||
monkeypatch.setattr("openvpncertupdate.copy_crl", MagicMock())
|
monkeypatch.setattr("openvpncertupdate.copy_crl", MagicMock())
|
||||||
@@ -108,6 +114,70 @@ def test_create_does_not_touch_crl(monkeypatch, capsys):
|
|||||||
mocks["copy_crl"].assert_not_called()
|
mocks["copy_crl"].assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_skips_revoke_when_cert_file_missing(monkeypatch, capsys):
|
||||||
|
# An index.txt copied from an older EasyRSA install lists V-status certs
|
||||||
|
# whose .crt was never carried over. EasyRSA reads the serial out of the
|
||||||
|
# .crt, so revoke-issued can only fail — issue the replacement instead.
|
||||||
|
mocks = _patch_issue(monkeypatch)
|
||||||
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||||
|
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||||
|
mocks["revoke_issued"].assert_not_called()
|
||||||
|
mocks["gen_crl"].assert_not_called()
|
||||||
|
mocks["copy_crl"].assert_not_called()
|
||||||
|
mocks["build_client_full"].assert_called_once()
|
||||||
|
assert mocks["build_client_full"].call_args.args[2] == "y.kuts"
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_warns_when_revoke_skipped(monkeypatch, capsys):
|
||||||
|
mocks = _patch_issue(monkeypatch)
|
||||||
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||||
|
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||||
|
err = capsys.readouterr().err
|
||||||
|
assert "nothing to revoke for y.kuts" in err
|
||||||
|
assert "issued/y.kuts.crt" in err
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_skipped_revoke_build_failure_does_not_claim_revocation(monkeypatch, capsys):
|
||||||
|
# Nothing was revoked, so the "has been revoked but no new cert" warning
|
||||||
|
# would be a lie here.
|
||||||
|
import openvpncertupdate
|
||||||
|
_patch_issue(monkeypatch)
|
||||||
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||||
|
monkeypatch.setattr("openvpncertupdate.build_client_full",
|
||||||
|
MagicMock(side_effect=openvpncertupdate.EasyRSAError("boom")))
|
||||||
|
with pytest.raises(SystemExit):
|
||||||
|
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||||
|
assert "has been revoked" not in capsys.readouterr().err
|
||||||
|
|
||||||
|
|
||||||
|
def _reissue_against_real_pki(monkeypatch, tmp_path, cert_files):
|
||||||
|
"""Run --reissue with the real has_issued_cert against a temp PKI layout."""
|
||||||
|
issued = tmp_path / "issued"
|
||||||
|
issued.mkdir()
|
||||||
|
for name in cert_files:
|
||||||
|
(issued / name).write_text("-----BEGIN CERTIFICATE-----\n")
|
||||||
|
mocks = _patch_issue(monkeypatch)
|
||||||
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert", real_has_issued_cert)
|
||||||
|
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
||||||
|
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||||
|
return mocks
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_reads_pki_dir_and_skips_revoke_for_missing_crt(monkeypatch, capsys, tmp_path):
|
||||||
|
# issued/ exists but holds other people's certs — exactly the server state.
|
||||||
|
mocks = _reissue_against_real_pki(
|
||||||
|
monkeypatch, tmp_path, ["ivan.radchenko.crt", "s.krasota.crt"])
|
||||||
|
mocks["revoke_issued"].assert_not_called()
|
||||||
|
mocks["build_client_full"].assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_reissue_reads_pki_dir_and_revokes_when_crt_present(monkeypatch, capsys, tmp_path):
|
||||||
|
mocks = _reissue_against_real_pki(monkeypatch, tmp_path, ["y.kuts.crt"])
|
||||||
|
mocks["revoke_issued"].assert_called_once()
|
||||||
|
mocks["gen_crl"].assert_called_once()
|
||||||
|
mocks["build_client_full"].assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
def test_reissue_continues_building_when_crl_regen_fails(monkeypatch, capsys):
|
def test_reissue_continues_building_when_crl_regen_fails(monkeypatch, capsys):
|
||||||
import openvpncertupdate
|
import openvpncertupdate
|
||||||
mocks = _patch_issue(monkeypatch)
|
mocks = _patch_issue(monkeypatch)
|
||||||
|
|||||||
@@ -5,14 +5,28 @@ from unittest.mock import patch, MagicMock
|
|||||||
from openvpncertupdate import (
|
from openvpncertupdate import (
|
||||||
revoke_issued, build_client_full, gen_crl, copy_crl, EasyRSAError,
|
revoke_issued, build_client_full, gen_crl, copy_crl, EasyRSAError,
|
||||||
is_ca_key_encrypted, resolve_ca_passphrase,
|
is_ca_key_encrypted, resolve_ca_passphrase,
|
||||||
|
has_issued_cert, issued_cert_path,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def ok_result():
|
def ok_result():
|
||||||
r = MagicMock(); r.returncode = 0; r.stderr = ""; return r
|
r = MagicMock(); r.returncode = 0; r.stdout = ""; r.stderr = ""; return r
|
||||||
|
|
||||||
def err_result():
|
def err_result(stdout="", stderr="oops"):
|
||||||
r = MagicMock(); r.returncode = 1; r.stderr = "oops"; return r
|
r = MagicMock(); r.returncode = 1; r.stdout = stdout; r.stderr = stderr; return r
|
||||||
|
|
||||||
|
|
||||||
|
# EasyRSA's print() is `printf '%s\n'` -> stdout, and both user_error() and
|
||||||
|
# die() route through it, so this is what a real failure looks like on the wire.
|
||||||
|
EASYRSA_USER_ERROR = """
|
||||||
|
EasyRSA version 3.2.6
|
||||||
|
|
||||||
|
Error
|
||||||
|
-----
|
||||||
|
Unable to revoke as no certificate was found.
|
||||||
|
Certificate was expected at:
|
||||||
|
* /etc/easy-rsa/pki/issued/alice.crt
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
@patch("openvpncertupdate.subprocess.run")
|
@patch("openvpncertupdate.subprocess.run")
|
||||||
@@ -43,6 +57,43 @@ def test_revoke_raises_on_failure(mock_run):
|
|||||||
revoke_issued("/er", "/pki", "alice", "")
|
revoke_issued("/er", "/pki", "alice", "")
|
||||||
|
|
||||||
|
|
||||||
|
@patch("openvpncertupdate.subprocess.run")
|
||||||
|
def test_error_reports_easyrsa_stdout_diagnostics(mock_run):
|
||||||
|
# EasyRSA writes its diagnostics to stdout; reporting stderr alone left the
|
||||||
|
# user with a blank error box.
|
||||||
|
mock_run.return_value = err_result(stdout=EASYRSA_USER_ERROR, stderr="")
|
||||||
|
with pytest.raises(EasyRSAError) as exc:
|
||||||
|
revoke_issued("/er", "/pki", "alice", "")
|
||||||
|
msg = str(exc.value)
|
||||||
|
assert "Unable to revoke as no certificate was found." in msg
|
||||||
|
assert "* /etc/easy-rsa/pki/issued/alice.crt" in msg
|
||||||
|
|
||||||
|
|
||||||
|
@patch("openvpncertupdate.subprocess.run")
|
||||||
|
def test_error_strips_banner_and_blank_padding(mock_run):
|
||||||
|
mock_run.return_value = err_result(stdout=EASYRSA_USER_ERROR, stderr="")
|
||||||
|
with pytest.raises(EasyRSAError) as exc:
|
||||||
|
revoke_issued("/er", "/pki", "alice", "")
|
||||||
|
lines = str(exc.value).splitlines()
|
||||||
|
assert "" not in lines # no blank padding
|
||||||
|
assert not any(l.startswith("EasyRSA version") for l in lines)
|
||||||
|
assert "-----" not in lines # no rule under "Error"
|
||||||
|
|
||||||
|
|
||||||
|
@patch("openvpncertupdate.subprocess.run")
|
||||||
|
def test_error_keeps_both_streams(mock_run):
|
||||||
|
# openssl failures land on stderr while EasyRSA's die() text lands on stdout.
|
||||||
|
mock_run.return_value = err_result(
|
||||||
|
stdout="Easy-RSA error:\n\nFailed to revoke certificate.",
|
||||||
|
stderr="unable to load CA private key",
|
||||||
|
)
|
||||||
|
with pytest.raises(EasyRSAError) as exc:
|
||||||
|
revoke_issued("/er", "/pki", "alice", "")
|
||||||
|
msg = str(exc.value)
|
||||||
|
assert "Failed to revoke certificate." in msg
|
||||||
|
assert "unable to load CA private key" in msg
|
||||||
|
|
||||||
|
|
||||||
@patch("openvpncertupdate.subprocess.run")
|
@patch("openvpncertupdate.subprocess.run")
|
||||||
def test_build_client_full_args(mock_run):
|
def test_build_client_full_args(mock_run):
|
||||||
mock_run.return_value = ok_result()
|
mock_run.return_value = ok_result()
|
||||||
@@ -99,6 +150,30 @@ def test_copy_crl_restorecon_failure_is_non_fatal(mock_chmod, mock_copy, mock_ru
|
|||||||
mock_chmod.assert_called_once()
|
mock_chmod.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# has_issued_cert
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_issued_cert_path(tmp_path):
|
||||||
|
assert issued_cert_path("/pki", "y.kuts") == "/pki/issued/y.kuts.crt"
|
||||||
|
|
||||||
|
|
||||||
|
def test_has_issued_cert_true_when_file_present(tmp_path):
|
||||||
|
issued = tmp_path / "issued"
|
||||||
|
issued.mkdir()
|
||||||
|
(issued / "y.kuts.crt").write_text("-----BEGIN CERTIFICATE-----\n")
|
||||||
|
assert has_issued_cert(str(tmp_path), "y.kuts") is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_has_issued_cert_false_when_index_lists_cert_but_file_is_gone(tmp_path):
|
||||||
|
(tmp_path / "issued").mkdir()
|
||||||
|
assert has_issued_cert(str(tmp_path), "y.kuts") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_has_issued_cert_false_when_issued_dir_missing(tmp_path):
|
||||||
|
assert has_issued_cert(str(tmp_path), "y.kuts") is False
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# is_ca_key_encrypted
|
# is_ca_key_encrypted
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user