fix(auth): enforce PAM account stack and default session cookie to Secure

cmd/server/service.go

@@ -316,7 +316,10 @@ skipConfigPrompt:
 		certLine = "# tls_cert: /var/lib/nadir/tls/cert.pem"
 		keyLine = "# tls_key:  /var/lib/nadir/tls/key.pem"
 	} else if isTrustProxy {
-		secureTLSVal = "false"
+		// The proxy terminates TLS, so the browser<->proxy leg is still HTTPS -
+		// the cookie stays Secure. Only the proxy<->nadir hop is plaintext, and
+		// the browser never sees it.
+		secureTLSVal = "true"
 		trustProxyLine = "trust_proxy: true"
 		certLine = "# tls_cert: /var/lib/nadir/tls/cert.pem"
 		keyLine = "# tls_key:  /var/lib/nadir/tls/key.pem"

internal/auth/pam.go

@@ -2,6 +2,14 @@ package auth
 
 import "github.com/msteinert/pam"
 
+// Authenticate verifies a username/password against the nadir PAM service.
+//
+// Both stacks are run. Authenticate checks the password; AcctMgmt runs the
+// `account` stack (pam_unix), which is what enforces account and password
+// expiry - the shadow sp_expire/sp_max fields an admin sets with `chage -E` or
+// `usermod --expiredate` to disable an account without deleting it. Skipping it
+// would let a disabled account keep logging in here long after ssh and console
+// reject it.
 func Authenticate(username, password string) error {
 	t, err := pam.StartFunc(PAMService, username, func(s pam.Style, msg string) (string, error) {
 		if s == pam.PromptEchoOff {
@@ -13,6 +21,10 @@ func Authenticate(username, password string) error {
 	if err != nil {
 		return err
 	}
+	// No explicit close: msteinert/pam ends the transaction via a finalizer.
 
-	return t.Authenticate(0)
+	if err := t.Authenticate(0); err != nil {
+		return err
+	}
+	return t.AcctMgmt(0)
 }

internal/config/config.go

@@ -69,10 +69,13 @@ type Server struct {
 }
 
 // SecureCookie reports whether the session cookie should carry the Secure
-// attribute, defaulting to false when server.secure_tls is omitted (plain HTTP).
+// attribute. An omitted server.secure_tls defaults to true: the safe default is
+// a cookie that only travels over HTTPS, so a config that forgets the key can't
+// silently ship a session cookie a plaintext request would leak. Local
+// plain-HTTP development opts out explicitly with secure_tls: false.
 func (f *File) SecureCookie() bool {
 	if f.Server.SecureTLS == nil {
-		return false
+		return true
 	}
 	return *f.Server.SecureTLS
 }

internal/config/config_test.go

@@ -30,13 +30,15 @@ func mods() []module.Module {
 	}
 }
 
-func TestSecureCookieDefaultsFalse(t *testing.T) {
-	if (&File{}).SecureCookie() {
-		t.Error("omitted secure_tls should default to false")
+func TestSecureCookieDefaultsTrue(t *testing.T) {
+	// Fail closed: a config that omits secure_tls must still mark the session
+	// cookie Secure, so a forgotten key can't leak it over plaintext.
+	if !(&File{}).SecureCookie() {
+		t.Error("omitted secure_tls should default to true")
 	}
-	yes := true
-	if !(&File{Server: Server{SecureTLS: &yes}}).SecureCookie() {
-		t.Error("secure_tls: true should enable the Secure flag")
+	no := false
+	if (&File{Server: Server{SecureTLS: &no}}).SecureCookie() {
+		t.Error("secure_tls: false should clear the Secure flag")
 	}
 }