Inline documentation of some known issues

psiphon/common/obfuscator/obfuscator.go

@@ -44,6 +44,12 @@ const (
 // Obfuscator implements the seed message, key derivation, and
 // stream ciphers for:
 // https://github.com/brl/obfuscated-openssh/blob/master/README.obfuscation
+//
+// Limitation: the RC4 cipher is vulnerable to ciphertext malleability and
+// the "magic" value provides only weak authentication due to its small
+// size. Increasing the size of the magic field will break compatibility
+// with legacy clients. New protocols and schemes should not use this
+// obfuscator.
 type Obfuscator struct {
 	seedMessage          []byte
 	paddingLength        int
@@ -278,6 +284,11 @@ func readSeedMessage(
 
 	buffer := bytes.NewReader(fixedLengthFields)
 
+	// The magic value must be validated before acting on paddingLength as
+	// paddingLength validation is vulnerable to a chosen ciphertext probing
+	// attack: only a fixed number of any possible byte value for each
+	// paddingLength is valid.
+
 	var magicValue, paddingLength int32
 	err = binary.Read(buffer, binary.BigEndian, &magicValue)
 	if err != nil {

psiphon/controller.go

@@ -1346,6 +1346,16 @@ func (controller *Controller) getTactics(done chan struct{}) {
 	defer controller.establishWaitGroup.Done()
 	defer close(done)
 
+	// Limitation: GetNetworkID may not account for device VPN status, so
+	// Psiphon-over-Psiphon or Psiphon-over-other-VPN scenarios can encounter
+	// this issue:
+	//
+	// 1. Tactics are established when tunneling through a VPN and egressing
+	//    through a remote region/ISP.
+	// 2. Psiphon is next run when _not_ tunneling through the VPN. Yet the
+	//    network ID remains the same. Initial applied tactics will be for the
+	//    remote egress region/ISP, not the local region/ISP.
+
 	tacticsRecord, err := tactics.UseStoredTactics(
 		GetTacticsStorer(),
 		controller.config.GetNetworkID())