diff --git a/docs/deployment.md b/docs/deployment.md index b0e61a3..64fc6ef 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -193,6 +193,24 @@ Gitea uses its own SQLite database at After TLS is enabled, use the equivalent `https://` URLs without the temporary HTTP test port. +### Final identity hardening + +After public DNS, WAN TCP 80/443 forwarding, and the Let's Encrypt certificate +are working, configure Microsoft Entra ID as Gitea's OpenID Connect +authentication source. Register the final callback URL: + +```text +https://GITEA_DOMAIN/user/oauth2/entra/callback +``` + +Require assignment to the Entra enterprise application, apply the company's +Duo-backed Conditional Access policy, and map the Entra `Gitea.User` and +`Gitea.Admin` application roles into Gitea. Verify an Entra administrator can +sign in and reach the broker before disabling Gitea's local password sign-in +form. Keep a documented VM-console recovery procedure and retain the package +reader and publisher as non-admin service accounts with narrowly scoped +tokens. + For local testing that bypasses Nginx, set `BROKER_DIRECT_PORT` and `GITEA_DIRECT_PORT` in `.env`. Set `GITEA_DIRECT_ROOT_URL` to the exact URL the test browser will use, including the VM hostname or IP address and port: diff --git a/internal/app/authorization.go b/internal/app/authorization.go index 8df5775..ab82907 100644 --- a/internal/app/authorization.go +++ b/internal/app/authorization.go @@ -874,3 +874,12 @@ func (s *Server) audit( } var errAuthorizationDenied = errors.New("authorization denied") + +var ( + errCodeNotFound = fmt.Errorf("%w: code not found", errAuthorizationDenied) + errCodeExpired = fmt.Errorf("%w: code expired", errAuthorizationDenied) + errCodeRevoked = fmt.Errorf("%w: code revoked", errAuthorizationDenied) + errActionNotAuthorized = fmt.Errorf("%w: action not authorized", errAuthorizationDenied) + errPackageNotAuthorized = fmt.Errorf("%w: package not authorized", errAuthorizationDenied) + errHostLimitReached = fmt.Errorf("%w: host limit reached", errAuthorizationDenied) +) diff --git a/internal/app/download.go b/internal/app/download.go index e3b73f9..113bd31 100644 --- a/internal/app/download.go +++ b/internal/app/download.go @@ -77,8 +77,12 @@ func (s *Server) handleExchange(w http.ResponseWriter, r *http.Request) { status = http.StatusForbidden message = "authorization is invalid, expired, revoked, or at its host limit" } - _ = s.audit(r.Context(), "code_exchange_failed", "", nil, - request.Hostname, "", sourceIP, err.Error()) + var failedAuthorizationID *uint64 + if authorizationID != 0 { + failedAuthorizationID = &authorizationID + } + _ = s.audit(r.Context(), "code_exchange_failed", "", failedAuthorizationID, + request.Hostname, "", sourceIP, codeExchangeFailureDetails(request.Code, err)) writeJSON(w, status, map[string]string{"error": message}) return } @@ -110,6 +114,55 @@ func (s *Server) exchangeRateLimited(r *http.Request, sourceIP string) (bool, er return attempts >= 10, err } +func codeExchangeFailureDetails(code string, err error) string { + reason := "internal_error" + switch { + case errors.Is(err, errCodeNotFound): + reason = "code_not_found" + case errors.Is(err, errCodeExpired): + reason = "code_expired" + case errors.Is(err, errCodeRevoked): + reason = "code_revoked" + case errors.Is(err, errActionNotAuthorized): + reason = "action_not_authorized" + case errors.Is(err, errPackageNotAuthorized): + reason = "package_not_authorized" + case errors.Is(err, errHostLimitReached): + reason = "host_limit_reached" + case errors.Is(err, errAuthorizationDenied): + reason = "authorization_denied" + } + + format := "unexpected" + if validDeploymentCodeFormat(code) { + format = "expected" + } + hint := "unavailable" + if strings.HasPrefix(code, "TAPM-") && len(code) >= 5 { + hint = code[len(code)-5:] + } + return fmt.Sprintf( + "reason=%s code_hint=%s normalized_length=%d format=%s", + reason, hint, len(code), format, + ) +} + +func validDeploymentCodeFormat(code string) bool { + if len(code) != 16 || !strings.HasPrefix(code, "TAPM-") || code[10] != '-' { + return false + } + const allowed = "0123456789ABCDEFGHJKMNPQRSTVWXYZ" + for index, character := range code { + if index < 5 || index == 10 { + continue + } + if !strings.ContainsRune(allowed, character) { + return false + } + } + return true +} + func (s *Server) exchangeDeploymentCode( r *http.Request, request exchangeRequest, @@ -127,21 +180,31 @@ func (s *Server) exchangeDeploymentCode( var authorizationID uint64 var hostLimit int var expiresAt time.Time + var authorizationStatus string err = tx.QueryRowContext( r.Context(), - `SELECT id, host_limit, expires_at + `SELECT id, host_limit, expires_at, + CASE + WHEN revoked_at IS NOT NULL THEN 'revoked' + WHEN expires_at <= CURRENT_TIMESTAMP THEN 'expired' + ELSE 'active' + END FROM authorizations - WHERE code_hash = ? - AND revoked_at IS NULL - AND expires_at > CURRENT_TIMESTAMP`, + WHERE code_hash = ?`, codeHash[:], - ).Scan(&authorizationID, &hostLimit, &expiresAt) + ).Scan(&authorizationID, &hostLimit, &expiresAt, &authorizationStatus) if errors.Is(err, sql.ErrNoRows) { - return response, 0, errAuthorizationDenied + return response, 0, errCodeNotFound } if err != nil { return response, 0, err } + switch authorizationStatus { + case "revoked": + return response, authorizationID, errCodeRevoked + case "expired": + return response, authorizationID, errCodeExpired + } if request.RequestedAction != "" { var authorized int @@ -158,7 +221,7 @@ func (s *Server) exchangeDeploymentCode( return response, 0, err } if authorized != 1 { - return response, 0, errAuthorizationDenied + return response, authorizationID, errActionNotAuthorized } } if request.RequestedPackage != "" { @@ -176,7 +239,7 @@ func (s *Server) exchangeDeploymentCode( return response, 0, err } if authorized != 1 { - return response, 0, errAuthorizationDenied + return response, authorizationID, errPackageNotAuthorized } } @@ -197,7 +260,7 @@ func (s *Server) exchangeDeploymentCode( return response, 0, err } if hostCount >= hostLimit { - return response, 0, errAuthorizationDenied + return response, authorizationID, errHostLimitReached } result, err := tx.ExecContext( r.Context(), diff --git a/internal/app/download_test.go b/internal/app/download_test.go new file mode 100644 index 0000000..0bf0138 --- /dev/null +++ b/internal/app/download_test.go @@ -0,0 +1,109 @@ +package app + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestCodeExchangeFailureDetailsRetainsOnlySafeDiagnostics(t *testing.T) { + t.Parallel() + code := "TAPM-12345-ABCDE" + details := codeExchangeFailureDetails(code, errCodeNotFound) + + for _, expected := range []string{ + "reason=code_not_found", + "code_hint=ABCDE", + "normalized_length=16", + "format=expected", + } { + if !strings.Contains(details, expected) { + t.Errorf("details %q do not contain %q", details, expected) + } + } + if strings.Contains(details, code) || strings.Contains(details, "12345") { + t.Fatalf("details contain more of the submitted code than its hint: %q", details) + } +} + +func TestCodeExchangeFailureDetailsDoesNotHintUnrelatedInput(t *testing.T) { + t.Parallel() + details := codeExchangeFailureDetails("not-a-deployment-secret", errCodeNotFound) + if !strings.Contains(details, "code_hint=unavailable") || + !strings.Contains(details, "format=unexpected") { + t.Fatalf("unexpected details for malformed input: %q", details) + } + if strings.Contains(details, "ecret") { + t.Fatalf("details retained part of unrelated input: %q", details) + } +} + +func TestCodeExchangeFailureReasons(t *testing.T) { + t.Parallel() + tests := []struct { + err error + reason string + }{ + {errCodeExpired, "code_expired"}, + {errCodeRevoked, "code_revoked"}, + {errActionNotAuthorized, "action_not_authorized"}, + {errPackageNotAuthorized, "package_not_authorized"}, + {errHostLimitReached, "host_limit_reached"}, + } + for _, test := range tests { + if details := codeExchangeFailureDetails("TAPM-12345-ABCDE", test.err); !strings.Contains(details, "reason="+test.reason) { + t.Errorf("details %q do not contain reason %q", details, test.reason) + } + } +} + +func TestFailedExpiredCodeAttemptIsLinkedToDeployment(t *testing.T) { + server, _ := newAuthorizationTestServer(t) + code := "TAPM-12345-ABCDE" + codeHash := hashValue(code) + if _, err := server.db.Exec( + `INSERT INTO authorizations + (id, code_hash, code_hint, created_by, customer_label, host_limit, expires_at) + VALUES (42, ?, 'ABCDE', 'taiadmin', 'Acme migration', 3, ?)`, + codeHash[:], time.Now().UTC().Add(-time.Hour), + ); err != nil { + t.Fatal(err) + } + + body, err := json.Marshal(exchangeRequest{ + Code: code, + HostFingerprint: "0123456789abcdef", + Hostname: "pve01", + }) + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequest(http.MethodPost, "/api/v1/exchange", bytes.NewReader(body)) + response := httptest.NewRecorder() + server.handleExchange(response, request) + if response.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden) + } + + var authorizationID uint64 + var hostname, details string + if err := server.db.QueryRow( + `SELECT authorization_id, hostname, details + FROM audit_events + WHERE event_type = 'code_exchange_failed'`, + ).Scan(&authorizationID, &hostname, &details); err != nil { + t.Fatal(err) + } + if authorizationID != 42 || hostname != "pve01" { + t.Fatalf("audit event authorization/hostname = %d/%q", authorizationID, hostname) + } + if !strings.Contains(details, "reason=code_expired") || + !strings.Contains(details, "code_hint=ABCDE") || + strings.Contains(details, code) { + t.Fatalf("unsafe or incomplete audit details: %q", details) + } +} diff --git a/internal/app/server_test.go b/internal/app/server_test.go index 7d154e0..9ba8db2 100644 --- a/internal/app/server_test.go +++ b/internal/app/server_test.go @@ -79,9 +79,14 @@ func TestPackageAndAuditTemplatesExecute(t *testing.T) { SourceIP: "203.0.113.10", CreatedAt: time.Now(), }} - if err := templates.ExecuteTemplate(io.Discard, "audit.html", data); err != nil { + var auditOutput bytes.Buffer + if err := templates.ExecuteTemplate(&auditOutput, "audit.html", data); err != nil { t.Fatal(err) } + if !strings.Contains(auditOutput.String(), "Failed code attempts") || + !strings.Contains(auditOutput.String(), "audit_event=code_exchange_failed") { + t.Fatal("audit page does not include the failed-code-attempt shortcut") + } } func TestFleetTemplateExecutes(t *testing.T) { diff --git a/internal/app/static/app.css b/internal/app/static/app.css index c0c74b0..6dff8cf 100644 --- a/internal/app/static/app.css +++ b/internal/app/static/app.css @@ -247,6 +247,7 @@ dd { margin: 4px 0 0; overflow-wrap: anywhere; } .package-row { display: flex; justify-content: space-between; gap: 16px; padding: 14px 0; border-bottom: 1px solid var(--line); } .package-row div { display: grid; gap: 4px; } .package-row small { color: var(--muted); } +.package-row .status { align-items: center; justify-content: center; align-self: center; line-height: 1; text-align: center; } .toggle { display: flex; align-items: center; gap: 9px; } .empty, .fine-print { color: var(--muted); } .package-forms { display: grid; gap: 26px; } diff --git a/internal/app/templates/audit.html b/internal/app/templates/audit.html index 5067fba..73ade30 100644 --- a/internal/app/templates/audit.html +++ b/internal/app/templates/audit.html @@ -46,9 +46,10 @@
-Audit records are retained indefinitely. Up to the newest 250 matching events are shown.
+Failed code attempts show a reason, hostname, source IP, code length, and only the five-character code hint. Full submitted codes are never retained. Up to the newest 250 matching events are shown.
{{template "audit-rows" .}}