Browse Source

Upgrade password hashing.

Previously, we generated and prepended a long salt before generating
password hashes. This resulted in the hash verification cutting off long
before it should do. This form of salting is also not necessary with
bcrypt as it's provided by the password hashing and verification
functions themselves, so totally rip it out.

This commit also adds the functionality for the server to automagically
upgrade users to use the new hashing system, which means better
security and more assurance that people can't bruteforce passwords.

No need to apply a database upgrade to do this, whoo! \o/
tags/v0.12.0
Daniel Oaks 6 years ago
parent
commit
6260869068
3 changed files with 63 additions and 20 deletions
  1. 47
    8
      irc/accounts.go
  2. 7
    6
      irc/errors.go
  3. 9
    6
      irc/passwd/unsalted.go

+ 47
- 8
irc/accounts.go View File

@@ -189,15 +189,11 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames
189 189
 	certFPKey := fmt.Sprintf(keyCertToAccount, certfp)
190 190
 
191 191
 	var creds AccountCredentials
192
-	// always set passphrase salt
193
-	creds.PassphraseSalt, err = passwd.NewSalt()
194
-	if err != nil {
195
-		return errAccountCreation
196
-	}
197 192
 	// it's fine if this is empty, that just means no certificate is authorized
198 193
 	creds.Certificate = certfp
