azaion/admin
1
0
Fork 0
mirror of https://github.com/azaion/admin.git synced 2026-06-21 23:41:09 +00:00
Code Issues Packages Projects Releases Wiki Activity
Files
5224a1258914f9360b7e5ca4db9cccd286160749
admin/_docs/05_security/security_report.md
T
Oleksandr Bezdieniezhnykh c7b297de83
ci/woodpecker/push/01-test Pipeline failed
Details
ci/woodpecker/push/02-build-push unknown status
Details
refactor: remove deploy.cmd and update Dockerfile for health checks 
- Deleted the deploy.cmd script as it was no longer needed.
- Updated Dockerfile to include curl for health checks and added a non-root user for improved security.
- Modified health check command to use curl for better reliability.
- Adjusted docker-compose.test.yml to reflect changes in health check configuration.
- Cleaned up appsettings.json and removed unused configuration properties.
- Removed Resource entity and related requests from the codebase as part of the architectural shift.
- Updated documentation to reflect the removal of hardware binding and related endpoints.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-13 08:47:21 +03:00

121 lines
9.8 KiB
Markdown
Raw Blame History

# Security Audit Report
**Date**: 2026-05-13
**Scope**: Azaion Admin API workspace (`/Users/obezdienie001/dev/azaion/suite/admin`) — full audit triggered by autodev cycle 1 completion (AZ-513, AZ-196, AZ-183, AZ-197).
**Verdict**: **PASS_WITH_WARNINGS** (Critical: 0; High: 1 open — F-2 path traversal pre-existing; deferred to a separate ticket. F-1, F-3, D-1 closed in this audit.)
## Summary
| Severity | Count | Closed in this audit |
|----------|-------|----------------------|
| Critical | 0 | 0 |
| High | 3 | 3 — D-1 (`Newtonsoft.Json` 13.0.1 → 13.0.4), F-1 (OTA feature reverted — endpoints, service, entity, table, request DTOs, response DTO, e2e tests, master-key config field all deleted), F-3 (`RegisterDevice` reuses `RegisterUser`; UNIQUE INDEX `users_email_uidx` added) |
| Medium | 5 | 0 |
| Low | 5 | 0 |
## Open / Deferred
- **F-2** (path traversal via `dataFolder` route segment) remains open. It is pre-existing and unrelated to cycle-1 changes. Recommended: file as a separate ticket and close in a focused refactor (the fix needs both validation and a deployment-time review of `Content/` permissions). Not a blocker on this cycle.
## OWASP Top 10 (2021) Assessment
| Category | Status | Findings |
|----------|--------|----------|
| A01 Broken Access Control | **FAIL** | F-2 (open) — F-1 closed via OTA feature revert |
| A02 Cryptographic Failures | PASS_WITH_WARNINGS | F-1 closed via revert; D-1 closed via Newtonsoft bump; F-7 (open, hardening) |
| A03 Injection | PASS | — |
| A04 Insecure Design | PASS_WITH_WARNINGS | F-3 closed; F-8 (open, hardening) |
| A05 Security Misconfiguration | **FAIL** | F-5, F-6, F-13, F-9, F-11 |
| A06 Vulnerable Components | PASS | D-1 closed; 3 deprecated-but-not-vulnerable packages logged |
| A07 Auth Failures | PASS_WITH_WARNINGS | F-3 closed; F-7 + F-8 (open, hardening) |
| A08 Data Integrity Failures | PASS | OTA flow that introduced the unsigned-manifest concern was reverted |
| A09 Logging Failures | PASS_WITH_WARNINGS | F-12; no separate security audit log |
| A10 SSRF | NOT_APPLICABLE | API makes no outbound calls based on user URLs |
## Findings (severity-ranked)
| # | Severity | Category | Location | Title |
|---|----------|----------|----------|-------|
| F-1 | **High** (CLOSED via revert) | A01 / A02 | (deleted) `Program.cs` `/get-update`, `ResourceUpdateService.cs` | `/get-update` exposed plaintext `EncryptionKey` to any authenticated caller. **Resolution**: the entire OTA feature was deleted — endpoints, `IResourceUpdateService`, `ResourceColumnEncryption`, `Resource` entity, `resources` table, `EncryptionMasterKey` config field, `apiUploaderPolicy`, `ResourceUpdateTests.cs`. AZ-183 is reverted; the OTA delivery model itself is obsolete in the target architecture. |
| F-2 | **High** (open) | A01 | `ResourcesService.cs:20-25` + `Program.cs:201,213,219,224` | Path traversal via `dataFolder` route segment (pre-existing) — deferred to a separate ticket |
| F-3 | **High** (CLOSED) | A04 / A07 | `env/db/06_users_email_unique.sql`, `UserService.cs` | `users.email` lacked UNIQUE → duplicate-row race in `RegisterUser` and `RegisterDevice`. **Resolution**: added migration `env/db/06_users_email_unique.sql` (`CREATE UNIQUE INDEX users_email_uidx ON public.users (email)`); refactored `RegisterUser` to drop the check-then-insert pattern and translate `Npgsql.PostgresException(SqlState=23505)` to `BusinessException(EmailExists)`; refactored `RegisterDevice` to delegate the row insert to `RegisterUser`. |
| D-1 | **High** (CLOSED) | A06 | `Azaion.Common.csproj`, `Azaion.Services.csproj` | `Newtonsoft.Json 13.0.1` < patched line for GHSA-5crp-9r3c-p9vr — **bumped to 13.0.4** |
| F-4 | Medium | A02 | `Program.cs:158-162` | `/devices` returns plaintext device password (accepted by design — needs `Cache-Control: no-store` and Swagger trim) |
| F-5 | Medium | A05 | `ResourceUpdateService.cs:86-97` | `EncryptionMasterKey` validated lazily on first call instead of at startup |
| F-6 | Medium | A05 | `Dockerfile:1,20-25` | API container runs as root |
| F-7 | Medium | A02 / A07 | `Security.cs:11-12` | SHA-384 password hashing without per-user salt or KDF (pre-existing) |
| F-8 | Medium | A04 / A07 | `Program.cs:137-143` | No rate limiting on `/login` (pre-existing) |
| F-9 | Low | A05 | `LoginRequest.cs`, `SetUserQueueOffsetsRequest.cs` | DTOs lack `AbstractValidator<T>` |
| F-10 | Low | — | `docker-compose.test.yml`, `appsettings.test.json` | Hardcoded credentials/JWT secret in test fixtures (test-only, accepted) |
| F-11 | Low | A05 | `env/db/01_permissions.sql:2,7,12` | Placeholder DB passwords as setup template — needs header comment |
| F-12 | Low | A09 | `ResourcesService.cs:63` | Unstructured `LogInformation($"...")` defeats Serilog property capture |
| F-13 | Low | A02 | `Program.cs` | No HTTPS enforcement / HSTS in code (assumed at reverse proxy) |
Detailed evidence and remediation steps for each finding are in `static_analysis.md` (F-N) and `dependency_scan.md` (D-N).
## Cycle-1 Specific Verdict
The four cycle-1 tasks introduced one **new** High finding (F-1) and amplified one pre-existing High (F-3). Both were closed before deploy:
- **F-1 (resolved by revert)**: `/get-update` and `/resources/publish` are deleted; the entire OTA feature (AZ-183) is reverted. The user assessment was that the feature is itself a leftover from the installer-shipping era and is no longer needed in the target architecture (browser-only SaaS + fTPM-secured Jetsons).
- **F-3 (resolved)**: `RegisterDevice` now delegates to `RegisterUser`; `users.email` has a UNIQUE INDEX; `RegisterUser` translates UNIQUE-violation to `EmailExists`. The duplicate-row race is closed atomically.
- AZ-513 (`/classes` CRUD) and AZ-197 (hardware removal) introduce no new security findings. AZ-197 closes the prior `Hardware Fingerprint Binding` section of `security_approach.md` — the corresponding code, error codes, and DTOs are all gone.
## Dependency Vulnerabilities
| Package | CVE / Advisory | Severity | Status |
|---------|---------------|----------|--------|
| `Newtonsoft.Json` 13.0.1 → 13.0.4 | GHSA-5crp-9r3c-p9vr (DoS via deeply nested JSON) | High | **Closed in this audit** |
`dotnet list package --vulnerable` returns clean for all five projects against the configured NuGet feeds. Three deprecated-but-not-vulnerable packages are tracked as forward-looking hygiene items in `dependency_scan.md` (`FluentValidation.AspNetCore` 11.3.0, `System.IdentityModel.Tokens.Jwt` 7.1.2, `xunit` 2.x).
## Recommendations
### Closed in this audit
- **F-1**: OTA feature reverted in full (see "Findings" above).
- **F-3**: UNIQUE INDEX added; `RegisterUser`/`RegisterDevice` consolidated; UNIQUE-violation translated to `EmailExists`.
- **D-1**: `Newtonsoft.Json` bumped to 13.0.4.
### Immediate (Critical / High — open)
1. **F-2** (deferred): Add `dataFolder` validation (`[A-Za-z0-9_-]+` only, plus `Path.GetFullPath(combined).StartsWith(...)` post-check) in `ResourcesService.GetResourceFolder`. Pre-existing finding; file as a separate ticket.
### Short-term (Medium)
4. **F-4**: Add `Cache-Control: no-store, no-cache` to the `/devices` response and document the operator runbook entry (no body logging at the reverse proxy).
5. **F-5**: Validate `ResourcesConfig.EncryptionMasterKey` at startup in non-Development environments.
6. **F-6**: Add `USER app` to the final stage of `Dockerfile`; verify `Content/` and `logs/` are writable to `app` (UID 1654).
7. **F-7**: Migrate password hashing to Argon2id with per-user salt (rolling rehash on next login).
8. **F-8**: Enable ASP.NET Core 10 rate limiter on `/login` (e.g., 10 req/IP/min).
### Long-term (Low / hardening)
9. **F-9**: Add validators for `LoginRequest`, `SetUserQueueOffsetsRequest`.
10. **F-11**: Add a header comment to `env/db/01_permissions.sql` flagging it as a template, or rename to `*.example.sql`.
11. **F-12**: Convert the one unstructured log line in `ResourcesService.SaveResource` to structured form.
12. **F-13**: Document the upstream HTTPS / HSTS / security-header chain in a deployment runbook; consider `app.UseHsts()` once the chain is documented.
13. **CI gate**: introduce a build that runs `dotnet build && dotnet test && dotnet list package --vulnerable` and fails the pipeline on any vulnerability finding.
## Verdict Logic
- **PASS_WITH_WARNINGS** because exactly one pre-existing **High** finding remains open (F-2 path traversal). The two cycle-1-attributable Highs (F-1 regression, F-3 amplified) and the dependency High (D-1) were all closed during the audit.
## Tracker Follow-Ups
The following items should be filed as separate Jira tasks in the AZ project (per `.cursor/rules/tracker.mdc`). Do not write to the tracker as part of this audit — surface them to the user for prioritization first.
| Ticket | Title | Points |
|--------|-------|--------|
| [AZ-516](https://denyspopov.atlassian.net/browse/AZ-516) | F-2: Sanitize `dataFolder` route segment to prevent path traversal | 3 |
| [AZ-517](https://denyspopov.atlassian.net/browse/AZ-517) | F-4: Harden `/devices` response (Cache-Control, runbook) | 2 |
| [AZ-518](https://denyspopov.atlassian.net/browse/AZ-518) | F-6: Run admin API container as non-root | 2 |
| [AZ-519](https://denyspopov.atlassian.net/browse/AZ-519) | F-7: Migrate password hashing to Argon2id with per-user salt | 5 |
| [AZ-520](https://denyspopov.atlassian.net/browse/AZ-520) | F-8: Add rate limiting to `/login` endpoint | 2 |
| [AZ-521](https://denyspopov.atlassian.net/browse/AZ-521) | Low-severity security hygiene bundle (F-9, F-11, F-12, F-13) | 3 |
**Closed in cycle 1 (no ticket needed)**:
- F-1 — OTA feature deleted end-to-end (AZ-183 reverted).
- F-3 — UNIQUE INDEX migration `env/db/06_users_email_unique.sql` + `RegisterUser`/`RegisterDevice` consolidation.
- D-1 — `Newtonsoft.Json` bumped to 13.0.4.
- F-5 — `EncryptionMasterKey` config field deleted along with the OTA feature; the lazy-validation surface no longer exists.
Reference in New Issue View Git Blame Copy Permalink