Files
satellite-provider/_docs/05_security/security_report.md
T
Oleksandr Bezdieniezhnykh 5214a4a647 [AZ-487] [AZ-488] security: cycle 2 delta audit (PASS_WITH_WARNINGS)
Step 14 (Security Audit) for cycle 2 — delta scan against the cycle-1
baseline. Verdict remains PASS_WITH_WARNINGS; no Critical/High.

Scope: JWT auth boundary (AZ-487) and UAV multipart upload + ImageSharp
decode of attacker-controlled bytes (AZ-488). Both new packages
(JwtBearer 8.0.21, ImageSharp 3.1.11 in Services.TileDownloader)
checked.

Cycle-2 delta:
* 0 Critical / 0 High
* 2 Medium: F-AUTH-2 (iss/aud not validated — by design until admin
  team publishes values, AZ-487 § Constraints), F-UAV-1 (ImageSharp
  decode now runs on attacker-controlled bytes — mitigations
  sufficient; pin to GHSA subscribe-and-bump policy).
* 4 Low: F-AUTH-1 (DEV-ONLY secret in appsettings.Development.json —
  accepted), F-AUTH-3 (rate-limit gap extends to 401 floods — folds
  into cycle-1 I3), F-UAV-2 (JsonDocument.Parse on signature-validated
  claims — bounded by Kestrel header cap), D3 (JwtBearer shares D1
  patch line).
* 1 Informational: F-UAV-3 (reject reasons disclose gate structure —
  accepted UX trade-off; documented in contract).

OWASP refresh: A01 / A07 move from N/A (with caveat) to
PASS_WITH_WARNINGS (per-tenant authz absent; iss/aud + revocation
gaps tracked).

Pre-deploy operational gate added: deploy pipeline must verify
JWT_SECRET != DEV-ONLY placeholder before promoting api.

Artifacts: dependency_scan.md, static_analysis.md, owasp_review.md,
infrastructure_review.md, security_report.md — all appended with a
"Cycle 2 Delta" section preserving cycle-1 finding IDs.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-12 00:13:58 +03:00

