Résumé

Nginx UI OTP secure-session step-up is missing on sensitive mutation routes

Détails de l’avis

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:

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:

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():

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:

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:

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)
}
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:

g := root.Group("/", middleware.AuthRequired(), middleware.Proxy())

but they do not add middleware.RequireSecureSession().

Automatic backup management

api/backup/router.go protects manual restore:

r.POST("/restore", middleware.AuthRequired(), middleware.RequireSecureSession(), middleware.EncryptedForm(), RestoreBackup)

but the automatic backup management router has no secure-session middleware:

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:

"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:

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():

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:

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:

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:

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:

record, err := svc.CreateRecord(c.Request.Context(), domainID, toRecordInput(payload))
...
record, err := svc.UpdateRecord(c.Request.Context(), domainID, recordID, toR

Références