Merge branch 'hotfix/wait4conf' into 'release'

Update client to wait for auth channel before sending messages, or error out loudly

See merge request !577

auth/callback.go

@@ -12,6 +12,7 @@ import (
 	jww "github.com/spf13/jwalterweatherman"
 	"gitlab.com/elixxir/client/interfaces"
 	"gitlab.com/elixxir/client/interfaces/contact"
+	"gitlab.com/elixxir/client/interfaces/message"
 	"gitlab.com/elixxir/client/stoppable"
 	"gitlab.com/elixxir/client/storage/auth"
 	"gitlab.com/elixxir/crypto/cyclic"
@@ -25,47 +26,54 @@ import (
 func (m *Manager) StartProcessies() stoppable.Stoppable {
 
 	stop := stoppable.NewSingle("Auth")
-	authStore := m.storage.Auth()
-	grp := m.storage.E2e().GetGroup()
 
 	go func() {
+		for {
 			select {
 			case <-stop.Quit():
 				return
 			case msg := <-m.rawMessages:
+				m.processAuthMessage(msg)
+			}
+		}
+	}()
+	return stop
+}
+
+func (m *Manager) processAuthMessage(msg message.Receive) {
+	authStore := m.storage.Auth()
 	//lookup the message, check if it is an auth request
 	cmixMsg := format.Unmarshal(msg.Payload)
 	fp := cmixMsg.GetKeyFP()
 	jww.INFO.Printf("RAW AUTH FP: %v", fp)
-			// this takes the request lock if it is a specific fp,
-			// all exits after this need to call fail or delete if it is
+	// this takes the request lock if it is a specific fp, all
+	// exits after this need to call fail or delete if it is
 	// specific
 	fpType, sr, myHistoricalPrivKey, err := authStore.GetFingerprint(fp)
 	if err != nil {
 		jww.TRACE.Printf("FINGERPRINT FAILURE: %s", err.Error())
-				// if the lookup fails, ignore the message. It is likely
-				// garbled or for a different protocol
-				break
+		// if the lookup fails, ignore the message. It is
+		// likely garbled or for a different protocol
+		return
 	}
 
 	//denote that the message is not garbled
 	m.storage.GetGarbledMessages().Remove(cmixMsg)
+	grp := m.storage.E2e().GetGroup()
 
 	switch fpType {
-			// if it is general, that means a new request has been received
 	case auth.General:
+		// if it is general, that means a new request has
+		// been received
 		m.handleRequest(cmixMsg, myHistoricalPrivKey, grp)
+	case auth.Specific:
 		// if it is specific, that means the original request was sent
 		// by this users and a confirmation has been received
-			case auth.Specific:
-				jww.INFO.Printf("Received AutConfirm from %s,"+
-					" msgDigest: %s", sr.GetPartner(), cmixMsg.Digest())
+		jww.INFO.Printf("Received AutConfirm from %s, msgDigest: %s",
+			sr.GetPartner(), cmixMsg.Digest())
 		m.handleConfirm(cmixMsg, sr, grp)
 	}
 }
-	}()
-	return stop
-}
 
 func (m *Manager) handleRequest(cmixMsg format.Message,
 	myHistoricalPrivKey *cyclic.Int, grp *cyclic.Group) {

cmd/root.go

@@ -52,6 +52,24 @@ var rootCmd = &cobra.Command{
 		jww.INFO.Printf("User: %s", user.ReceptionID)
 		writeContact(user.GetContact())
 
+		// Get Recipient and/or set it to myself
+		isPrecanPartner := false
+		recipientContact := readContact()
+		recipientID := recipientContact.ID
+
+		// Try to get recipientID from destid
+		if recipientID == nil {
+			recipientID, isPrecanPartner = parseRecipient(
+				viper.GetString("destid"))
+		}
+
+		// Set it to myself
+		if recipientID == nil {
+			jww.INFO.Printf("sending message to self")
+			recipientID = user.ReceptionID
+			recipientContact = user.GetContact()
+		}
+
 		// Set up reception handler
 		swboard := client.GetSwitchboard()
 		recvCh := make(chan message.Receive, 10000)
@@ -65,12 +83,12 @@ var rootCmd = &cobra.Command{
 		authMgr.AddGeneralRequestCallback(printChanRequest)
 
 		// If unsafe channels, add auto-acceptor
-		num_channels_confirmed := 0
+		authConfirmed := false
 		authMgr.AddGeneralConfirmCallback(func(
 			partner contact.Contact) {
 			jww.INFO.Printf("Channel Confirmed: %s",
 				partner.ID)
-			num_channels_confirmed++
+			authConfirmed = recipientID.Cmp(partner.ID)
 		})
 		if viper.GetBool("unsafe-channel-creation") {
 			authMgr.AddGeneralRequestCallback(func(
@@ -82,7 +100,8 @@ var rootCmd = &cobra.Command{
 				if err != nil {
 					jww.FATAL.Panicf("%+v", err)
 				}
-				num_channels_confirmed++
+				authConfirmed = recipientID.Cmp(
+					requestor.ID)
 			})
 		}
 
@@ -113,39 +132,53 @@ var rootCmd = &cobra.Command{
 		// Send Messages
 		msgBody := viper.GetString("message")
 
-		isPrecanPartner := false
-		recipientContact := readContact()
-		recipientID := recipientContact.ID
-
-		// Try to get recipientID from destid
-		if recipientID == nil {
-			recipientID, isPrecanPartner = parseRecipient(
-				viper.GetString("destid"))
-		}
-
-		// Set it to myself
-		if recipientID == nil {
-			jww.INFO.Printf("sending message to self")
-			recipientID = user.ReceptionID
-			recipientContact = user.GetContact()
-		}
-
 		time.Sleep(10 * time.Second)
 
 		// Accept auth request for this recipient
 		if viper.GetBool("accept-channel") {
 			acceptChannel(client, recipientID)
+			// Do not wait for channel confirmations if we
+			// accepted one
+			authConfirmed = true
+		}
+
+		if client.HasAuthenticatedChannel(recipientID) {
+			jww.INFO.Printf("Authenticated channel already in "+
+				"place for %s", recipientID)
+			authConfirmed = true
 		}
 
 		// Send unsafe messages or not?
 		unsafe := viper.GetBool("unsafe")
-		assumeAuth := viper.GetBool("assume-auth-channel")
-		if !unsafe && !assumeAuth {
+
+		sendAuthReq := viper.GetBool("send-auth-request")
+		if !unsafe && !authConfirmed && !isPrecanPartner &&
+			sendAuthReq {
 			addAuthenticatedChannel(client, recipientID,
-				recipientContact, isPrecanPartner)
-			// Do not wait for channel confirmations if we
-			// tried to add a channel
-			num_channels_confirmed++
+				recipientContact)
+		} else if !unsafe && !authConfirmed && isPrecanPartner {
+			addPrecanAuthenticatedChannel(client,
+				recipientID, recipientContact)
+			authConfirmed = true
+		}
+
+		if !unsafe && !authConfirmed {
+			jww.INFO.Printf("Waiting for authentication channel "+
+				" confirmation with partner %s", recipientID)
+			scnt := uint(0)
+			waitSecs := viper.GetUint("auth-timeout")
+			for !authConfirmed && scnt < waitSecs {
+				time.Sleep(1 * time.Second)
+				scnt++
+			}
+			if scnt == waitSecs {
+				jww.FATAL.Panicf("Could not confirm "+
+					"authentication channel for %s, "+
+					"waited %d seconds.", recipientID,
+					waitSecs)
+			}
+			jww.INFO.Printf("Authentication channel confirmation"+
+				" took %d seconds", waitSecs)
 		}
 
 		msg := message.Send{
@@ -216,13 +249,6 @@ var rootCmd = &cobra.Command{
 			}
 		}
 		fmt.Printf("Received %d\n", receiveCnt)
-		if receiveCnt == 0 && sendCnt == 0 {
-			scnt := uint(0)
-			for num_channels_confirmed == 0 && scnt < waitSecs {
-				time.Sleep(1 * time.Second)
-				scnt++
-			}
-		}
 		err = client.StopNetworkFollower(5 * time.Second)
 		if err != nil {
 			jww.WARN.Printf(
@@ -390,14 +416,27 @@ func printChanRequest(requestor contact.Contact, message string) {
 	//fmt.Printf(msg)
 }
 
-func addAuthenticatedChannel(client *api.Client, recipientID *id.ID,
-	recipient contact.Contact, isPrecanPartner bool) {
-	if client.HasAuthenticatedChannel(recipientID) {
-		jww.INFO.Printf("Authenticated channel already in place for %s",
-			recipientID)
-		return
+func addPrecanAuthenticatedChannel(client *api.Client, recipientID *id.ID,
+	recipient contact.Contact) {
+	jww.WARN.Printf("Precanned user id detected: %s", recipientID)
+	preUsr, err := client.MakePrecannedAuthenticatedChannel(
+		getPrecanID(recipientID))
+	if err != nil {
+		jww.FATAL.Panicf("%+v", err)
+	}
+	// Sanity check, make sure user id's haven't changed
+	preBytes := preUsr.ID.Bytes()
+	idBytes := recipientID.Bytes()
+	for i := 0; i < len(preBytes); i++ {
+		if idBytes[i] != preBytes[i] {
+			jww.FATAL.Panicf("no id match: %v %v",
+				preBytes, idBytes)
+		}
+	}
 }
 
+func addAuthenticatedChannel(client *api.Client, recipientID *id.ID,
+	recipient contact.Contact) {
 	var allowed bool
 	if viper.GetBool("unsafe-channel-creation") {
 		msg := "unsafe channel creation enabled\n"
@@ -418,24 +457,7 @@ func addAuthenticatedChannel(client *api.Client, recipientID *id.ID,
 
 	recipientContact := recipient
 
-	if isPrecanPartner {
-		jww.WARN.Printf("Precanned user id detected: %s",
-			recipientID)
-		preUsr, err := client.MakePrecannedAuthenticatedChannel(
-			getPrecanID(recipientID))
-		if err != nil {
-			jww.FATAL.Panicf("%+v", err)
-		}
-		// Sanity check, make sure user id's haven't changed
-		preBytes := preUsr.ID.Bytes()
-		idBytes := recipientID.Bytes()
-		for i := 0; i < len(preBytes); i++ {
-			if idBytes[i] != preBytes[i] {
-				jww.FATAL.Panicf("no id match: %v %v",
-					preBytes, idBytes)
-			}
-		}
-	} else if recipientContact.ID != nil && recipientContact.DhPubKey != nil {
+	if recipientContact.ID != nil && recipientContact.DhPubKey != nil {
 		me := client.GetUser().GetContact()
 		jww.INFO.Printf("Requesting auth channel from: %s",
 			recipientID)
@@ -707,16 +729,22 @@ func init() {
 	viper.BindPFlag("unsafe-channel-creation",
 		rootCmd.Flags().Lookup("unsafe-channel-creation"))
 
-	rootCmd.Flags().BoolP("assume-auth-channel", "", false,
-		"Do not check for an authentication channel for this user")
-	viper.BindPFlag("assume-auth-channel",
-		rootCmd.Flags().Lookup("assume-auth-channel"))
-
 	rootCmd.Flags().BoolP("accept-channel", "", false,
 		"Accept the channel request for the corresponding recipient ID")
 	viper.BindPFlag("accept-channel",
 		rootCmd.Flags().Lookup("accept-channel"))
 
+	rootCmd.Flags().BoolP("send-auth-request", "", false,
+		"Send an auth request to the specified destination and wait"+
+			"for confirmation")
+	viper.BindPFlag("send-auth-request",
+		rootCmd.Flags().Lookup("send-auth-request"))
+	rootCmd.Flags().UintP("auth-timeout", "", 120,
+		"The number of seconds to wait for an authentication channel"+
+			"to confirm")
+	viper.BindPFlag("auth-timeout",
+		rootCmd.Flags().Lookup("auth-timeout"))
+
 	rootCmd.Flags().BoolP("forceHistoricalRounds", "", false,
 		"Force all rounds to be sent to historical round retrieval")
 	viper.BindPFlag("forceHistoricalRounds",

keyExchange/exchange.go

@@ -39,7 +39,7 @@ func Start(switchboard *switchboard.Switchboard, sess *storage.Session, net inte
 		})
 
 	// start the trigger thread
-	go startTrigger(sess, net, triggerCh, triggerStop.Quit(), params)
+	go startTrigger(sess, net, triggerCh, triggerStop, params)
 
 	//register the rekey confirm thread
 	confirmCh := make(chan message.Receive, 100)

keyExchange/trigger.go

@@ -16,6 +16,7 @@ import (
 	"gitlab.com/elixxir/client/interfaces/message"
 	"gitlab.com/elixxir/client/interfaces/params"
 	"gitlab.com/elixxir/client/interfaces/utility"
+	"gitlab.com/elixxir/client/stoppable"
 	"gitlab.com/elixxir/client/storage"
 	"gitlab.com/elixxir/client/storage/e2e"
 	ds "gitlab.com/elixxir/comms/network/dataStructures"
@@ -26,20 +27,22 @@ import (
 const (
 	errBadTrigger = "non-e2e trigger from partner %s"
 	errUnknown    = "unknown trigger from partner %s"
+	errFailed     = "Failed to handle rekey trigger: %s"
 )
 
 func startTrigger(sess *storage.Session, net interfaces.NetworkManager,
-	c chan message.Receive, quitCh <-chan struct{}, params params.Rekey) {
+	c chan message.Receive, stop *stoppable.Single, params params.Rekey) {
 	for true {
 		select {
-		case <-quitCh:
+		case <-stop.Quit():
 			return
 		case request := <-c:
+			go func() {
 				err := handleTrigger(sess, net, request, params)
 				if err != nil {
-				jww.ERROR.Printf("Failed to handle rekey trigger: %s",
-					err)
+					jww.ERROR.Printf(errFailed, err)
 				}
+			}()
 		}
 	}
 }

storage/e2e/relationship.go

@@ -293,17 +293,24 @@ func (r *relationship) getNewestRekeyableSession() *Session {
 	if len(sessions) == 0 {
 		return nil
 	}
+
+	var unconfirmed *Session
+
 	for _, s := range r.sessions {
 		//fmt.Println(i)
 		// This looks like it might not be thread safe, I think it is because
 		// the failure mode is it skips to a lower key to rekey with, which is
 		// always valid. It isn't clear it can fail though because we are
 		// accessing the data in the same order it would be written (i think)
-		if s.Status() != RekeyEmpty && s.IsConfirmed() {
+		if s.Status() != RekeyEmpty {
+			if s.IsConfirmed(){
 				return s
+			}else if unconfirmed == nil{
+				unconfirmed = s
 			}
 		}
-	return nil
+	}
+	return unconfirmed
 }
 
 func (r *relationship) GetByID(id SessionID) *Session {

storage/e2e/relationship_test.go

@@ -14,6 +14,7 @@ import (
 	"gitlab.com/elixxir/ekv"
 	"gitlab.com/xx_network/primitives/id"
 	"reflect"
+	"sync"
 	"testing"
 )
 
@@ -196,8 +197,8 @@ func TestRelationship_GetNewestRekeyableSession(t *testing.T) {
 	sb.sessions[0].negotiationStatus = Unconfirmed
 	// no available rekeyable sessions: nil
 	session2 := sb.getNewestRekeyableSession()
-	if session2 != nil {
-		t.Error("newest rekeyable session should be nil")
+	if session2 != sb.sessions[0] {
+		t.Error("newest rekeyable session should be the unconfired session")
 	}
 
 	// add a rekeyable session: that session
@@ -534,3 +535,40 @@ func relationshipsEqual(buff *relationship, buff2 *relationship) bool {
 	}
 	return true
 }
+
+func Test_relationship_getNewestRekeyableSession(t *testing.T) {
+	type fields struct {
+		manager     *Manager
+		t           RelationshipType
+		kv          *versioned.KV
+		sessions    []*Session
+		sessionByID map[SessionID]*Session
+		fingerprint []byte
+		mux         sync.RWMutex
+		sendMux     sync.Mutex
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   *Session
+	}{
+		// TODO: Add test cases.
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := &relationship{
+				manager:     tt.fields.manager,
+				t:           tt.fields.t,
+				kv:          tt.fields.kv,
+				sessions:    tt.fields.sessions,
+				sessionByID: tt.fields.sessionByID,
+				fingerprint: tt.fields.fingerprint,
+				mux:         tt.fields.mux,
+				sendMux:     tt.fields.sendMux,
+			}
+			if got := r.getNewestRekeyableSession(); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("getNewestRekeyableSession() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
\ No newline at end of file