199 194
 	if passphrase != "" {
200
-		creds.PassphraseHash, err = am.server.passwords.GenerateFromPassword(creds.PassphraseSalt, passphrase)
195
+		creds.PassphraseHash, err = passwd.GenerateEncodedPasswordBytes(passphrase)
196
+		creds.PassphraseIsV2 = true
201 197
 		if err != nil {
202 198
 			am.server.logger.Error("internal", fmt.Sprintf("could not hash password: %v", err))
203 199
 			return errAccountCreation
@@ -522,8 +518,50 @@ func (am *AccountManager) AuthenticateByPassphrase(client *Client, accountName s
522 518
 		return errAccountUnverified
523 519
 	}
524 520
 
525
-	err = am.server.passwords.CompareHashAndPassword(
526
-		account.Credentials.PassphraseHash, account.Credentials.PassphraseSalt, passphrase)
521
+	if account.Credentials.PassphraseIsV2 {
522
+		err = passwd.ComparePassword(account.Credentials.PassphraseHash, []byte(passphrase))
523
+	} else {
524
+		// compare using legacy method
525
+		err = am.server.passwords.CompareHashAndPassword(account.Credentials.PassphraseHash, account.Credentials.PassphraseSalt, passphrase)
526
+		if err == nil {
527
+			// passphrase worked! silently upgrade them to use v2 hashing going forward.
528
+			//TODO(dan): in future, replace this with an am.updatePassphrase(blah) function, which we can reuse in /ns update pass?
529
+			err = am.server.store.Update(func(tx *buntdb.Tx) error {
530
+				var creds AccountCredentials
531
+				creds.Certificate = account.Credentials.Certificate
532
+				creds.PassphraseHash, err = passwd.GenerateEncodedPasswordBytes(passphrase)
533
+				creds.PassphraseIsV2 = true
534
+				if err != nil {
535
+					am.server.logger.Error("internal", fmt.Sprintf("could not hash password (updating existing hash version): %v", err))
536
+					return errAccountCredUpdate
537
+				}
538
+
539
+				credText, err := json.Marshal(creds)
540
+				if err != nil {
541
+					am.server.logger.Error("internal", fmt.Sprintf("could not marshal credentials (updating existing hash version): %v", err))
542
+					return errAccountCredUpdate
543
+				}
544
+				credStr := string(credText)
545
+
546
+				// we know the account name is valid if this line is reached, otherwise the
547
+				// above would have failed. as such, chuck out and ignore err on casefolding
548
+				casefoldedAccountName, _ := CasefoldName(accountName)
549
+				credentialsKey := fmt.Sprintf(keyAccountCredentials, casefoldedAccountName)
550
+
551
+				//TODO(dan): sling, can you please checkout this mutex usage, see if it
552
+				// makes sense or not? bleh
553
+				am.serialCacheUpdateMutex.Lock()
554
+				defer am.serialCacheUpdateMutex.Unlock()
555
+
556
+				tx.Set(credentialsKey, credStr, nil)
557
+
558
+				return nil
559
+			})
560
+		}
561
+		if err != nil {
562
+			return err
563
+		}
564
+	}
527 565
 	if err != nil {
528 566
 		return errAccountInvalidCredentials
529 567
 	}
@@ -984,6 +1022,7 @@ var (
984 1022
 type AccountCredentials struct {
985 1023
 	PassphraseSalt []byte
986 1024
 	PassphraseHash []byte
1025
+	PassphraseIsV2 bool   `json:"passphrase-is-v2"`
987 1026
 	Certificate    string // fingerprint
988 1027
 }
989 1028
 

+ 7
- 6
irc/errors.go View File

@@ -10,17 +10,18 @@ import "errors"
10 10
 // Runtime Errors
11 11
 var (
12 12
 	errAccountAlreadyRegistered       = errors.New("Account already exists")
13
+	errAccountAlreadyVerified         = errors.New("Account is already verified")
14
+	errAccountCantDropPrimaryNick     = errors.New("Can't unreserve primary nickname")
13 15
 	errAccountCreation                = errors.New("Account could not be created")
16
+	errAccountCredUpdate              = errors.New("Could not update password hash to new method")
14 17
 	errAccountDoesNotExist            = errors.New("Account does not exist")
18
+	errAccountInvalidCredentials      = errors.New("Invalid account credentials")
19
+	errAccountNickReservationFailed   = errors.New("Could not (un)reserve nick")
15 20
 	errAccountNotLoggedIn             = errors.New("You're not logged into an account")
21
+	errAccountTooManyNicks            = errors.New("Account has too many reserved nicks")
22
+	errAccountUnverified              = errors.New("Account is not yet verified")
16 23
 	errAccountVerificationFailed      = errors.New("Account verification failed")
17 24
 	errAccountVerificationInvalidCode = errors.New("Invalid account verification code")
18
-	errAccountUnverified              = errors.New("Account is not yet verified")
19
-	errAccountAlreadyVerified         = errors.New("Account is already verified")
20
-	errAccountInvalidCredentials      = errors.New("Invalid account credentials")
21
-	errAccountTooManyNicks            = errors.New("Account has too many reserved nicks")
22
-	errAccountNickReservationFailed   = errors.New("Could not (un)reserve nick")
23
-	errAccountCantDropPrimaryNick     = errors.New("Can't unreserve primary nickname")
24 25
 	errAccountUpdateFailed            = errors.New("Error while updating your account information")
25 26
 	errCallbackFailed                 = errors.New("Account verification could not be sent")
26 27
 	errCertfpAlreadyExists            = errors.New("An account already exists with your certificate")

+ 9
- 6
irc/passwd/unsalted.go View File

@@ -15,16 +15,19 @@ var (
15 15
 	ErrEmptyPassword = errors.New("empty password")
16 16
 )
17 17
 
18
-// GenerateEncodedPassword returns an encrypted password, encoded into a string with base64.
19
-func GenerateEncodedPassword(passwd string) (encoded string, err error) {
18
+// GenerateEncodedPasswordBytes returns an encrypted password, returning the bytes directly.
19
+func GenerateEncodedPasswordBytes(passwd string) (encoded []byte, err error) {
20 20
 	if passwd == "" {
21 21
 		err = ErrEmptyPassword
22 22
 		return
23 23
 	}
24
-	bcrypted, err := bcrypt.GenerateFromPassword([]byte(passwd), bcrypt.MinCost)
25
-	if err != nil {
26
-		return
27
-	}
24
+	encoded, err = bcrypt.GenerateFromPassword([]byte(passwd), bcrypt.MinCost)
25
+	return
26
+}
27
+
28
+// GenerateEncodedPassword returns an encrypted password, encoded into a string with base64.
29
+func GenerateEncodedPassword(passwd string) (encoded string, err error) {
30
+	bcrypted, err := GenerateEncodedPasswordBytes(passwd)
28 31
 	encoded = base64.StdEncoding.EncodeToString(bcrypted)
29 32
 	return
30 33
 }

Loading…
Cancel
Save