Browse Source

Fix: log correct form of broker IDs

- Also add more proxy retry comments
Rod Hynes 1 year ago
parent
commit
dffd28c73f
2 changed files with 26 additions and 7 deletions
  1. 6 3
      psiphon/common/inproxy/broker.go
  2. 20 4
      psiphon/inproxy.go

+ 6 - 3
psiphon/common/inproxy/broker.go

@@ -208,16 +208,19 @@ func NewBroker(config *BrokerConfig) (*Broker, error) {
 		return nil, errors.Trace(err)
 		return nil, errors.Trace(err)
 	}
 	}
 
 
-	// The broker ID is the broker's session public key.
+	// The broker ID is the broker's session public key in Curve25519 form.
 	publicKey, err := config.PrivateKey.GetPublicKey()
 	publicKey, err := config.PrivateKey.GetPublicKey()
 	if err != nil {
 	if err != nil {
 		return nil, errors.Trace(err)
 		return nil, errors.Trace(err)
 	}
 	}
-	brokerID := ID(publicKey)
+	brokerID, err := publicKey.ToCurve25519()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
 
 
 	b := &Broker{
 	b := &Broker{
 		config:            config,
 		config:            config,
-		brokerID:          brokerID,
+		brokerID:          ID(brokerID),
 		initiatorSessions: initiatorSessions,
 		initiatorSessions: initiatorSessions,
 		responderSessions: responderSessions,
 		responderSessions: responderSessions,
 		matcher: NewMatcher(&MatcherConfig{
 		matcher: NewMatcher(&MatcherConfig{

+ 20 - 4
psiphon/inproxy.go

@@ -443,10 +443,6 @@ func NewInproxyBrokerClientInstance(
 		brokerSpec = brokerSpecs[0]
 		brokerSpec = brokerSpecs[0]
 	}
 	}
 
 
-	// The broker ID is the broker's session public key.
-	brokerID := brokerSpec.BrokerPublicKey
-	NoticeInfo("inproxy: selected broker %s", brokerID)
-
 	// Generate new broker dial parameters if not replaying. Later, isReplay
 	// Generate new broker dial parameters if not replaying. Later, isReplay
 	// is used to report the replay metric.
 	// is used to report the replay metric.
 
 
@@ -568,6 +564,13 @@ func NewInproxyBrokerClientInstance(
 		return nil, errors.Trace(err)
 		return nil, errors.Trace(err)
 	}
 	}
 
 
+	// The broker ID is the broker's session public key in Curve25519 form.
+	brokerID, err := brokerPublicKey.ToCurve25519()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	NoticeInfo("inproxy: selected broker %s", inproxy.ID(brokerID))
+
 	return b, nil
 	return b, nil
 }
 }
 
 
@@ -894,6 +897,19 @@ func (b *InproxyBrokerClientInstance) BrokerClientRoundTripperFailed(roundTrippe
 	// A delay before retrying announce requests is appropriate, but there is
 	// A delay before retrying announce requests is appropriate, but there is
 	// no delay added here since Proxy.proxyOneClient already schedule delays
 	// no delay added here since Proxy.proxyOneClient already schedule delays
 	// between announcements.
 	// between announcements.
+	//
+	// Limitation: BrokerClientRoundTripperSucceeded is not invoked -- and no
+	// recent last success time is set -- for proxies which announce, don't
+	// match, and then hit the misaligned fronting provider request timeout
+	// issue. See the ""unexpected response status code" case and comment in
+	// InproxyBrokerRoundTripper.RoundTrip. This case should be mitigated by
+	// configuring InproxyFrontingProviderServerMaxRequestTimeouts.
+	//
+	// TODO: also retry after initial startup, with no previous success? This
+	// would further retain random load balancing of proxies newly starting
+	// at the same time that their initially selected broker is restarted or
+	// briefly unavailable.
+
 	if b.brokerClientManager.isProxy &&
 	if b.brokerClientManager.isProxy &&
 		!b.config.IsInproxyPersonalPairingMode() &&
 		!b.config.IsInproxyPersonalPairingMode() &&
 		b.retryOnFailedPeriod > 0 &&
 		b.retryOnFailedPeriod > 0 &&