Browse Source

fix a bug

In the previous commit, the client would receive a failure message but would
actually remain logged in after an authzid/authcid mismatch. This was a
correctness rather than a security issue, but now it's fixed so that the client
never logs in in the first place.
tags/v2.0.0-rc1
Shivaram Lingamneni 4 years ago
parent
commit
33c8b2177e
4 changed files with 28 additions and 19 deletions
  1. 5
    1
      irc/accounts.go
  2. 1
    0
      irc/errors.go
  3. 21
    17
      irc/handlers.go
  4. 1
    1
      irc/nickserv.go

+ 5
- 1
irc/accounts.go View File

@@ -971,7 +971,7 @@ func (am *AccountManager) ChannelsForAccount(account string) (channels []string)
971 971
 	return unmarshalRegisteredChannels(channelStr)
972 972
 }
973 973
 
974
-func (am *AccountManager) AuthenticateByCertFP(client *Client) error {
974
+func (am *AccountManager) AuthenticateByCertFP(client *Client, authzid string) error {
975 975
 	if client.certfp == "" {
976 976
 		return errAccountInvalidCredentials
977 977
 	}
@@ -991,6 +991,10 @@ func (am *AccountManager) AuthenticateByCertFP(client *Client) error {
991 991
 		return err
992 992
 	}
993 993
 
994
+	if authzid != "" && authzid != account {
995
+		return errAuthzidAuthcidMismatch
996
+	}
997
+
994 998
 	// ok, we found an account corresponding to their certificate
995 999
 	clientAccount, err := am.LoadAccount(account)
996 1000
 	if err != nil {

+ 1
- 0
irc/errors.go View File

@@ -27,6 +27,7 @@ var (
27 27
 	errAccountVerificationInvalidCode = errors.New("Invalid account verification code")
28 28
 	errAccountUpdateFailed            = errors.New(`Error while updating your account information`)
29 29
 	errAccountMustHoldNick            = errors.New(`You must hold that nickname in order to register it`)
30
+	errAuthzidAuthcidMismatch         = errors.New(`authcid and authzid must be the same`)
30 31
 	errCallbackFailed                 = errors.New("Account verification could not be sent")
31 32
 	errCertfpAlreadyExists            = errors.New(`An account already exists for your certificate fingerprint`)
32 33
 	errChannelNotOwnedByAccount       = errors.New("Channel not owned by the specified account")

+ 21
- 17
irc/handlers.go View File

@@ -446,13 +446,14 @@ func authPlainHandler(server *Server, client *Client, mechanism string, value []
446 446
 }
447 447
 
448 448
 func authErrorToMessage(server *Server, err error) (msg string) {
449
-	if err == errAccountDoesNotExist || err == errAccountUnverified || err == errAccountInvalidCredentials {
450
-		msg = err.Error()
451
-	} else {
449
+	switch err {
450
+	case errAccountDoesNotExist, errAccountUnverified, errAccountInvalidCredentials, errAuthzidAuthcidMismatch:
451
+		return err.Error()
452
+	default:
453
+		// don't expose arbitrary error messages to the user
452 454
 		server.logger.Error("internal", "sasl authentication failure", err.Error())
453
-		msg = "Unknown"
455
+		return "Unknown"
454 456
 	}
455
-	return
456 457
 }
457 458
 
458 459
 // AUTHENTICATE EXTERNAL
@@ -462,24 +463,27 @@ func authExternalHandler(server *Server, client *Client, mechanism string, value
462 463
 		return false
463 464
 	}
464 465
 
465
-	err := server.accounts.AuthenticateByCertFP(client)
466
-	if err != nil {
467
-		msg := authErrorToMessage(server, err)
468
-		rb.Add(nil, server.name, ERR_SASLFAIL, client.nick, fmt.Sprintf("%s: %s", client.t("SASL authentication failed"), client.t(msg)))
469
-		return false
470
-	}
471
-
472 466
 	// EXTERNAL doesn't carry an authentication ID (this is determined from the
473 467
 	// certificate), but does carry an optional authorization ID.
468
+	var authzid string
469
+	var err error
474 470
 	if len(value) != 0 {
475
-		authcid := client.Account()
476
-		cfAuthzid, err := CasefoldName(string(value))
477
-		if err != nil || cfAuthzid != authcid {
478
-			rb.Add(nil, server.name, ERR_SASLFAIL, client.Nick(), client.t("SASL authentication failed: authcid and authzid should be the same"))
479
-			return false
471
+		authzid, err = CasefoldName(string(value))
472
+		if err != nil {
473
+			err = errAuthzidAuthcidMismatch
480 474
 		}
481 475
 	}
482 476
 
477
+	if err == nil {
478
+		err = server.accounts.AuthenticateByCertFP(client, authzid)
479
+	}
480
+
481
+	if err != nil {
482
+		msg := authErrorToMessage(server, err)
483
+		rb.Add(nil, server.name, ERR_SASLFAIL, client.nick, fmt.Sprintf("%s: %s", client.t("SASL authentication failed"), client.t(msg)))
484
+		return false
485
+	}
486
+
483 487
 	sendSuccessfulAccountAuth(client, rb, false, true)
484 488
 	return false
485 489
 }

+ 1
- 1
irc/nickserv.go View File

@@ -536,7 +536,7 @@ func nsIdentifyHandler(server *Server, client *Client, command string, params []
536 536
 
537 537
 	// try certfp
538 538
 	if !loginSuccessful && client.certfp != "" {
539
-		err := server.accounts.AuthenticateByCertFP(client)
539
+		err := server.accounts.AuthenticateByCertFP(client, "")
540 540
 		loginSuccessful = (err == nil)
541 541
 	}
542 542
 

Loading…
Cancel
Save