Require the current password to change the password

server/proto/auth.go

@@ -16,6 +16,8 @@ type GetAccountRsp struct {
 type ChangePasswordReq struct {
 	Username string `json:"username" validate:"required"`
 	Password string `json:"password" validate:"required"`
+	// OldPassword is required unless the device still uses the default account.
+	OldPassword string `json:"oldPassword"`
 }
 
 type IsPasswordUpdatedRsp struct {

server/service/auth/account.go

@@ -4,14 +4,16 @@ import (
 	"NanoKVM-Server/utils"
 	"encoding/json"
 	"errors"
+	"io/fs"
 	"os"
 	"path/filepath"
 
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/crypto/bcrypt"
 )
 
-const AccountFile = "/etc/kvm/pwd"
+// AccountFile is a variable rather than a constant so tests can redirect it.
+var AccountFile = "/etc/kvm/pwd"
 
 type Account struct {
 	Username string `json:"username"`
@@ -40,6 +42,20 @@ func GetAccount() (*Account, error) {
 	return &account, nil
 }
 
+// isAccountConfigured reports whether a password has ever been set on this
+// device. Without the file, GetAccount falls back to the admin/admin default.
+//
+// Only a missing file counts as unconfigured. Any other stat error means the
+// answer is unknown, and the caller uses this to decide whether to demand the
+// current password, so an unknown answer has to keep the check in force.
+func isAccountConfigured() bool {
+	if _, err := os.Stat(AccountFile); err != nil {
+		return !errors.Is(err, fs.ErrNotExist)
+	}
+
+	return true
+}
+
 func SetAccount(username string, hashedPassword string) error {
 	account, err := json.Marshal(&Account{
 		Username: username,

server/service/auth/password.go

@@ -14,6 +14,9 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
+// setRootPassword is a variable so tests can avoid shelling out to passwd(1).
+var setRootPassword = changeRootPassword
+
 func (s *Service) ChangePassword(c *gin.Context) {
 	var req proto.ChangePasswordReq
 	var rsp proto.Response
@@ -23,6 +26,16 @@ func (s *Service) ChangePassword(c *gin.Context) {
 		return
 	}
 
+	// Require the current password, otherwise a stolen session - or a request
+	// forged by another site - is enough to take over the device permanently.
+	// A device that still has no account file uses the documented admin/admin
+	// default, so the check adds nothing and would block the initial setup.
+	if isAccountConfigured() && !CompareAccount(req.Username, req.OldPassword) {
+		time.Sleep(2 * time.Second)
+		rsp.ErrRsp(c, -6, "invalid current password")
+		return
+	}
+
 	password, err := utils.DecodeDecrypt(req.Password)
 	if err != nil || password == "" {
 		rsp.ErrRsp(c, -2, "invalid password")
@@ -41,7 +54,7 @@ func (s *Service) ChangePassword(c *gin.Context) {
 	}
 
 	// change root password
-	err = changeRootPassword(password)
+	err = setRootPassword(password)
 	if err != nil {
 		_ = DelAccount()
 		rsp.ErrRsp(c, -5, "failed to change password")

server/service/auth/password_test.go

@@ -0,0 +1,176 @@
+package auth
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"NanoKVM-Server/utils"
+
+	"github.com/gin-gonic/gin"
+	"github.com/mervick/aes-everywhere/go/aes256"
+	"golang.org/x/crypto/bcrypt"
+)
+
+// encryptPassword mirrors what the web UI sends: the password encrypted with
+// the shared key, then URL-escaped.
+func encryptPassword(password string) string {
+	return url.QueryEscape(aes256.Encrypt(password, utils.SecretKey))
+}
+
+// useTempAccountFile points the account store at a throwaway file. When
+// currentPassword is empty no account file is created, which is how a
+// factory-fresh device looks.
+func useTempAccountFile(t *testing.T, currentPassword string) string {
+	t.Helper()
+
+	original := AccountFile
+	t.Cleanup(func() { AccountFile = original })
+
+	AccountFile = filepath.Join(t.TempDir(), "pwd")
+
+	if currentPassword != "" {
+		hashed, err := bcrypt.GenerateFromPassword([]byte(currentPassword), bcrypt.DefaultCost)
+		if err != nil {
+			t.Fatalf("failed to hash password: %s", err)
+		}
+		if err := SetAccount("admin", string(hashed)); err != nil {
+			t.Fatalf("failed to write account: %s", err)
+		}
+	}
+
+	return AccountFile
+}
+
+// stubRootPassword prevents the test from shelling out to passwd(1).
+func stubRootPassword(t *testing.T) *string {
+	t.Helper()
+
+	original := setRootPassword
+	t.Cleanup(func() { setRootPassword = original })
+
+	var applied string
+	setRootPassword = func(password string) error {
+		applied = password
+		return nil
+	}
+
+	return &applied
+}
+
+func changePassword(t *testing.T, body string) *httptest.ResponseRecorder {
+	t.Helper()
+
+	gin.SetMode(gin.TestMode)
+	r := gin.New()
+	r.POST("/api/auth/password", NewService().ChangePassword)
+
+	req := httptest.NewRequest(http.MethodPost, "/api/auth/password", strings.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+
+	w := httptest.NewRecorder()
+	r.ServeHTTP(w, req)
+
+	return w
+}
+
+func TestChangePasswordRejectsWrongCurrentPassword(t *testing.T) {
+	accountFile := useTempAccountFile(t, "correct-horse")
+	stubRootPassword(t)
+
+	before, err := os.ReadFile(accountFile)
+	if err != nil {
+		t.Fatalf("failed to read account: %s", err)
+	}
+
+	w := changePassword(t, `{"username":"admin","oldPassword":"`+encryptPassword("wrong-guess")+
+		`","password":"`+encryptPassword("new-password")+`"}`)
+
+	if !strings.Contains(w.Body.String(), `"code"`) || strings.Contains(w.Body.String(), `"code":0`) {
+		t.Fatalf("expected an error response, got %s", w.Body.String())
+	}
+
+	after, err := os.ReadFile(accountFile)
+	if err != nil {
+		t.Fatalf("failed to re-read account: %s", err)
+	}
+	if string(before) != string(after) {
+		t.Fatal("account must not be modified when the current password is wrong")
+	}
+}
+
+func TestChangePasswordRejectsMissingCurrentPassword(t *testing.T) {
+	useTempAccountFile(t, "correct-horse")
+	stubRootPassword(t)
+
+	w := changePassword(t, `{"username":"admin","password":"`+encryptPassword("new-password")+`"}`)
+
+	if strings.Contains(w.Body.String(), `"code":0`) {
+		t.Fatalf("expected an error response, got %s", w.Body.String())
+	}
+}
+
+func TestChangePasswordAcceptsCorrectCurrentPassword(t *testing.T) {
+	useTempAccountFile(t, "correct-horse")
+	applied := stubRootPassword(t)
+
+	w := changePassword(t, `{"username":"admin","oldPassword":"`+encryptPassword("correct-horse")+
+		`","password":"`+encryptPassword("new-password")+`"}`)
+
+	if !strings.Contains(w.Body.String(), `"code":0`) {
+		t.Fatalf("expected success, got %s", w.Body.String())
+	}
+
+	if !CompareAccount("admin", encryptPassword("new-password")) {
+		t.Fatal("the new password should authenticate after the change")
+	}
+
+	if *applied != "new-password" {
+		t.Fatalf("root password should be updated too, got %q", *applied)
+	}
+}
+
+func TestChangePasswordAllowsFirstTimeSetupWithoutCurrentPassword(t *testing.T) {
+	// A factory-fresh device has no account file and uses the documented
+	// admin/admin default, so demanding the old password adds nothing and
+	// would block the initial setup flow.
+	useTempAccountFile(t, "")
+	stubRootPassword(t)
+
+	w := changePassword(t, `{"username":"admin","password":"`+encryptPassword("new-password")+`"}`)
+
+	if !strings.Contains(w.Body.String(), `"code":0`) {
+		t.Fatalf("expected success on first-time setup, got %s", w.Body.String())
+	}
+}
+
+func TestChangePasswordKeepsTheCheckWhenTheAccountFileCannotBeRead(t *testing.T) {
+	// Only a missing file means "not configured yet". Any other stat error
+	// leaves the answer unknown, and treating unknown as unconfigured would
+	// drop the current-password check exactly when something is wrong.
+	original := AccountFile
+	t.Cleanup(func() { AccountFile = original })
+
+	blocker := filepath.Join(t.TempDir(), "not-a-directory")
+	if err := os.WriteFile(blocker, []byte("x"), 0o600); err != nil {
+		t.Fatalf("setup: %s", err)
+	}
+	// Stat now fails with ENOTDIR rather than ENOENT.
+	AccountFile = filepath.Join(blocker, "pwd")
+
+	if !isAccountConfigured() {
+		t.Fatal("an unreadable account file must not report as unconfigured")
+	}
+
+	stubRootPassword(t)
+
+	w := changePassword(t, `{"username":"admin","password":"`+encryptPassword("new-password")+`"}`)
+
+	if strings.Contains(w.Body.String(), `"code":0`) {
+		t.Fatalf("expected the change to be refused, got %s", w.Body.String())
+	}
+}

web/src/api/auth.ts

@@ -16,10 +16,11 @@ export function getAccount() {
   return http.get('/api/auth/account');
 }
 
-export function changePassword(username: string, password: string) {
+export function changePassword(username: string, password: string, oldPassword: string) {
   const data = {
     username,
-    password
+    password,
+    oldPassword
   };
   return http.post('/api/auth/password', data);
 }

web/src/i18n/locales/en.ts

@@ -12,8 +12,11 @@ const en = {
       placeholderUsername: 'Username',
       placeholderPassword: 'Password',
       placeholderPassword2: 'Please enter password again',
+      placeholderOldPassword: 'Current password',
       noEmptyUsername: 'Username required',
       noEmptyPassword: 'Password required',
+      noEmptyOldPassword: 'Current password required',
+      invalidOldPassword: 'Current password is incorrect',
       noAccount: 'Failed to get user information, please refresh web page or reset password',
       invalidUser: 'Invalid username or password',
       locked: 'Too many logins, please try again later',

web/src/pages/auth/password/index.tsx

@@ -12,6 +12,9 @@ import { Head } from '@/components/head.tsx';
 export const Password = () => {
   const { t } = useTranslation();
   const [msg, setMsg] = useState('');
+  // A device still on the default account has no password worth confirming,
+  // so the current-password field only appears once one has been set.
+  const [needOldPassword, setNeedOldPassword] = useState(false);
   const navigate = useNavigate();
 
   useEffect(() => {
@@ -20,6 +23,17 @@ export const Password = () => {
     }
   }, [msg]);
 
+  useEffect(() => {
+    api
+      .isPasswordUpdated()
+      .then((rsp: any) => {
+        setNeedOldPassword(rsp.code === 0 && rsp.data?.isUpdated === true);
+      })
+      .catch(() => {
+        setNeedOldPassword(true);
+      });
+  }, []);
+
   function changePassword(values: any) {
     if (values.password !== values.password2) {
       setMsg(t('auth.differentPassword'));
@@ -36,10 +50,15 @@ export const Password = () => {
 
     const username = values.username;
     const password = encrypt(values.password);
+    const oldPassword = values.oldPassword ? encrypt(values.oldPassword) : '';
 
     api
-      .changePassword(username, password)
+      .changePassword(username, password, oldPassword)
       .then((rsp: any) => {
+        if (rsp.code === -6) {
+          setMsg(t('auth.invalidOldPassword'));
+          return;
+        }
         if (rsp.code !== 0) {
           setMsg(t('auth.error'));
           return;
@@ -81,6 +100,19 @@ export const Password = () => {
             <Input prefix={<UserOutlined />} placeholder={t('auth.placeholderUsername')} />
           </Form.Item>
 
+          {needOldPassword && (
+            <Form.Item
+              name="oldPassword"
+              rules={[{ required: true, message: t('auth.noEmptyOldPassword'), min: 1 }]}
+            >
+              <Input
+                prefix={<LockOutlined />}
+                type="password"
+                placeholder={t('auth.placeholderOldPassword')}
+              />
+            </Form.Item>
+          )}
+
           <Form.Item
             name="password"
             rules={[{ required: true, message: t('auth.noEmptyPassword'), min: 1 }]}