mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 18:31:15 +00:00
5214a4a647
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>
164 lines
14 KiB
Markdown
164 lines
14 KiB
Markdown
# 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 1–4 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.
|