Fix packed encoding of server entry boolean fields

- Also change TargetServerEntry case to use original JSON fields and not
  ServerEntry struct as signed server entry source.

psiphon/common/protocol/packed.go

@@ -808,15 +808,15 @@ func init() {
 		{26, "meekFrontingDomain", nil},
 		{27, "meekFrontingAddresses", nil},
 		{28, "meekFrontingAddressesRegex", nil},
-		{29, "meekFrontingDisableSNI", intConverter},
+		{29, "meekFrontingDisableSNI", nil},
 		{30, "tacticsRequestPublicKey", base64Converter},
 		{31, "tacticsRequestObfuscatedKey", base64Converter},
 		{32, "configurationVersion", nil},
 		{33, "signature", base64Converter},
-		{34, "disableHTTPTransforms", intConverter},
-		{35, "disableObfuscatedQUICTransforms", intConverter},
-		{36, "disableOSSHTransforms", intConverter},
-		{37, "disableOSSHPrefix", intConverter},
+		{34, "disableHTTPTransforms", nil},
+		{35, "disableObfuscatedQUICTransforms", nil},
+		{36, "disableOSSHTransforms", nil},
+		{37, "disableOSSHPrefix", nil},
 		{38, "inproxySessionPublicKey", unpaddedBase64Converter},
 		{39, "inproxySessionRootObfuscationSecret", unpaddedBase64Converter},
 		{40, "inproxySSHPort", nil},
@@ -826,7 +826,7 @@ func init() {
 		{44, "inproxyTlsOSSHPort", nil},
 		{45, "localSource", nil},
 		{46, "localTimestamp", nil},
-		{47, "isLocalDerivedTag", intConverter},
+		{47, "isLocalDerivedTag", nil},
 	}
 
 	for _, spec := range packedServerEntryFieldSpecs {

psiphon/common/protocol/serverEntry.go

@@ -434,6 +434,15 @@ func (fields ServerEntryFields) RemoveUnsignedFields() {
 	delete(fields, "isLocalDerivedTag")
 }
 
+// ToSignedFields checks for a signature and calls RemoveUnsignedFields.
+func (fields ServerEntryFields) ToSignedFields() error {
+	if !fields.HasSignature() {
+		return errors.TraceNew("missing signature field")
+	}
+	fields.RemoveUnsignedFields()
+	return nil
+}
+
 // NewServerEntrySignatureKeyPair creates an ed25519 key pair for use in
 // server entry signing and verification.
 func NewServerEntrySignatureKeyPair() (string, string, error) {
@@ -899,40 +908,6 @@ func (serverEntry *ServerEntry) HasSignature() bool {
 	return serverEntry.Signature != ""
 }
 
-// GetSignedServerEntryFields converts a signed ServerEntry into
-// ServerEntryFields.
-//
-// Limitation: a ServerEntry loaded from the local datastore may be missing
-// unmarshaled fields in the case where the server entry JSON contains fields
-// introduced in newer code; the ServerEntryFields in the db will contain
-// these fields, but not the ServerEntry. Prefer loading the
-// ServerEntryFields from psiphon.GetSignedServerEntryFields, which will
-// include all these fields, which may be included in the signature.
-//
-// GetSignedServerEntryFields is intended for use with limited cases such as
-// testing with TargetServerEntry.
-func (serverEntry *ServerEntry) GetSignedServerEntryFields() (ServerEntryFields, error) {
-
-	if !serverEntry.HasSignature() {
-		return nil, errors.TraceNew("missing signature")
-	}
-
-	serverEntryJSON, err := json.Marshal(serverEntry)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	var serverEntryFields ServerEntryFields
-	err = json.Unmarshal(serverEntryJSON, &serverEntryFields)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	serverEntryFields.RemoveUnsignedFields()
-
-	return serverEntryFields, nil
-}
-
 func (serverEntry *ServerEntry) GetDiagnosticID() string {
 	return TagToDiagnosticID(serverEntry.Tag)
 }

psiphon/dataStore.go

@@ -2092,14 +2092,11 @@ func GetSignedServerEntryFields(ipAddress string) (protocol.ServerEntryFields, e
 		return nil, errors.Trace(err)
 	}
 
-	if !serverEntryFields.HasSignature() {
-		return nil, errors.TraceNew("server entry not signed")
+	err = serverEntryFields.ToSignedFields()
+	if err != nil {
+		return nil, errors.Trace(err)
 	}
 
-	// RemoveUnsignedFields also removes potentially sensitive local fields, so
-	// explicitly strip these.
-	serverEntryFields.RemoveUnsignedFields()
-
 	return serverEntryFields, nil
 }
 

psiphon/dialParameters.go

@@ -1096,19 +1096,30 @@ func MakeDialParameters(
 		}
 
 		// Load the signed server entry to be presented to the broker as proof
-		// that the in-proxy destination is a Psiphon server. For all but
-		// TARGET server entries, load the JSON server entry fields from the
-		// local data store, since the signature may include fields, added
-		// after this client version, which are in the JSON but not in the
-		// protocol.ServerEntry. In the TARGET case, the protocol.ServerEntry
-		// is used as the source.
+		// that the in-proxy destination is a Psiphon server. The original
+		// JSON server entry fields are loaded from the local data store
+		// (or from config.TargetServerEntry), since the signature may
+		// include fields, added after this client version, which are in the
+		// JSON but not in the protocol.ServerEntry.
+
 		var serverEntryFields protocol.ServerEntryFields
 		if serverEntry.LocalSource == protocol.SERVER_ENTRY_SOURCE_TARGET {
-			serverEntryFields, err = serverEntry.GetSignedServerEntryFields()
+
+			serverEntryFields, err = protocol.DecodeServerEntryFields(
+				config.TargetServerEntry, "", protocol.SERVER_ENTRY_SOURCE_TARGET)
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+			if serverEntryFields.GetIPAddress() != serverEntry.IpAddress {
+				return nil, errors.TraceNew("unexpected TargetServerEntry")
+			}
+			err = serverEntryFields.ToSignedFields()
 			if err != nil {
 				return nil, errors.Trace(err)
 			}
+
 		} else {
+
 			serverEntryFields, err = GetSignedServerEntryFields(serverEntry.IpAddress)
 			if err != nil {
 				return nil, errors.Trace(err)