don't log all errors

Also, return errors when validating handshake payloads and remote
peer IDs. These were being logged but not returned, so the handshake
would complete successfully even if the payload signature was
invalid

p2p/security/noise/ik_handshake.go

@@ -24,14 +24,12 @@ func (s *secureSession) ik_sendHandshakeMessage(payload []byte, initial_stage bo
 
 	err := s.writeLength(len(encMsgBuf))
 	if err != nil {
-		log.Error("ik_sendHandshakeMessage initiator=%v err=%s", s.initiator, err)
 		return fmt.Errorf("ik_sendHandshakeMessage write length err=%s", err)
 	}
 
 	// send message
 	_, err = s.insecure.Write(encMsgBuf)
 	if err != nil {
-		log.Error("ik_sendHandshakeMessage initiator=%v err=%s", s.initiator, err)
 		return fmt.Errorf("ik_sendHandshakeMessage write to conn err=%s", err)
 	}
 
@@ -61,13 +59,11 @@ func (s *secureSession) ik_recvHandshakeMessage(initial_stage bool) (buf []byte,
 	log.Debugf("ik_recvHandshakeMessage initiator=%v msgbuf=%v", s.initiator, msgbuf)
 
 	if err != nil {
-		log.Errorf("ik_recvHandshakeMessage initiator=%v decode err=%s", s.initiator, err)
 		return buf, nil, false, fmt.Errorf("ik_recvHandshakeMessage decode msg fail: %s", err)
 	}
 
 	s.ik_ns, plaintext, valid = ik.RecvMessage(s.ik_ns, msgbuf)
 	if !valid {
-		log.Errorf("ik_recvHandshakeMessage initiator=%v err=%s", s.initiator, "validation fail")
 		return buf, nil, false, fmt.Errorf("ik_recvHandshakeMessage validation fail")
 	}
 
@@ -99,7 +95,6 @@ func (s *secureSession) runHandshake_ik(ctx context.Context, payload []byte) ([]
 		// stage 0 //
 		err := s.ik_sendHandshakeMessage(payload, true)
 		if err != nil {
-			log.Errorf("runHandshake_ik stage=0 initiator=true send err=%s", err)
 			return nil, fmt.Errorf("runHandshake_ik stage=0 initiator=true err=%s", err)
 		}
 
@@ -125,20 +120,19 @@ func (s *secureSession) runHandshake_ik(ctx context.Context, payload []byte) ([]
 		// set remote libp2p public key
 		err = s.setRemotePeerInfo(nhp.GetIdentityKey())
 		if err != nil {
-			log.Errorf("runHandshake_ik stage=1 initiator=true set remote peer info err=%s", err)
 			return buf, fmt.Errorf("runHandshake_ik stage=1 initiator=true err=read remote libp2p key fail")
 		}
 
 		// assert that remote peer ID matches libp2p key
 		err = s.setRemotePeerID(s.RemotePublicKey())
 		if err != nil {
-			log.Errorf("runHandshake_ik stage=1 initiator=true set remote peer id err=%s", err)
+			return buf, fmt.Errorf("runHandshake_ik stage=1 initiator=true err=%s", err)
 		}
 
 		// verify payload is signed by libp2p key
 		err = s.verifyPayload(nhp, remoteNoiseKey)
 		if err != nil {
-			log.Errorf("runHandshake_ik stage=1 initiator=true verify payload err=%s", err)
+			return buf, fmt.Errorf("runHandshake_ik stage=1 initiator=true verify payload err=%s", err)
 		}
 
 	} else {

p2p/security/noise/protocol.go

@@ -170,7 +170,6 @@ func (s *secureSession) runHandshake(ctx context.Context) error {
 	noise_pub := s.noiseKeypair.publicKey
 	signedPayload, err := s.localKey.Sign(append([]byte(payload_string), noise_pub[:]...))
 	if err != nil {
-		log.Errorf("runHandshake signing payload err=%s", err)
 		return fmt.Errorf("runHandshake signing payload err=%s", err)
 	}
 
@@ -180,7 +179,6 @@ func (s *secureSession) runHandshake(ctx context.Context) error {
 	payload.IdentitySig = signedPayload
 	payloadEnc, err := proto.Marshal(payload)
 	if err != nil {
-		log.Errorf("runHandshake marshal payload err=%s", err)
 		return fmt.Errorf("runHandshake proto marshal payload err=%s", err)
 	}
 
@@ -195,12 +193,9 @@ func (s *secureSession) runHandshake(ctx context.Context) error {
 		// we're either a responder or an initiator with a known static key for the remote peer, try IK
 		buf, err := s.runHandshake_ik(ctx, payloadEnc)
 		if err != nil {
-			log.Error("runHandshake ik err=%s", err)
-
 			// IK failed, pipe to XXfallback
 			err = s.runHandshake_xx(ctx, true, payloadEnc, buf)
 			if err != nil {
-				log.Error("runHandshake xx err=err", err)
 				return fmt.Errorf("runHandshake xx err=%s", err)
 			}
 
@@ -212,7 +207,6 @@ func (s *secureSession) runHandshake(ctx context.Context) error {
 		// unknown static key for peer, try XX
 		err := s.runHandshake_xx(ctx, false, payloadEnc, nil)
 		if err != nil {
-			log.Error("runHandshake xx err=%s", err)
 			return err
 		}
 
@@ -262,13 +256,11 @@ func (s *secureSession) Read(buf []byte) (int, error) {
 		ciphertext := make([]byte, l)
 		_, err = io.ReadFull(s.insecure, ciphertext)
 		if err != nil {
-			log.Error("read ciphertext err", err)
 			return 0, err
 		}
 
 		plaintext, err := s.Decrypt(ciphertext)
 		if err != nil {
-			log.Error("decrypt err", err)
 			return 0, err
 		}
 
@@ -328,13 +320,11 @@ func (s *secureSession) Write(in []byte) (int, error) {
 	writeChunk := func(in []byte) (int, error) {
 		ciphertext, err := s.Encrypt(in)
 		if err != nil {
-			log.Error("encrypt error", err)
 			return 0, err
 		}
 
 		err = s.writeLength(len(ciphertext))
 		if err != nil {
-			log.Error("write length err: ", err)
 			return 0, err
 		}
 

p2p/security/noise/xx_handshake.go

@@ -24,13 +24,11 @@ func (s *secureSession) xx_sendHandshakeMessage(payload []byte, initial_stage bo
 
 	err := s.writeLength(len(encMsgBuf))
 	if err != nil {
-		log.Error("xx_sendHandshakeMessage initiator=%v err=%s", s.initiator, err)
 		return fmt.Errorf("xx_sendHandshakeMessage write length err=%s", err)
 	}
 
 	_, err = s.insecure.Write(encMsgBuf)
 	if err != nil {
-		log.Error("xx_sendHandshakeMessage initiator=%v err=%s", s.initiator, err)
 		return fmt.Errorf("xx_sendHandshakeMessage write to conn err=%s", err)
 	}
 
@@ -66,7 +64,6 @@ func (s *secureSession) xx_recvHandshakeMessage(initial_stage bool) (buf []byte,
 
 	s.xx_ns, plaintext, valid = xx.RecvMessage(s.xx_ns, msgbuf)
 	if !valid {
-		log.Error("xx_recvHandshakeMessage initiator=%v err=validation fail", s.initiator)
 		return buf, nil, false, fmt.Errorf("xx_recvHandshakeMessage validation fail")
 	}
 
@@ -137,7 +134,6 @@ func (s *secureSession) runHandshake_xx(ctx context.Context, fallback bool, payl
 
 			s.xx_ns, plaintext, valid = xx.RecvMessage(s.xx_ns, msgbuf)
 			if !valid {
-				log.Errorf("xx_recvHandshakeMessage initiator=%v", s.initiator, "error", "validation fail")
 				return fmt.Errorf("runHandshake_xx validation fail")
 			}
 
@@ -160,22 +156,21 @@ func (s *secureSession) runHandshake_xx(ctx context.Context, fallback bool, payl
 		// set remote libp2p public key
 		err = s.setRemotePeerInfo(nhp.GetIdentityKey())
 		if err != nil {
-			log.Errorf("runHandshake_xx stage=2 initiator=true set remote peer info err=%s", err)
 			return fmt.Errorf("runHandshake_xx stage=2 initiator=true read remote libp2p key fail")
 		}
 
 		// assert that remote peer ID matches libp2p public key
 		pid, err := peer.IDFromPublicKey(s.RemotePublicKey())
 		if pid != s.remotePeer {
-			log.Errorf("runHandshake_xx stage=2 initiator=true check remote peer id err: expected %x got %x", s.remotePeer, pid)
+			return fmt.Errorf("runHandshake_xx stage=2 initiator=true check remote peer id err: expected %x got %x", s.remotePeer, pid)
 		} else if err != nil {
-			log.Errorf("runHandshake_xx stage 2 initiator check remote peer id err %s", err)
+			return fmt.Errorf("runHandshake_xx stage 2 initiator check remote peer id err %s", err)
 		}
 
 		// verify payload is signed by libp2p key
 		err = s.verifyPayload(nhp, s.xx_ns.RemoteKey())
 		if err != nil {
-			log.Errorf("runHandshake_xx stage=2 initiator=true verify payload err=%s", err)
+			return fmt.Errorf("runHandshake_xx stage=2 initiator=true verify payload err=%s", err)
 		}
 
 		if s.noisePipesSupport {
@@ -205,7 +200,6 @@ func (s *secureSession) runHandshake_xx(ctx context.Context, fallback bool, payl
 			var msgbuf *xx.MessageBuffer
 			msgbuf, err = xx.Decode0(initialMsg)
 			if err != nil {
-				log.Errorf("runHandshake_xx recv msg err", err)
 				return err
 			}
 
@@ -214,7 +208,6 @@ func (s *secureSession) runHandshake_xx(ctx context.Context, fallback bool, payl
 
 			s.xx_ns, plaintext, valid = xx.RecvMessage(s.xx_ns, &xx_msgbuf)
 			if !valid {
-				log.Errorf("runHandshake_xx initiator=false recv msg err=%s", "validation fail")
 				return fmt.Errorf("runHandshake_xx validation fail")
 			}
 		}
@@ -249,14 +242,13 @@ func (s *secureSession) runHandshake_xx(ctx context.Context, fallback bool, payl
 		// set remote libp2p public key
 		err = s.setRemotePeerInfo(nhp.GetIdentityKey())
 		if err != nil {
-			log.Errorf("runHandshake_xx stage=2 initiator=false set remote peer info err=%s", err)
 			return fmt.Errorf("runHandshake_xx stage=2 initiator=false read remote libp2p key fail")
 		}
 
-		// assert that remote peer ID matches libp2p key
+		// set remote libp2p public key from payload
 		err = s.setRemotePeerID(s.RemotePublicKey())
 		if err != nil {
-			log.Errorf("runHandshake_xx stage=2 initiator=false set remote peer id err=%s", err)
+			return fmt.Errorf("runHandshake_xx stage=2 initiator=false set remote peer id err=%s", err)
 		}
 
 		s.remote.noiseKey = s.xx_ns.RemoteKey()
@@ -264,7 +256,6 @@ func (s *secureSession) runHandshake_xx(ctx context.Context, fallback bool, payl
 		// verify payload is signed by libp2p key
 		err = s.verifyPayload(nhp, s.remote.noiseKey)
 		if err != nil {
-			log.Errorf("runHandshake_xx stage=2 initiator=false verify payload err=%s", err)
 			return fmt.Errorf("runHandshake_xx stage=2 initiator=false err=%s", err)
 		}