diff --git a/hotline/account.go b/hotline/account.go index 895d8a0..6f5c2e3 100644 --- a/hotline/account.go +++ b/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"))) } diff --git a/internal/mobius/handlers_accounts.go b/internal/mobius/handlers_accounts.go index 5014d84..ce6a87a 100644 --- a/internal/mobius/handlers_accounts.go +++ b/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. diff --git a/internal/mobius/handlers_accounts_test.go b/internal/mobius/handlers_accounts_test.go index 60b4f6e..5cf952f 100644 --- a/internal/mobius/handlers_accounts_test.go +++ b/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}), }, },