Don't send the account password hash in GetUser replies

hotline/account.go

@@ -30,6 +30,11 @@ func NewAccount(login, name, password string, access AccessBitmap) *Account {
 	}
 }
 
+// HasPassword reports whether the account has a non-empty password.
+func (a *Account) HasPassword() bool {
+	return bcrypt.CompareHashAndPassword([]byte(a.Password), []byte("")) != nil
+}
+
 // Read implements io.Reader interface for Account
 func (a *Account) Read(p []byte) (int, error) {
 	fields := []Field{
@@ -38,7 +43,7 @@ func (a *Account) Read(p []byte) (int, error) {
 		NewField(FieldUserAccess, a.Access[:]),
 	}
 
-	if bcrypt.CompareHashAndPassword([]byte(a.Password), []byte("")) != nil {
+	if a.HasPassword() {
 		fields = append(fields, NewField(FieldUserPassword, []byte("x")))
 	}
 

internal/mobius/handlers_accounts.go

@@ -88,7 +88,7 @@ func HandleSetUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotlin
 // Fields used in the reply:
 //   - 102 User name      Account display name
 //   - 105 User login     Account login (encoded, each character negated)
-//   - 106 User password  Account password
+//   - 106 User password  "x" if the account has a password, omitted otherwise
 //   - 110 User access    User access privileges bitmap
 func HandleGetUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotline.Transaction) {
 	if !cc.Authorize(hotline.AccessOpenUser) {
@@ -100,12 +100,18 @@ func HandleGetUser(cc *hotline.ClientConn, t *hotline.Transaction) (res []hotlin
 		return cc.NewErrReply(t, ErrMsgAccountNotExist)
 	}
 
-	return append(res, cc.NewReply(t,
+	// Never send the stored hash: it is crackable offline, and a client that sends the
+	// field back unchanged in SetUser would have the hash itself become the password.
+	fields := []hotline.Field{
 		hotline.NewField(hotline.FieldUserName, []byte(account.Name)),
 		hotline.NewField(hotline.FieldUserLogin, hotline.EncodeString(t.GetField(hotline.FieldUserLogin).Data)),
-		hotline.NewField(hotline.FieldUserPassword, []byte(account.Password)),
-		hotline.NewField(hotline.FieldUserAccess, account.Access[:]),
-	))
+	}
+	if account.HasPassword() {
+		fields = append(fields, hotline.NewField(hotline.FieldUserPassword, []byte("x")))
+	}
+	fields = append(fields, hotline.NewField(hotline.FieldUserAccess, account.Access[:]))
+
+	return append(res, cc.NewReply(t, fields...))
 }
 
 // HandleListUsers returns a list of all user accounts on the server.

internal/mobius/handlers_accounts_test.go

@@ -22,7 +22,7 @@ func TestHandleGetUser(t *testing.T) {
 		wantRes []hotline.Transaction
 	}{
 		{
-			name: "when account is valid",
+			name: "when account has a password",
 			args: args{
 				cc: &hotline.ClientConn{
 					Account: &hotline.Account{
@@ -40,7 +40,50 @@ func TestHandleGetUser(t *testing.T) {
 							m.On("Get", "guest").Return(&hotline.Account{
 								Login:    "guest",
 								Name:     "Guest",
-								Password: "password",
+								Password: hotline.HashAndSalt([]byte("password")),
+								Access:   hotline.AccessBitmap{},
+							})
+							return &m
+						}(),
+					},
+				},
+				t: hotline.NewTransaction(
+					hotline.TranGetUser, [2]byte{0, 1},
+					hotline.NewField(hotline.FieldUserLogin, []byte("guest")),
+				),
+			},
+			wantRes: []hotline.Transaction{
+				{
+					IsReply: 0x01,
+					Fields: []hotline.Field{
+						hotline.NewField(hotline.FieldUserName, []byte("Guest")),
+						hotline.NewField(hotline.FieldUserLogin, hotline.EncodeString([]byte("guest"))),
+						hotline.NewField(hotline.FieldUserPassword, []byte("x")),
+						hotline.NewField(hotline.FieldUserAccess, []byte{0, 0, 0, 0, 0, 0, 0, 0}),
+					},
+				},
+			},
+		},
+		{
+			name: "when account has no password",
+			args: args{
+				cc: &hotline.ClientConn{
+					Account: &hotline.Account{
+						Access: func() hotline.AccessBitmap {
+							var bits hotline.AccessBitmap
+							bits.Set(hotline.AccessOpenUser)
+							return bits
+						}(),
+					},
+					Server: &hotline.Server{
+						TextDecoder: charmap.Macintosh.NewDecoder(),
+						TextEncoder: charmap.Macintosh.NewEncoder(),
+						AccountManager: func() *MockAccountManager {
+							m := MockAccountManager{}
+							m.On("Get", "guest").Return(&hotline.Account{
+								Login:    "guest",
+								Name:     "Guest",
+								Password: hotline.HashAndSalt([]byte("")),
 								Access:   hotline.AccessBitmap{},
 							})
 							return &m
@@ -58,7 +101,6 @@ func TestHandleGetUser(t *testing.T) {
 					Fields: []hotline.Field{
 						hotline.NewField(hotline.FieldUserName, []byte("Guest")),
 						hotline.NewField(hotline.FieldUserLogin, hotline.EncodeString([]byte("guest"))),
-						hotline.NewField(hotline.FieldUserPassword, []byte("password")),
 						hotline.NewField(hotline.FieldUserAccess, []byte{0, 0, 0, 0, 0, 0, 0, 0}),
 					},
 				},