docs(sprint-10): plan flip verify_tls default to false (redteam-friendly)
This commit is contained in:
148
tasks/todo.md
148
tasks/todo.md
@@ -1,73 +1,115 @@
|
||||
# Sprint 9 — UI: engagement 2-col + global contrast pass
|
||||
# Sprint 10 — C2 TLS verify: redteam-friendly defaults + warning suppression
|
||||
|
||||
**Base**: `sprint/8-c2` (sprint 8 not yet merged on origin/main, but its `C2ConfigCard` is the right pane).
|
||||
**Scope**: frontend-only. No backend, no schema. No new features.
|
||||
**Base**: `origin/main` (PR #11 merged — sprint 8 + 9 are in).
|
||||
**Branch**: `sprint/10-c2-tls-default`.
|
||||
**Scope**: tiny correctness sprint. No new feature. Flip a security-relevant default to match the actual operator usage of this tool.
|
||||
|
||||
---
|
||||
|
||||
## Symptom (user report)
|
||||
|
||||
> "Impossible de se connecter à mon C2 à cause de la vérification du certificat SSL (self signed)"
|
||||
|
||||
Operator opens C2 config card, fills URL + token, hits **Test connection** without noticing the checkbox → SSL VERIFY FAILED against self-signed Mythic.
|
||||
|
||||
## Root cause (workflow diagnosis confirmed)
|
||||
|
||||
The `verify_tls` chain is **fully intact** end-to-end :
|
||||
React `verifyTls` state → `C2ConfigInput.verify_tls` → PUT body → `C2Config.verify_tls` column → `cfg.verify_tls` in API loader → `get_adapter(verify_tls=)` → `MythicAdapter._verify` → `requests.post(verify=self._verify)`.
|
||||
|
||||
The bug is the **default** at every layer is `True` :
|
||||
- `backend/app/models/c2_config.py:22` — `default=True`
|
||||
- `backend/migrations/versions/0006_c2_layer.py:29` — `server_default=sa.true()`
|
||||
- `backend/app/api/c2.py:87` — `data.get("verify_tls", True)`
|
||||
- `backend/app/services/c2/mythic.py:116` — `verify_tls: bool = True`
|
||||
- `backend/app/services/c2/factory.py:9` — `verify_tls=True`
|
||||
- `frontend/src/components/C2ConfigCard.tsx:27` — `useState(true)`
|
||||
- `frontend/src/components/C2ConfigCard.tsx:74` — reset on delete `setVerifyTls(true)`
|
||||
|
||||
Mimic is a BAS / red-team lab tool ; the dominant case is a **self-signed Mythic instance**, not a publicly-trusted cert chain. Defaulting `verify=True` is hostile to the actual workflow.
|
||||
|
||||
Secondary defect : `urllib3.exceptions.InsecureRequestWarning` is never suppressed (zero hits for `disable_warnings` / `urllib3` in `backend/`). When operators correctly uncheck verify, stderr gets spammed once per HTTP call.
|
||||
|
||||
---
|
||||
|
||||
## Decisions (locked)
|
||||
|
||||
1. **Engagement page** : passer en **2 colonnes** sur desktop (`lg:grid-cols-2`), `[engagement form | C2ConfigCard]`. Mobile/tablet : stack vertical (comportement actuel).
|
||||
2. **Contraste global** : le problème est que `canvas` (page bg) et `paper` (card bg) sont **tous deux `#ffffff`** en light mode. Les cartes ne ressortent que par leur hairline 1px → fatigue oculaire confirmée par l'utilisateur.
|
||||
3. **Fix retenu** : **tinter le canvas light** d'un neutre froid très pâle. `paper` reste blanc pur. Les cartes "lèvent" naturellement sans casser le brutalisme.
|
||||
- Proposition : `canvas` light `#f3f5f8` (gris-bleu très pâle, cohérent avec l'electric blue), `paper` light `#ffffff`.
|
||||
- Dark mode **inchangé** (`canvas #111827` / `paper #1f2937` déjà différenciés).
|
||||
4. **Pas de shadow**, pas de radius. La brutalité reste intacte — seul le contraste de surface change.
|
||||
5. **Hairline** : à vérifier sur le nouveau canvas. Si nécessaire, passer `hairline` light de la valeur actuelle à un poil plus sombre pour préserver la lisibilité du bord sur tinted canvas. Mais éviter si la lecture est déjà bonne.
|
||||
1. **Flip default to `False` at every layer** — model, API fallback, adapter, factory, React state, delete reset.
|
||||
2. **New migration 0008** : flip `server_default` to `sa.false()`. Existing rows are NOT mutated (their stored boolean is preserved).
|
||||
3. **Suppress `urllib3.InsecureRequestWarning` ONLY when `verify_tls=False`** — gated inside `MythicAdapter.__init__`. Keeps the warning live for any future code that legitimately verifies.
|
||||
4. **Add helper text under checkbox** : "Leave unchecked for lab Mythic with self-signed certificates." Operators see why the box matters.
|
||||
5. **No API contract change** — `C2ConfigInput.verify_tls: boolean` stays required. The fallback in `data.get("verify_tls", False)` only matters for hand-crafted requests.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Don't touch existing C2 endpoints behavior (route paths, payload shapes).
|
||||
- Don't change the `verify_tls` field type or remove the column.
|
||||
- Don't change the FakeAdapter (it makes no HTTP calls).
|
||||
|
||||
---
|
||||
|
||||
## Task A — EngagementFormPage 2-col
|
||||
|
||||
**File** : `frontend/src/pages/EngagementFormPage.tsx`
|
||||
|
||||
- Remplacer le wrapper `<div className="flex flex-col gap-xl max-w-2xl">` par un container plus large + grid 2-col responsive.
|
||||
- Header reste en haut, full width.
|
||||
- Body : `grid grid-cols-1 lg:grid-cols-2 gap-xl` avec :
|
||||
- Col gauche : `<form>` engagement (déjà en `card-product`).
|
||||
- Col droite : `<C2ConfigCard>` (seulement quand `editing && canEditEngagements`).
|
||||
- Si pas en edit (création) : col droite vide → garder la grid mais que la col gauche se déploie via `lg:col-span-2` (pour pas avoir un vide à droite). Acceptable alternative : `flex` + `max-w-2xl` quand non-editing.
|
||||
- Pas de modif sur la logique de form, validation, mutations.
|
||||
|
||||
## Task B — Contrast pass (tokens)
|
||||
## Task A — Backend (backend-builder)
|
||||
|
||||
**Files** :
|
||||
- `DESIGN.md` § Surface : mettre à jour `canvas` light = `#f3f5f8`, conserver `paper` light = `#ffffff`. Documenter dans la même section que "canvas tints lift paper cards in light mode without violating brutalism".
|
||||
- Token source de vérité (Tailwind config ou CSS vars). Localiser et appliquer la même MAJ. Probablement `frontend/tailwind.config.js` ou un `frontend/src/styles/tokens.css` / `index.css`.
|
||||
- Vérifier qu'aucun composant ne hardcode `#ffffff` pour la page bg (devrait utiliser `bg-canvas`).
|
||||
- Tests CSS smoke : `bg-canvas` continue de matcher, dark mode inchangé.
|
||||
- `backend/app/models/c2_config.py` — line 22 : `default=True` → `default=False`
|
||||
- `backend/app/api/c2.py` — line 87 : `data.get("verify_tls", True)` → `data.get("verify_tls", False)`
|
||||
- `backend/app/services/c2/factory.py` — line 9 : `verify_tls: bool = True` → `verify_tls: bool = False`
|
||||
- `backend/app/services/c2/mythic.py` — line 116 : `verify_tls: bool = True` → `verify_tls: bool = False`, AND add at top of file `import urllib3` + `from urllib3.exceptions import InsecureRequestWarning`, AND inside `__init__` after `self._verify = verify_tls`:
|
||||
```python
|
||||
if not verify_tls:
|
||||
urllib3.disable_warnings(InsecureRequestWarning)
|
||||
```
|
||||
- **NEW migration** `backend/migrations/versions/0008_c2_verify_tls_default_false.py` — flip `server_default` to `sa.false()`. Use `op.batch_alter_table("c2_config")` for SQLite compatibility (Mimic uses SQLite per sprint 1 SPEC). Down-migration restores `sa.true()`.
|
||||
- **Tests** : grep `backend/tests/` for `verify_tls` and flip every assertion that presupposed the old `True` default (likely in `test_c2_config*.py` PUT-without-verify-tls tests and GET-fresh-row tests). Don't add new tests — adapt existing ones.
|
||||
|
||||
## Task C — Visual regression check
|
||||
**Constraints** :
|
||||
- `pytest` baseline 468/468 must hold (or grow ; never shrink).
|
||||
- `ruff` + `mypy --strict` clean.
|
||||
- Migration 0008 must be reversible — round-trip `alembic upgrade head` then `alembic downgrade -1` then `alembic upgrade head` must work on a fresh SQLite DB.
|
||||
- Don't restructure or refactor anything else. Minimum surface.
|
||||
|
||||
- `pnpm vitest run` clean.
|
||||
- `pnpm tsc --noEmit` clean.
|
||||
- `pnpm lint` clean.
|
||||
- Screenshots avant/après (au moins) :
|
||||
- EngagementsListPage (cards-on-canvas)
|
||||
- EngagementDetailPage
|
||||
- EngagementFormPage (edit, avec C2ConfigCard à droite)
|
||||
- SimulationFormPage (déjà 2-col sprint 7, vérifier que le tinted canvas n'écrase pas)
|
||||
- LoginPage
|
||||
- Dark mode : passe rapide pour confirmer aucune régression.
|
||||
## Task B — Frontend (frontend-builder)
|
||||
|
||||
---
|
||||
**Files** :
|
||||
- `frontend/src/components/C2ConfigCard.tsx` :
|
||||
- Line 27 : `useState(true)` → `useState(false)`
|
||||
- Line 74 (delete handler) : `setVerifyTls(true)` → `setVerifyTls(false)`
|
||||
- Under the checkbox JSX (around lines 169-182) : add a `<p>` with helper text :
|
||||
```tsx
|
||||
<p className="text-[12px] text-charcoal mt-xxs">
|
||||
Leave unchecked for lab Mythic with self-signed certificates.
|
||||
</p>
|
||||
```
|
||||
(Use the existing DESIGN.md tokens — `text-[12px] text-charcoal` matches the `hint` style on `FormField`. Confirm token name by reading neighboring components first.)
|
||||
- **Vitest** : if `C2ConfigCard.test.tsx` exists, flip any "starts checked" assertion to "starts unchecked".
|
||||
|
||||
## Sequencing
|
||||
**Constraints** :
|
||||
- `vitest` baseline 212/212 must hold.
|
||||
- `tsc --noEmit` + `eslint --max-warnings=0` clean.
|
||||
- No type change to `C2ConfigInput` (the field is already required `boolean`).
|
||||
- Visual: same row, just one extra `<p>` hint below the checkbox-label row. Same brutalist treatment, no transition.
|
||||
|
||||
1. **frontend-builder** : Task A + B + C. Une seule passe, commits atomiques.
|
||||
2. **design-reviewer** : revue visuelle après merge des commits builder. Focus :
|
||||
- Lecture confortable cards-on-tinted-canvas.
|
||||
- Hairline encore visible.
|
||||
- Dark mode inchangé.
|
||||
- Pas de régression sur components qui pourraient ré-utiliser `bg-canvas` pour autre chose (dropdowns, modals).
|
||||
## Task C — Sequencing
|
||||
|
||||
---
|
||||
Both tasks have **zero shared files**. Dispatch backend-builder + frontend-builder **in parallel**. No ordering constraint.
|
||||
|
||||
After both report green :
|
||||
- **code-reviewer** : sprint diff scan (focus : migration reversibility, urllib3 gating, no leftover hardcoded `True`).
|
||||
- **design-reviewer** : helper-text placement, token compliance, focus ring still works, no regression on the card.
|
||||
- **spec-reviewer** : verify SPEC.md § Intégration C2 still matches the new defaults (may need a one-line note about lab-mode default ; check before editing).
|
||||
|
||||
## Definition of Done
|
||||
|
||||
- EngagementFormPage en édition : 2 colonnes desktop, stack mobile.
|
||||
- Page bg différencié de card bg en light mode (eyes confort).
|
||||
- Vitest + typecheck + lint verts.
|
||||
- Design-reviewer APPROVED.
|
||||
- Screenshots livrés ou écueil documenté.
|
||||
- Commits conventional, branche `sprint/9-ui-contrast`.
|
||||
- Test connection against self-signed Mythic from a freshly-created C2 config works **without unchecking anything**.
|
||||
- Existing rows are untouched (operators who saved verify_tls=true keep it until they re-save).
|
||||
- `pytest` 468/468 → 468+ (no shrink), `vitest` 212/212.
|
||||
- `ruff` + `mypy --strict` + `tsc --noEmit` + `eslint` clean.
|
||||
- Migration 0008 round-trip OK.
|
||||
- No `urllib3.InsecureRequestWarning` on stderr when `verify_tls=False`.
|
||||
- Code-reviewer + design-reviewer + spec-reviewer APPROVED.
|
||||
- PR opened on Gitea ; tasks/pr-body-sprint-10.md drafted by team-lead.
|
||||
|
||||
## Operator notes (for PR body)
|
||||
|
||||
- This sprint flips a security-relevant default. Production operators who legitimately use a publicly-trusted cert chain will need to explicitly check the box — but the dominant case in this BAS tool is lab Mythic with self-signed, so the new default matches the actual workflow.
|
||||
- Existing engagements with `verify_tls=true` already stored remain unchanged. They keep their current behaviour. If the operator reported the bug because their existing row has `verify_tls=true`, they will still need to uncheck + save once after this sprint lands.
|
||||
|
||||
Reference in New Issue
Block a user