164 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Security Audit Report
**Date**: 2026-05-11 (cycle 1 baseline) · 2026-05-11 cycle 2 refresh appended below
**Scope**: Satellite Provider — full repository (Api, Common, DataAccess, Services.*, Tests, infra)
**Trigger**: `/autodev` Step 14 (Security Audit) — feature cycle 1, post-AZ-484; **cycle 2 delta scan added 2026-05-11 covering AZ-487 JWT validation baseline + AZ-488 UAV tile upload endpoint**
**Verdict (current, post-cycle-2)**: **PASS_WITH_WARNINGS**
## Summary
| Severity | Cycle 1 | Cycle 2 delta | Current total |
|----------|---------|---------------|---------------|
| Critical | 0 | 0 | 0 |
| High | 0 | 0 | 0 |
| Medium | 5 | 2 (F-AUTH-2, F-DEPS-UAV) | 7 |
| Low | 5 | 4 (F-AUTH-1, F-AUTH-3, F-UAV-2, D3) | 9 |
| Info | — | 1 (F-UAV-3) | 1 |
No Critical or High findings in either cycle. The verdict remains `PASS_WITH_WARNINGS`. Cycle 2's two new Medium findings (`iss`/`aud` not validated yet; ImageSharp decode exposure widened) are both bounded by mitigations already in place and tracked as follow-ups rather than gating items. **AZ-484 (cycle 1's only feature change) introduced zero new findings** — it remained a pure data-layer change.
## OWASP Top 10:2025 Assessment
| Category | Status | Findings |
|----------|--------|----------|
| A01 Broken Access Control | N/A (with caveat) | — |
| A02 Security Misconfiguration | FAIL | S1, S2/I6, I1, I2 |
| A03 Software Supply Chain Failures | PASS_WITH_WARNINGS | D1, D2 |
| A04 Cryptographic Failures | N/A | — |
| A05 Injection | PASS | — |
| A06 Insecure Design | FAIL | S3, S4, I3 |
| A07 Authentication Failures | N/A (with caveat) | — |
| A08 Software or Data Integrity Failures | PASS | — |
| A09 Security Logging and Alerting Failures | PASS_WITH_WARNINGS | I4 |
| A10 Mishandling of Exceptional Conditions | PASS | — |
The two **N/A (with caveat)** entries (A01, A07) reflect the documented architectural choice (`architecture.md` §7) that this is an internal/trusted-network service. **The audit does not endorse that choice — it merely notes that the choice has been made deliberately.** If the deployment trust boundary ever changes, A01 and A07 immediately become FAIL and every endpoint becomes an unauthenticated surface; that decision must be re-examined before any internet-facing exposure.
## Findings
| # | Severity | Category | Location | Title |
|----|----------|------------------------|---------------------------------------------------------|--------------------------------------------------------------------------------------|
| S1 | Medium | A02 — Misconfiguration | `SatelliteProvider.Api/appsettings.json:24` | Default Postgres password (`postgres/postgres`) committed in `appsettings.json` |
| S2 | Medium | A02 — Misconfiguration | `docker-compose.yml:6-7,30` | Weak Postgres credentials in compose (mirrors S1) |
| S3 | Low | A06 — Insecure Design | `SatelliteProvider.Api/Program.cs:169,207,237` | Latitude/longitude inputs not range-validated at API boundary |
| S4 | Medium | A06 — Insecure Design | `.env` (workspace root) | Apparent real Google Maps API key on developer filesystem; no `.env.example` |
| D1 | Medium | A03 — Supply Chain | `SatelliteProvider.Api.csproj``Microsoft.AspNetCore.OpenApi 8.0.21` | CVE-2026-26130 SignalR DoS (not reachable in this app — codebase has zero SignalR use) |
| D2 | Low | A03 — Supply Chain | `SatelliteProvider.Tests.csproj``Microsoft.NET.Test.Sdk 17.8.0` | CVE-2022-30184 transitive via `NuGet.Frameworks <6.2.1` (test-only) |
| I1 | Low | A02 — Misconfiguration | `SatelliteProvider.Api/Dockerfile` | Container runs as root (no `USER` directive) |
| I2 | Low | A02 — Misconfiguration | `SatelliteProvider.Api/Program.cs` | No security headers middleware |
| I3 | Medium | A06 — Insecure Design | `SatelliteProvider.Api/Program.cs` | No inbound rate limiting on any HTTP endpoint |
| I4 | Low | A09 — Logging | `SatelliteProvider.Api/*` (logging strategy) | No security-event logs / alerting |
| I5 | Medium | A02 — Misconfiguration | `.dockerignore` + `Dockerfile:15` (`COPY . .`) | `.env` not in `.dockerignore` — risk of API key being baked into image layers |
I6 in the infra report is a duplicate of S2 (same root cause) and is not double-counted in the summary.
### Finding Details
Full evidence and remediation for every finding lives in the per-phase reports. The detail tables there are the source of truth — this top-level report intentionally avoids restating multi-paragraph remediation steps.
- **Phase 1**: `_docs/05_security/dependency_scan.md` — D1, D2, full dependency inventory + cross-version sanity check
- **Phase 2**: `_docs/05_security/static_analysis.md` — S1, S2, S3, S4, plus the categories that were checked clean (SQL injection, command injection, deserialization, path traversal, log leakage, exception leakage)
- **Phase 3**: `_docs/05_security/owasp_review.md` — OWASP Top 10:2025 per-category assessment + cross-reference table
- **Phase 4**: `_docs/05_security/infrastructure_review.md` — I1, I2, I3, I4, I5, I6, plus the items checked clean (CI secrets handling, image attribution labels)
## Dependency Vulnerabilities
| Package | CVE | Severity | Reachable? | Fix Version |
|---------|-----|----------|------------|-------------|
| Microsoft.AspNetCore.OpenApi (→ ASP.NET Core 8 runtime) | CVE-2026-26130 | High (paper) / Low (this app) | **No** — codebase has zero SignalR use | 8.0.25 |
| Microsoft.NET.Test.Sdk → NuGet.Frameworks | CVE-2022-30184 | Medium (paper) / Low (this app) | Test project only — never shipped | Microsoft.NET.Test.Sdk 17.9.0+ |
All other dependencies (`Newtonsoft.Json 13.0.4`, `SixLabors.ImageSharp 3.1.11`, `Npgsql 9.0.2`, `Dapper 2.1.35`, `Swashbuckle.AspNetCore 6.6.2`, `Serilog.AspNetCore 8.0.3`, `dbup-postgresql 6.0.3`, `Microsoft.Extensions.* 9.0.10`) are at or beyond the patched line for every CVE I could find.
## AZ-484 Cycle-Specific Verdict
The reason this audit was triggered (the AZ-484 multi-source tile storage cycle) is independently **clean**:
- Migration 013 is transactional and idempotent — no data loss / data integrity finding.
- `TileSourceConverter` enforces a closed value space at the language layer; `TileEntity.Source` is `string` only as a Dapper-bug workaround documented in `_docs/LESSONS.md` L-001.
- `TileRepository` queries continue to use parameterised Dapper — no new SQL injection surface.
- No new external dependencies, no new endpoints, no new untrusted-input flows.
- All findings in this report predate AZ-484 and are unchanged by it.
## Recommendations
### Immediate (Critical/High)
None — there are no Critical or High findings. The audit does not block the next deploy on its own merit.
### Short-term (Medium — pick before next public-network exposure or any post-deploy hardening pass)
1. **S1 + S2 + I5** — De-default DB credentials and stop shipping the .env into the build context. One coordinated change:
- Remove `ConnectionStrings:DefaultConnection` from `appsettings.json` (rely on env-var via the existing throw on null).
- Add `POSTGRES_USER` / `POSTGRES_PASSWORD` to a tracked `.env.example` and source them from a dev `.env`; bind `5432` to `127.0.0.1`.
- Append `.env` and `.env.*` (with `!.env.example` exception) to `.dockerignore`.
2. **S4** — Rotate the Google Maps API key out-of-band, add `.env.example`, add Google Cloud key restrictions (HTTP referrer or IP allowlist + per-API quotas). The audit deliberately did not echo the key value into any artifact.
3. **D1** — Bump `Microsoft.AspNetCore.OpenApi` from `8.0.21` to the current 8.0.x patch (≥ 8.0.25) and rebuild the deployed image so the vulnerable SignalR code paths are physically absent.
4. **I3** — Wire `Microsoft.AspNetCore.RateLimiting` (built into .NET 8 — no new package). Conservative starting threshold in the per-phase report.
### Long-term (Low — hardening backlog)
1. **I1** — Add a non-root `USER` to the API Dockerfile.
2. **I2** — Add a tiny security-headers middleware (or pull `NWebsec.AspNetCore.Middleware`).
3. **S3** — Add explicit lat/lon range guards at the API boundary (matches the existing `SizeMeters` 100-10000 pattern).
4. **D2** — Bump `Microsoft.NET.Test.Sdk` to ≥ 17.9.0 next time the test project's deps are touched.
5. **I4** — Defer until the trust boundary changes; if/when the API moves toward a less-trusted network, add structured 4xx logging per IP + a basic alerting rule.
## Verdict Logic
- No Critical or High findings → **not FAIL**
- 5 Medium + 5 Low findings exist → **not PASS**
- Therefore: **PASS_WITH_WARNINGS**
This satisfies the autodev gate to proceed to Step 15 (Performance Test). The recommendations above should be tracked as separate Jira tasks under a hardening epic before the first non-internal deployment.
## Self-verification
- [x] All findings from Phases 14 included
- [x] No duplicate findings (I6 explicitly noted as a duplicate of S2 and not double-counted)
- [x] Every finding has remediation guidance (in per-phase reports)
- [x] Verdict matches severity logic (no Critical/High → not FAIL; >0 findings → not PASS)
- [x] No real secret values printed in any audit artifact (S4 described without echoing the API key)
---
## Cycle 2 Delta Summary (AZ-487 + AZ-488)
### What changed in cycle 2
AZ-487 introduced a JWT validation baseline (HS256, `JWT_SECRET` env var, `.RequireAuthorization()` on every endpoint, Swagger Bearer hook). AZ-488 replaced the 501 `/api/satellite/upload` stub with a multipart batch endpoint that validates JPEGs via a 5-rule quality gate and persists accepted tiles. Two new packages were added: `Microsoft.AspNetCore.Authentication.JwtBearer 8.0.21` (Api) and `SixLabors.ImageSharp 3.1.11` (TileDownloader + Tests; consistent with the existing Api-level reference).
### Findings table (cycle-2 delta)
| # | Severity | Category | Location | Title |
|------------|---------------|------------------------------------------|-------------------------------------------------------------------------|-----------------------------------------------------------------------------|
| F-AUTH-1 | Low (accepted)| A02 — Misconfiguration | `SatelliteProvider.Api/appsettings.Development.json:14` | DEV-ONLY JWT secret committed; env-var overrides; operator must verify in prod |
| F-AUTH-2 | Medium | A07 — AuthN / Identification | `Authentication/AuthenticationServiceCollectionExtensions.cs:31-32` | `iss`/`aud` not validated (intentional — suite contract has not defined values) |
| F-AUTH-3 | Low (rec. I3) | A06 — Insecure Design | every `/api/satellite/*` endpoint | No rate limiting on 401-producing paths (extends cycle-1 I3) |
| F-UAV-1 | Medium | A03 — Supply Chain (exposure) | `Services.TileDownloader/UavTileQualityGate.cs:60-95` | ImageSharp decode now runs on attacker-controlled JPEGs (mitigations OK) |
| F-UAV-2 | Low | A07 — AuthN claim parsing | `Authentication/PermissionsRequirement.cs:84-111` | `JsonDocument.Parse` on signature-validated claim values (bounded by header cap) |
| F-UAV-3 | Informational | A06 — Insecure Design (info-disclosure) | `Services.TileDownloader/UavTileQualityGate.cs` | Reject reasons disclose gate structure (accepted UX trade-off; documented in contract) |
| D3 | Low | A03 — Supply Chain | `SatelliteProvider.Api.csproj` (new JwtBearer 8.0.21) | Shares D1 patch line; same remediation |
| F-DEPS-UAV | Medium | A03 — Supply Chain (exposure) | new ImageSharp call site in TileDownloader | Documented in dependency_scan.md cycle-2 delta |
### Verdict reconciliation
- No new Critical or High findings → cycle 2 does NOT escalate the verdict.
- Two new Medium findings — both are *follow-ups under existing remediations*, not blockers:
- F-AUTH-2 waits on the admin team defining `iss`/`aud` (already flagged in AZ-487 § Constraints).
- F-UAV-1 + F-DEPS-UAV jointly say "subscribe to ImageSharp GHSA and bump aggressively" — no immediate change needed.
- F-AUTH-1 and F-UAV-3 are explicitly accepted.
- F-AUTH-3 + D3 fold into existing cycle-1 remediations (I3 rate limiting, D1 8.0.x patch bump).
**Current verdict: PASS_WITH_WARNINGS** (cycle 2 satisfies the autodev Step-14 gate; proceed to Step 15).
### New / refreshed cycle-2 recommendations
- **Pre-deploy gate (operational, NOT code)**: `deploy/SKILL.md` must verify `JWT_SECRET` is set to a ≥ 32-byte value distinct from the DEV-ONLY placeholder. Cycle-2 deploys without this verification step are gated.
- **Coordinate with admin team**: confirm expected `iss`/`aud` values; flip `ValidateIssuer` / `ValidateAudience` to `true` as soon as those values land. Track under AZ-487 § Constraints follow-up.
- **Bump 8.0.x ASP.NET Core packages together**: the next D1 hardening commit must bump both `Microsoft.AspNetCore.OpenApi` AND `Microsoft.AspNetCore.Authentication.JwtBearer` to ≥ 8.0.25.
- **ImageSharp subscribe-and-bump policy**: add to the runbook — patch within 7 days of any `SixLabors.ImageSharp` GHSA. Reconsider sandboxing if the upload endpoint is exposed beyond the trust boundary documented in architecture.md § 7.
- **Cycle-2 hardening backlog (Low priority)**:
- Pass `JsonDocumentOptions { MaxDepth = 8 }` and a max-claim-length check to `PermissionsAuthorizationHandler.TryReadJsonArray`.
- Document in `architecture.md` that reject-reason codes are NOT a security boundary.