CVE-2026-84315: Nginx UI OTP secure-session step-up is missing on sensitive mutation routes

Reporter: Jagadesh Achanta Target: `0xJacky/nginx-ui` Affected version / commit tested: - App version: `2.3.10` - Commit: `4948fba8744e82c3d7003f68ee164e9f49421066` - Disclosure channel: GitHub Private Vulnerability Reporting - Credential reference: `cred:github-jagstack-gh-cli` ## AI assistance disclosure This report was prepared by Jagadesh Achanta with assistance from OpenAI/Codex for public source review, local evidence collection, and report drafting. ## Summary Nginx UI has a short-lived OTP/passkey "secure session" step-up boundary for users with 2FA enabled. `RequireSecureSession()` rejects an OTP-enabled user's request unless the request includes a valid `X-Secure-Session-ID` issued after a fresh OTP/passkey verification. The project already applies this step-up middleware to sensitive operations such as: - Nginx config create/edit/rename/delete; - site save/delete/enable/disable/rename/duplicate; - stream save/delete/enable/disable/rename/duplicate; - settings save and protected secret reads; - backup restore. However, several similarly sensitive authenticated mutation routes remain outside `RequireSecureSession()`. An attacker who obtains an authenticated JWT/session for a 2FA-enabled user, but does not have the victim's fresh OTP/passkey secure-session token, can still perform those operations. The affected route groups include: - automatic backup create/modify/delete/restore/test-S3 operations; - DNS credential create/edit/delete operations; - certificate and ACME user create/modify/delete/register operations; - DNS domain, record, and DDNS create/update/delete operations; - Nginx reload/restart, stub-status toggle, performance-setting update, and module-cache refresh operations; - site/stream advanced-mode toggles and batch updates; - auto-certificate binding/unbinding for sites; - site navigation order and health-check updates. This report is distinct from my earlier Nginx UI report about auto-backup path traversal. This report is about missing 2FA step-up authorization on sensitive authenticated mutation routes. ## Why this is a security boundary `internal/middleware/secure_session.go` only requires a secure-session token for users with OTP enabled. For OTP users, it reads `X-Secure-Session-ID` from the header or query string and aborts with `401 Unauthorized` if the secure session is missing or invalid: ```go if !cUser.EnabledOTP() { c.Next() return } ssid := c.GetHeader("X-Secure-Session-ID") if ssid == "" { ssid = c.Query("X-Secure-Session-ID") } if ssid == "" { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{ "message": "Secure Session ID is empty", }) return } if user.VerifySecureSessionID(ssid, cUser.ID) { c.Next() return } ``` `internal/user/otp.go` issues the secure-session ID for only 10 minutes: ```go func SetSecureSessionID(userId uint64) (sessionId string) { sessionId = uuid.NewString() cache.Set(secureSessionIDCacheKey(sessionId), userId, 10*time.Minute) return } ``` The UI/server already rely on this boundary for sensitive actions. For example, `api/config/router.go` wraps config mutations with `RequireSecureSession()`: ```go o := r.Group("", middleware.RequireSecureSession()) { o.POST("configs", AddConfig) o.POST("config", EditConfig) o.POST("config_mkdir", Mkdir) o.POST("config_rename", Rename) o.POST("config_delete", DeleteConfig) } ``` `api/sites/router.go` protects normal site mutations: ```go o := r.Group("", middleware.RequireSecureSession()) { o.POST("sites/:name/rename", RenameSite) o.POST("sites/:name/enable", EnableSite) o.POST("sites/:name/disable", DisableSite) o.POST("sites/:name", SaveSite) o.DELETE("sites/:name", DeleteSite) o.POST("sites/:name/duplicate", DuplicateSite) o.POST("sites/:name/maintenance", EnableMaintenanceSite) } ``` `api/streams/router.go` protects normal stream mutations, and `api/settings/router.go` protects secret reads/settings saves: ```go o := r.Group("", middleware.RequireSecureSession()) { o.POST("streams/:name", SaveStream) o.POST("streams/:name/rename", RenameStream) o.POST("streams/:name/enable", EnableStream) o.POST("streams/:name/disable", DisableStream) o.DELETE("streams/:name", DeleteStream) o.POST("streams/:name/duplicate", Duplicate) } ``` ```go r.GET("settings/protected", middleware.RequireSecureSession(), GetProtectedSetting) r.POST("settings", middleware.RequireSecureSession(), SaveSettings) ``` The repository's own tests also verify this expected behavior. `api/config/security_test.go`, `api/sites/security_test.go`, and `api/streams/security_test.go` each assert that OTP-enabled users receive `401 Unauthorized` when protected mutations are attempted without a secure session. ## Affected routes outside the step-up boundary The affected routes are under the authenticated private router: ```go g := root.Group("/", middleware.AuthRequired(), middleware.Proxy()) ``` but they do not add `middleware.RequireSecureSession()`. ### Automatic backup management `api/backup/router.go` protects manual restore: ```go r.POST("/restore", middleware.AuthRequired(), middleware.RequireSecureSession(), middleware.EncryptedForm(), RestoreBackup) ``` but the automatic backup management router has no secure-session middleware: ```go r.POST("/auto_backup", CreateAutoBackup) r.POST("/auto_backup/:id", ModifyAutoBackup) r.DELETE("/auto_backup/:id", DestroyAutoBackup) r.PATCH("/auto_backup/:id", RestoreAutoBackup) r.POST("/auto_backup/test_s3", TestS3Connection) ``` Those handlers create or modify backup jobs, register cron jobs, test attacker-supplied S3 settings, and store S3 credentials: ```go "s3_access_key_id": "omitempty", "s3_secret_access_key": "omitempty", ... if ctx.Model.Enabled { if err := cron.AddAutoBackupJob(ctx.Model.ID, ctx.Model.CronExpression); err != nil { ``` The model stores S3 secret material: ```go S3AccessKeyID string `json:"s3_access_key_id" gorm:"comment:S3 access key ID;serializer:json[aes]"` S3SecretAccessKey string `json:"s3_secret_access_key" gorm:"comment:S3 secret access key;serializer:json[aes]"` ``` ### Certificate, ACME, and DNS credential management `api/certificate/router.go` registers credential and certificate mutations without `RequireSecureSession()`: ```go r.POST("dns_credentials", AddDnsCredential) r.POST("dns_credentials/:id", EditDnsCredential) r.DELETE("dns_credentials/:id", DeleteDnsCredential) r.POST("certs", AddCert) r.POST("certs/:id", ModifyCert) r.DELETE("certs/:id", RemoveCert) r.PUT("cert_sync", SyncCertificate) r.POST("acme_users", CreateAcmeUser) r.POST("acme_users/:id", ModifyAcmeUser) r.POST("acme_users/:id/register", RegisterAcmeUser) r.DELETE("acme_users/:id", DestroyAcmeUser) ``` The DNS credential handlers persist provider configuration: ```go dnsCredential := model.DnsCredential{ Name: json.Name, Config: &json.Config, Provider: json.Provider, ProviderCode: providerCode, } ... err := d.Create(&dnsCredential) ``` The model serializes that configuration with AES: ```go Config *dns.Config `json:"config,omitempty" gorm:"serializer:json[aes]"` ``` ### DNS domains, records, and DDNS `api/dns/router.go` registers DNS domain, record, and DDNS mutations without secure-session middleware: ```go group.POST("/domains", CreateDomain) group.POST("/domains/:id", UpdateDomain) group.DELETE("/domains/:id", DeleteDomain) group.POST("/domains/:id/records", CreateRecord) group.PUT("/domains/:id/records/:record_id", UpdateRecord) group.DELETE("/domains/:id/records/:record_id", DeleteRecord) group.PUT("/domains/:id/ddns", UpdateDDNSConfig) group.DELETE("/domains/:id/ddns", DeleteDDNSConfig) ``` Those handlers call the DNS provider service and modify DDNS jobs: ```go record, err := svc.CreateRecord(c.Request.Context(), domainID, toRecordInput(payload)) ... record, err := svc.UpdateRecord(c.Request.Context(), domainID, recordID, toRecordInput(payload)) ... if err := cron.AddOrUpdateDDNSJob(domainID, cfg.IntervalSeconds); err != nil { ``` ### Nginx control and configuration-side effects `api/nginx/router.go` registers several server/Nginx mutation endpoints without `RequireSecureSession()`: ```go r.POST("nginx/reload", Reload) r.POST("nginx/restart", Restart) r.POST("nginx/stub_status", ToggleStubStatus) r.POST("nginx/performance", UpdatePerformanceSettings) r.POST("nginx/modules/refresh", RefreshModulesCache) ``` The handlers perform control actions or persist Nginx-related changes: ```go func Reload(c *gin.Context) { nginx.Control(nginx.Reload).Resp(c) } func Restart(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "message": "ok", }) go nginx.Restart() } ``` ```go if json.Enable { err = performance.EnableStubStatus() } else { err = performance.DisableStubStatus() } ... reloadOutput, err := nginx.Reload() ``` ```go err := performance.UpdatePerfOpt(&perfOpt) ``` ### Site and stream advanced/batch mutations `api/sites/router.go` leaves these site mutations before the secure-session group: ```go r.PUT("sites", BatchUpdateSites) r.POST("sites/:name/advance", DomainEditByAdvancedMode) r.POST("auto_cert/:name", AddDomainToAutoCert) r.DELETE("auto_cert/:name", RemoveDomainFromAutoCert) r.POST("site_navigation/order", UpdateSiteOrder) r.POST("site_navigation/health_check/:id", UpdateHealthCheck) ``` `DomainEditByAdvancedMode()` persists the site's `Advanced` mode: ```go _, err = s.Where(s.Path.Eq(path)).Update(s.Advanced, json.Advanced) ``` `AddDomainToAutoCert()` writes certificate automation settings including DNS credential ID, domains, challenge method, and ACME user: ```go err = certModel.Updates(&model.Cert{ Name: name, Domains: json.Domains, AutoCert: model.AutoCertEnabled, DnsCredentialID: json.DnsCredentialID, ChallengeMethod: json.ChallengeMethod, KeyType: helper.GetKeyType(json.KeyType), ACMEUserID: json.ACMEUserID, }) ``` `api/streams/router.go` protects normal stream save/delete operations, but leaves batch update and advanced-mode outside: ```go r.PUT("streams", BatchUpdateStreams) ... r.POST("streams/:name/advance", AdvancedEdit) ``` `AdvancedEdit()` persists the stream's `Advanced` mode: ```go _, err = s.Where(s.Path.Eq(path)).Update(s.Advanced, json.Advanced) ``` ## Safe local proof I did not access any external Nginx UI deployment. The following proof only inspects the checked-out source tree and confirms that: - `RequireSecureSession()` is a real OTP-gated 10-minute step-up mechanism; - high-risk control routes are intentionally protected elsewhere; - the affected route groups are authenticated but outside `RequireSecureSession()`; - the affected handlers perform persistent or server-control side effects. Run: ```bash python3 reports/nginx-ui-missing-secure-session-step-up-poc.py ``` Observed output: ```json { "all_affected_groups_present": true, "all_controls_present": true, "all_side_effects_present": true, "commit": "4948fba8744e82c3d7003f68ee164e9f49421066", "result": "VULNERABLE_SOURCE_CONFIRMED", "target": "0xJacky/nginx-ui" } ``` The full local proof output is available in my workspace as `tmp/nginx-ui-missing-secure-session-step-up-poc-output.txt`. ## HTTP reproduction outline This is a local-only reproduction outline for an isolated Nginx UI test instance, consistent with the project's security policy: 1. Create a test user with OTP enabled. 2. Log in as that user and keep only the normal authenticated session/JWT. 3. Do not obtain or send a fresh `X-Secure-Session-ID`. 4. Verify the expected protected control behavior: ```http POST /api/config Authorization: <otp-user-session> Content-Type: application/json { "name": "example.conf", "content": "server { listen 80; }" } ``` Expected result: `401 Unauthorized` because config edits require `RequireSecureSession()`. 5. Attempt an affected operation with the same OTP-enabled session and no `X-Secure-Session-ID`, for example: ```http POST /api/dns_credentials Authorization: <otp-user-session> Content-Type: application/json { "name": "attacker-controlled-test", "provider": "cloudflare", "provider_code": "cloudflare", "token": "test-token" } ``` Expected secure behavior: `401 Unauthorized`, matching the secure-session requirement for other sensitive configuration changes. Actual behavior from source review: the request reaches `AddDnsCredential()` because the DNS credential router is under `AuthRequired()` but does not apply `RequireSecureSession()`. The same missing step-up applies to the other route groups listed above. ## Impact If an attacker obtains the authenticated web session/JWT for a 2FA-enabled Nginx UI user, the secure-session boundary should limit what they can do until they also pass a fresh OTP/passkey challenge. The current route map leaves major administrative actions outside that boundary. Depending on the victim user's privileges and deployment configuration, this can allow the attacker to: - add, edit, or remove DNS provider credentials; - alter certificate and ACME automation; - create, modify, delete, or restore automatic backup jobs; - configure S3 backup destinations and trigger S3 connection tests; - alter DNS domains, records, or DDNS schedules through configured providers; - restart or reload Nginx; - enable/disable stub-status and change performance settings; - switch sites/streams into advanced mode or change site automation bindings. This undermines the value of the secure-session step-up for OTP-enabled accounts. I am not claiming pre-authentication access or remote code execution from this issue. The issue is missing step-up authorization for sensitive authenticated mutations. ## Suggested fix Recommended mitigation: 1. Apply `middleware.RequireSecureSession()` to all high-impact authenticated mutation routes that alter server, Nginx, certificate, DNS, backup, credential, or automation state. 2. At minimum, protect: - auto-backup create/modify/delete/restore/test-S3; - DNS credential create/edit/delete; - certificate and ACME user mutations; - DNS domain, record, and DDNS mutations; - Nginx reload/restart/stub-status/performance/module-cache mutation routes; - site/stream advanced-mode, batch, auto-cert, navigation order, and health-check mutations. 3. Add regression tests mirroring the existing config/site/stream security tests: - OTP-enabled user without `X-Secure-Session-ID` receives `401`; - OTP-enabled user with a valid secure-session ID succeeds; - non-OTP users retain existing behavior if that is the intended product policy. 4. Consider centralizing "sensitive mutation" routing to make future omissions harder.