Refactor feedback upload mechanism

- Add feedback upload parameters to config and tactics
- Get tactics before uploading feedback
- Refactor Android and iOS library feedback upload mechanisms to
  remove dependence on PsiphonTunnel instance

MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -69,7 +69,11 @@ import psi.PsiphonProvider;
 
 public class PsiphonTunnel {
 
-    public interface HostService {
+    public interface HostServiceLogger {
+        default public void onDiagnosticMessage(String message) {}
+    }
+
+    public interface HostService extends HostServiceLogger {
 
         public String getAppName();
         public Context getContext();
@@ -77,7 +81,6 @@ public class PsiphonTunnel {
 
         default public Object getVpnService() {return null;} // Object must be a VpnService (Android < 4 cannot reference this class name)
         default public Object newVpnServiceBuilder() {return null;} // Object must be a VpnService.Builder (Android < 4 cannot reference this class name)
-        default public void onDiagnosticMessage(String message) {}
         default public void onAvailableEgressRegions(List<String> regions) {}
         default public void onSocksProxyPortInUse(int port) {}
         default public void onHttpProxyPortInUse(int port) {}
@@ -292,12 +295,21 @@ public class PsiphonTunnel {
         return Psi.importExchangePayload(payload);
     }
 
-    public static void sendFeedback(String configJson, String diagnosticsJson, String b64EncodedPublicKey, String uploadServer,
-                             String uploadPath, String uploadServerHeaders) throws Exception {
+    // Upload a feedback package to Psiphon Inc. The app collects feedback and diagnostics
+    // information in a particular format, then calls this function to upload it for later
+    // investigation. The feedback compatible config and upload path must be provided by
+    // Psiphon Inc.
+    public static void sendFeedback(Context context, HostServiceLogger logger, String feedbackConfigJson,
+                                    String diagnosticsJson, String uploadPath,
+                                    String clientPlatformPrefix, String clientPlatformSuffix) throws Exception {
+
         try {
-            Psi.sendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer, uploadPath, uploadServerHeaders);
+            // Adds fields used in feedback upload, e.g. client platform.
+            String psiphonConfig = buildPsiphonConfig(context, logger, feedbackConfigJson,
+                    clientPlatformPrefix, clientPlatformSuffix, false, 0);
+            Psi.sendFeedback(psiphonConfig, diagnosticsJson, uploadPath);
         } catch (java.lang.Exception e) {
-            throw new Exception("failed to send feedback", e);
+            throw new Exception("Error sending feedback", e);
         }
     }
 
@@ -624,9 +636,19 @@ public class PsiphonTunnel {
     private String loadPsiphonConfig(Context context)
             throws IOException, JSONException, Exception {
 
+        return buildPsiphonConfig(context, mHostService, mHostService.getPsiphonConfig(),
+                mClientPlatformPrefix.get(), mClientPlatformSuffix.get(), isVpnMode(),
+                mLocalSocksProxyPort.get());
+    }
+
+    private static String buildPsiphonConfig(Context context, HostServiceLogger logger, String psiphonConfig,
+                                             String clientPlatformPrefix, String clientPlatformSuffix,
+                                             boolean isVpnMode, Integer localSocksProxyPort)
+            throws IOException, JSONException, Exception {
+
         // Load settings from the raw resource JSON config file and
         // update as necessary. Then write JSON to disk for the Go client.
-        JSONObject json = new JSONObject(mHostService.getPsiphonConfig());
+        JSONObject json = new JSONObject(psiphonConfig);
 
         // On Android, this directory must be set to the app private storage area.
         // The Psiphon library won't be able to use its current working directory
@@ -667,46 +689,44 @@ public class PsiphonTunnel {
 
         // This parameter is for stats reporting
         if (!json.has("TunnelWholeDevice")) {
-            json.put("TunnelWholeDevice", isVpnMode() ? 1 : 0);
+            json.put("TunnelWholeDevice", isVpnMode ? 1 : 0);
         }
 
         json.put("EmitBytesTransferred", true);
 
-        if (mLocalSocksProxyPort.get() != 0 && (!json.has("LocalSocksProxyPort") || json.getInt("LocalSocksProxyPort") == 0)) {
+        if (localSocksProxyPort != 0 && (!json.has("LocalSocksProxyPort") || json.getInt("LocalSocksProxyPort") == 0)) {
             // When mLocalSocksProxyPort is set, tun2socks is already configured
             // to use that port value. So we force use of the same port.
             // A side-effect of this is that changing the SOCKS port preference
             // has no effect with restartPsiphon(), a full stop() is necessary.
-            json.put("LocalSocksProxyPort", mLocalSocksProxyPort);
+            json.put("LocalSocksProxyPort", localSocksProxyPort);
         }
 
         if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) {
             try {
                 json.put(
                         "TrustedCACertificatesFilename",
-                        setupTrustedCertificates(mHostService.getContext()));
+                        setupTrustedCertificates(context, logger));
             } catch (Exception e) {
-                mHostService.onDiagnosticMessage(e.getMessage());
+                logger.onDiagnosticMessage(e.getMessage());
             }
         }
 
-        json.put("DeviceRegion", getDeviceRegion(mHostService.getContext()));
+        json.put("DeviceRegion", getDeviceRegion(context));
 
         StringBuilder clientPlatform = new StringBuilder();
 
-        String prefix = mClientPlatformPrefix.get();
-        if (prefix.length() > 0) {
-            clientPlatform.append(prefix);
+        if (clientPlatformPrefix.length() > 0) {
+            clientPlatform.append(clientPlatformPrefix);
         }
 
         clientPlatform.append("Android_");
         clientPlatform.append(Build.VERSION.RELEASE);
         clientPlatform.append("_");
-        clientPlatform.append(mHostService.getContext().getPackageName());
+        clientPlatform.append(context.getPackageName());
 
-        String suffix = mClientPlatformSuffix.get();
-        if (suffix.length() > 0) {
-            clientPlatform.append(suffix);
+        if (clientPlatformSuffix.length() > 0) {
+            clientPlatform.append(clientPlatformSuffix);
         }
 
         json.put("ClientPlatform", clientPlatform.toString().replaceAll("[^\\w\\-\\.]", "_"));
@@ -811,7 +831,7 @@ public class PsiphonTunnel {
         }
     }
 
-    private String setupTrustedCertificates(Context context) throws Exception {
+    private static String setupTrustedCertificates(Context context, HostServiceLogger logger) throws Exception {
 
         // Copy the Android system CA store to a local, private cert bundle file.
         //
@@ -872,7 +892,7 @@ public class PsiphonTunnel {
                     output.println("-----END CERTIFICATE-----");
                 }
 
-                mHostService.onDiagnosticMessage("prepared PsiphonCAStore");
+                logger.onDiagnosticMessage("prepared PsiphonCAStore");
 
                 return file.getAbsolutePath();
 

MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.h

@@ -54,6 +54,20 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
     PsiphonConnectionStateWaitingForNetwork
 };
 
+@protocol TunneledAppDelegateLogger <NSObject>
+
+@optional
+
+/*!
+ Gets runtime errors info that may be useful for debugging.
+ @param message  The diagnostic message string.
+ @param timestamp RFC3339 encoded timestamp.
+ Swift: @code func onDiagnosticMessage(_ message: String, withTimestamp timestamp: String) @endcode
+ */
+- (void)onDiagnosticMessage:(NSString * _Nonnull)message withTimestamp:(NSString * _Nonnull)timestamp;
+
+@end
+
 
 /*!
  @protocol TunneledAppDelegate
@@ -62,7 +76,7 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
 
  All delegate methods will be called on a single serial dispatch queue. They will be made asynchronously unless otherwise noted (specifically when calling getPsiphonConfig and getEmbeddedServerEntries).
  */
-@protocol TunneledAppDelegate <NSObject>
+@protocol TunneledAppDelegate <NSObject, TunneledAppDelegateLogger>
 
 //
 // Required delegate methods
@@ -139,14 +153,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
  */
 - (NSString * _Nullable)getEmbeddedServerEntriesPath;
 
-/*!
- Gets runtime errors info that may be useful for debugging.
- @param message  The diagnostic message string.
- @param timestamp RFC3339 encoded timestamp.
- Swift: @code func onDiagnosticMessage(_ message: String) @endcode
- */
-- (void)onDiagnosticMessage:(NSString * _Nonnull)message withTimestamp:(NSString * _Nonnull)timestamp;
-
 /*!
  Called when the tunnel is in the process of connecting.
  Swift: @code func onConnecting() @endcode
@@ -452,17 +458,19 @@ Returns the path where the rotated notices file will be created.
 
 /*!
  Upload a feedback package to Psiphon Inc. The app collects feedback and diagnostics information in a particular format, then calls this function to upload it for later investigation.
- @note The key, server, path, and headers must be provided by Psiphon Inc.
- @param feedbackJson  The feedback and diagnostics data to upload.
- @param b64EncodedPublicKey  The key that will be used to encrypt the payload before uploading.
- @param uploadServer  The server and path to which the data will be uploaded.
- @param uploadServerHeaders  The request headers that will be used when uploading.
- Swift: @code func sendFeedback(_ feedbackJson: String, publicKey b64EncodedPublicKey: String, uploadServer: String, uploadServerHeaders: String) @endcode
- */
-- (void)sendFeedback:(NSString * _Nonnull)feedbackJson
-           publicKey:(NSString * _Nonnull)b64EncodedPublicKey
-        uploadServer:(NSString * _Nonnull)uploadServer
- uploadServerHeaders:(NSString * _Nonnull)uploadServerHeaders;
+ @param feedbackJson  The feedback data to upload.
+ @param feedbackConfigJson  The feedback compatible config. Must be an NSDictionary or NSString. Must be provided by Psiphon Inc.
+ @param uploadPath  The path at which to upload the diagnostic data. Must be provided by Psiphon Inc.
+ @param logger  The logger which will be used to log informational notices including warnings.
+ @param outError  Any error encountered while sending feedback. If set, then sending feedback failed.
+ Swift: @code func sendFeedback(_ feedbackJson: String, feedbackConfigJson: Any, uploadPath: String, logger: TunneledAppDelegateLogger?, error outError: NSErrorPointer) @endcode
+ */
+// See comment in header.
++ (void)sendFeedback:(NSString * _Nonnull)feedbackJson
+  feedbackConfigJson:(id _Nonnull)feedbackConfigJson
+          uploadPath:(NSString * _Nonnull)uploadPath
+              logger:(id<TunneledAppDelegateLogger> _Nullable)logger
+               error:(NSError * _Nullable * _Nonnull)outError;
 
 /*!
  Provides the tunnel-core build info json as a string. See the tunnel-core build info code for details https://github.com/Psiphon-Labs/psiphon-tunnel-core/blob/master/psiphon/common/buildinfo.go.

MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -47,13 +47,43 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     PsiphonTunnelErrorCodeUnknown = -1,
 
     /*!
-     * An error was encountered either obtaining the default library directory.
+     * An error was encountered obtaining the default library directory.
      * @code
      * // Underlying error will be set with more information
      * [error.userInfo objectForKey:NSUnderlyingErrorKey]
      * @endcode
      */
     PsiphonTunnelErrorCodeLibraryDirectoryError,
+
+    /*!
+     * An error was encountered with the provided config.
+     * @code
+     * // Localized description will be set with more information.
+     * // Underlying error may be set with more information.
+     * [error.userInfo objectForKey:NSUnderlyingErrorKey]
+     * error.localizedDescription
+     * @endcode
+     */
+    PsiphonTunnelErrorCodeConfigError,
+
+    /*!
+     * An error was encountered while generating the session ID.
+     * @code
+     * // Localized description will be set with more information.
+     * error.localizedDescription
+     * @endcode
+     */
+    PsiphonTunnelErrorCodeGenerateSessionIDError,
+
+    /*!
+     * An error was encountered while sending feedback.
+     * @code
+     * // Localized description and underlying error will be set with more information.
+     * [error.userInfo objectForKey:NSUnderlyingErrorKey]
+     * error.localizedDescription
+     * @endcode
+     */
+    PsiphonTunnelErrorCodeSendFeedbackError,
 };
 
 @interface PsiphonTunnel () <GoPsiPsiphonProvider>
@@ -122,14 +152,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     self->initialDNSCache = [self getDNSServers];
     atomic_init(&self->useInitialDNS, [self->initialDNSCache count] > 0);
 
-    // RFC3339 formatter.
-    NSLocale *enUSPOSIXLocale = [NSLocale localeWithLocaleIdentifier:@"en_US_POSIX"];
-    rfc3339Formatter = [[NSDateFormatter alloc] init];
-    [rfc3339Formatter setLocale:enUSPOSIXLocale];
-    
-    // Example: notice time format from Go code: "2006-01-02T15:04:05.999Z07:00"
-    [rfc3339Formatter setDateFormat:@"yyyy'-'MM'-'dd'T'HH':'mm':'ss.SSSZZZZZ"];
-    [rfc3339Formatter setTimeZone:[NSTimeZone timeZoneForSecondsFromGMT:0]];
+    rfc3339Formatter = [PsiphonTunnel rfc3339Formatter];
     
     return self;
 }
@@ -186,9 +209,10 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 - (BOOL)start:(BOOL)ifNeeded {
 
     // Set a new session ID, as this is a user-initiated session start.
-    NSString *sessionID = [self generateSessionID];
-    if (sessionID == nil) {
-        // generateSessionID logs error message
+    NSError *err;
+    NSString *sessionID = [PsiphonTunnel generateSessionID:&err];
+    if (err != nil) {
+        [self logMessage:[NSString stringWithFormat:@"%@", err.localizedDescription]];
         return FALSE;
     }
     self.sessionID = sessionID;
@@ -250,9 +274,14 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         const BOOL useIPv6Synthesizer = TRUE;
 
         BOOL usingNoticeFiles = FALSE;
-        
-        NSString *configStr = [self getConfig: &usingNoticeFiles];
-        if (configStr == nil) {
+
+        NSError *err;
+        NSString *configStr = [self getConfig:&usingNoticeFiles error:&err];
+        if (err != nil) {
+            [self logMessage:[NSString stringWithFormat:@"Error getting config: %@", err.localizedDescription]];
+            return FALSE;
+        } else if (configStr == nil) {
+            // Should never happen.
             [self logMessage:@"Error getting config"];
             return FALSE;
         }
@@ -270,7 +299,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
                 }
             });
         }
-        
+
         // If getEmbeddedServerEntriesPath returns an empty string,
         // call getEmbeddedServerEntries
         if ([embeddedServerEntriesPath length] == 0) {
@@ -426,29 +455,59 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 }
 
 // See comment in header.
-- (void)sendFeedback:(NSString * _Nonnull)feedbackJson
-           publicKey:(NSString * _Nonnull)b64EncodedPublicKey
-        uploadServer:(NSString * _Nonnull)uploadServer
- uploadServerHeaders:(NSString * _Nonnull)uploadServerHeaders {
-    dispatch_async(self->workQueue, ^{
-
-        BOOL usingNoticeFiles = FALSE;
-
-        NSString *connectionConfigJson = [self getConfig: &usingNoticeFiles];
-        if (connectionConfigJson == nil) {
-           [self logMessage:@"Error getting config for feedback upload"];
++ (void)sendFeedback:(NSString * _Nonnull)feedbackJson
+  feedbackConfigJson:(id _Nonnull)feedbackConfigJson
+          uploadPath:(NSString * _Nonnull)uploadPath
+              logger:(id<TunneledAppDelegateLogger> _Nullable)logger
+               error:(NSError * _Nullable * _Nonnull)outError {
+
+    *outError = nil;
+
+    void (^logMessage)(NSString * _Nonnull) = ^void(NSString * _Nonnull message) {
+        if (logger != nil && [logger respondsToSelector:@selector(onDiagnosticMessage:withTimestamp:)]) {
+            NSString *timestamp = [[PsiphonTunnel rfc3339Formatter] stringFromDate:[NSDate date]];
+            [logger onDiagnosticMessage:message withTimestamp:timestamp];
         }
+    };
+
+    NSError *err;
+    NSString *sessionID = [PsiphonTunnel generateSessionID:&err];
+    if (err != nil) {
+        *outError = err;
+        return;
+    }
 
-        NSError *e;
+    BOOL usingNoticeFiles = FALSE;
+    BOOL tunnelWholeDevice = FALSE;
+    NSString *psiphonConfig = [PsiphonTunnel buildPsiphonConfig:feedbackConfigJson
+                                               usingNoticeFiles:&usingNoticeFiles
+                                              tunnelWholeDevice:&tunnelWholeDevice
+                                                      sessionID:sessionID
+                                                     logMessage:logMessage
+                                                          error:&err];
+    if (err != nil) {
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Error building config",
+                                               NSUnderlyingErrorKey:err}];
+        return;
+    } else if (psiphonConfig == nil) {
+        // Should never happen.
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Error built config nil"}];
+        return;
+    }
 
-        GoPsiSendFeedback(connectionConfigJson, feedbackJson, b64EncodedPublicKey, uploadServer, @"", uploadServerHeaders, &e);
+    GoPsiSendFeedback(psiphonConfig, feedbackJson, uploadPath, &err);
 
-        if (e != nil) {
-            [self logMessage:[NSString stringWithFormat: @"Feedback upload error: %@", e.localizedDescription]];
-        } else {
-            [self logMessage:@"Feedback upload successful"];
-        }
-    });
+    if (err != nil) {
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeSendFeedbackError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Error sending feedback",
+                                               NSUnderlyingErrorKey:err}];
+        return;
+    }
 }
 
 // See comment in header.
@@ -490,7 +549,11 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
  Build the config string for the tunnel.
  @returns String containing the JSON config. `nil` on error.
  */
-- (NSString * _Nullable)getConfig:(BOOL * _Nonnull)usingNoticeFiles {
+- (NSString * _Nullable)getConfig:(BOOL * _Nonnull)usingNoticeFiles
+                            error:(NSError *_Nullable *_Nonnull)outError {
+
+    *outError = nil;
+
     // tunneledAppDelegate is a weak reference, so check it.
     if (self.tunneledAppDelegate == nil) {
         [self logMessage:@"tunneledApp delegate lost"];
@@ -501,11 +564,45 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     dispatch_sync(self->callbackQueue, ^{
         configObject = [self.tunneledAppDelegate getPsiphonConfig];
     });
-    
+
+    __weak PsiphonTunnel *weakSelf = self;
+    void (^logMessage)(NSString * _Nonnull) = ^void(NSString * _Nonnull message) {
+        __strong PsiphonTunnel *strongSelf = weakSelf;
+        if (strongSelf != nil) {
+            [strongSelf logMessage:message];
+        }
+    };
+
     if (configObject == nil) {
-        [self logMessage:@"Error getting config from delegate"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Error config object nil"}];
         return nil;
     }
+
+    NSError *err;
+    NSString *psiphonConfig = [PsiphonTunnel buildPsiphonConfig:configObject
+                                               usingNoticeFiles:usingNoticeFiles
+                                              tunnelWholeDevice:&self->tunnelWholeDevice
+                                                      sessionID:self.sessionID
+                                                     logMessage:logMessage
+                                                          error:&err];
+    if (err != nil) {
+        *outError = err;
+        return nil;
+    }
+
+    return psiphonConfig;
+}
+
++ (NSString * _Nullable)buildPsiphonConfig:(id _Nonnull)configObject
+                          usingNoticeFiles:(BOOL * _Nonnull)usingNoticeFiles
+                         tunnelWholeDevice:(BOOL * _Nonnull)tunnelWholeDevice
+                                 sessionID:(NSString * _Nonnull)sessionID
+                                logMessage:(void (^)(NSString * _Nonnull))logMessage
+                                     error:(NSError *_Nullable *_Nonnull)outError {
+
+    *outError = nil;
     
     __block NSDictionary *initialConfig = nil;
     
@@ -520,7 +617,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         
         id eh = ^(NSError *err) {
             initialConfig = nil;
-            [self logMessage:[NSString stringWithFormat: @"Config JSON parse failed: %@", err.description]];
+            logMessage([NSString stringWithFormat: @"Config JSON parse failed: %@", err.description]);
         };
         
         id parser = [SBJson4Parser parserWithBlock:block allowMultiRoot:NO unwrapRootArray:NO errorHandler:eh];
@@ -528,13 +625,14 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         
     } else if ([configObject isKindOfClass:[NSDictionary class]]) {
         
-        initialConfig = (NSDictionary *) configObject;
+        initialConfig = (NSDictionary *)configObject;
         
     } else {
-        [self logMessage:@"getPsiphonConfig should either return an NSDictionary object or an NSString object"];
-    }
-    
-    if (initialConfig == nil) {
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:
+                                                   @"getPsiphonConfig should either return an "
+                                                    "NSDictionary object or an NSString object"}];
         return nil;
     }
     
@@ -545,12 +643,18 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     //
     
     if (config[@"PropagationChannelId"] == nil) {
-        [self logMessage:@"Config missing PropagationChannelId"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:
+                                                   @"Config missing PropagationChannelId"}];
         return nil;
     }
 
     if (config[@"SponsorId"] == nil) {
-        [self logMessage:@"Config missing SponsorId"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:
+                                                   @"Config missing SponsorId"}];
         return nil;
     }
     
@@ -575,8 +679,12 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     // Note: this deprecates the "DataStoreDirectory" config field.
     NSURL *defaultDataRootDirectoryURL = [PsiphonTunnel defaultDataRootDirectoryWithError:&err];
     if (err != nil) {
-       [self logMessage:[NSString stringWithFormat:@"Unable to get defaultDataRootDirectoryURL: %@", err.localizedDescription]];
-       return nil;
+        NSString *s = [NSString stringWithFormat:@"Unable to get defaultDataRootDirectoryURL: %@",
+                       err.localizedDescription];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:s}];
+        return nil;
     }
 
     if (config[@"DataRootDirectory"] == nil) {
@@ -588,14 +696,18 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
                                attributes:nil
                                     error:&err];
         if (err != nil) {
-           [self logMessage:[NSString stringWithFormat: @"Unable to create defaultRootDirectoryURL '%@': %@", defaultDataRootDirectoryURL, err.localizedDescription]];
-           return nil;
+            NSString *s = [NSString stringWithFormat: @"Unable to create defaultRootDirectoryURL '%@': %@",
+                           defaultDataRootDirectoryURL, err.localizedDescription];
+            *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                            code:PsiphonTunnelErrorCodeConfigError
+                                        userInfo:@{NSLocalizedDescriptionKey:s}];
+            return nil;
         }
 
         config[@"DataRootDirectory"] = defaultDataRootDirectoryURL.path;
     }
     else {
-        [self logMessage:[NSString stringWithFormat:@"DataRootDirectory overridden from '%@' to '%@'", defaultDataRootDirectoryURL.path, config[@"DataRootDirectory"]]];
+        logMessage([NSString stringWithFormat:@"DataRootDirectory overridden from '%@' to '%@'", defaultDataRootDirectoryURL.path, config[@"DataRootDirectory"]]);
     }
 
     // Ensure that the configured data root directory is not backed up to iCloud or iTunes.
@@ -603,10 +715,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 
     BOOL succeeded = [Backups excludeFileFromBackup:dataRootDirectory.path err:&err];
     if (!succeeded) {
-        NSString *msg = [NSString stringWithFormat:@"Failed to exclude data root directory from backup: %@", err.localizedDescription];
-        [self logMessage:msg];
+        logMessage([NSString stringWithFormat:@"Failed to exclude data root directory from backup: %@", err.localizedDescription]);
     } else {
-        [self logMessage:@"Excluded data root directory from backup"];
+        logMessage(@"Excluded data root directory from backup");
     }
 
     //
@@ -615,7 +726,10 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 
     NSURL *libraryURL = [PsiphonTunnel libraryURLWithError:&err];
     if (err != nil) {
-        [self logMessage:[NSString stringWithFormat: @"Unable to get Library URL: %@", err.localizedDescription]];
+        NSString *s = [NSString stringWithFormat: @"Unable to get Library URL: %@", err.localizedDescription];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:s}];
         return nil;
     }
 
@@ -631,7 +745,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
                                                                       isDirectory:YES];
     
     if (defaultDataStoreDirectoryURL == nil) {
-        [self logMessage:@"Unable to create defaultDataStoreDirectoryURL"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Unable to create defaultDataStoreDirectoryURL"}];
         return nil;
     }
     
@@ -639,7 +755,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         config[@"MigrateDataStoreDirectory"] = defaultDataStoreDirectoryURL.path;
     }
     else {
-        [self logMessage:[NSString stringWithFormat: @"DataStoreDirectory overridden from '%@' to '%@'", [defaultDataStoreDirectoryURL path], config[@"DataStoreDirectory"]]];
+        logMessage([NSString stringWithFormat: @"DataStoreDirectory overridden from '%@' to '%@'", [defaultDataStoreDirectoryURL path], config[@"DataStoreDirectory"]]);
     }
 
     //
@@ -654,7 +770,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     // explict new field "MigrateRemoteServerListDownloadFilename".
     NSString *defaultRemoteServerListFilename = [[libraryURL URLByAppendingPathComponent:@"remote_server_list" isDirectory:NO] path];
     if (defaultRemoteServerListFilename == nil) {
-        [self logMessage:@"Unable to create defaultRemoteServerListFilename"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Unable to create defaultRemoteServerListFilename"}];
         return nil;
     }
     
@@ -662,14 +780,15 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         config[@"MigrateRemoteServerListDownloadFilename"] = defaultRemoteServerListFilename;
     }
     else {
-        [self logMessage:[NSString stringWithFormat: @"RemoteServerListDownloadFilename overridden from '%@' to '%@'", defaultRemoteServerListFilename, config[@"RemoteServerListDownloadFilename"]]];
+        logMessage([NSString stringWithFormat: @"RemoteServerListDownloadFilename overridden from '%@' to '%@'",
+                defaultRemoteServerListFilename, config[@"RemoteServerListDownloadFilename"]]);
     }
     
     // If RemoteServerListUrl/RemoteServerListURLs and RemoteServerListSignaturePublicKey
     // are absent, we'll just leave them out, but we'll log about it.
     if ((config[@"RemoteServerListUrl"] == nil && config[@"RemoteServerListURLs"] == nil) ||
         config[@"RemoteServerListSignaturePublicKey"] == nil) {
-        [self logMessage:@"Remote server list functionality will be disabled"];
+        logMessage(@"Remote server list functionality will be disabled");
     }
     
     //
@@ -684,20 +803,23 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     // more explict new field "MigrateObfuscatedServerListDownloadDirectory".
     NSURL *defaultOSLDirectoryURL = [libraryURL URLByAppendingPathComponent:@"osl" isDirectory:YES];
     if (defaultOSLDirectoryURL == nil) {
-        [self logMessage:@"Unable to create defaultOSLDirectory"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Unable to create defaultOSLDirectory"}];
         return nil;
     }
     if (config[@"ObfuscatedServerListDownloadDirectory"] == nil) {
         config[@"MigrateObfuscatedServerListDownloadDirectory"] = defaultOSLDirectoryURL.path;
     }
     else {
-        [self logMessage:[NSString stringWithFormat: @"ObfuscatedServerListDownloadDirectory overridden from '%@' to '%@'", [defaultOSLDirectoryURL path], config[@"ObfuscatedServerListDownloadDirectory"]]];
+        logMessage([NSString stringWithFormat: @"ObfuscatedServerListDownloadDirectory overridden from '%@' to '%@'",
+                [defaultOSLDirectoryURL path], config[@"ObfuscatedServerListDownloadDirectory"]]);
     }
     
     // If ObfuscatedServerListRootURL/ObfuscatedServerListRootURLs is absent,
     // we'll leave it out, but log the absence.
     if (config[@"ObfuscatedServerListRootURL"] == nil && config[@"ObfuscatedServerListRootURLs"] == nil) {
-        [self logMessage:@"Obfuscated server list functionality will be disabled"];
+        logMessage(@"Obfuscated server list functionality will be disabled");
     }
 
     //
@@ -705,7 +827,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     //
 
     // We'll record our state about what mode we're in.
-    self->tunnelWholeDevice = ([config[@"TunnelWholeDevice"] integerValue] == 1);
+    *tunnelWholeDevice = ([config[@"TunnelWholeDevice"] integerValue] == 1);
 
     // Other optional fields not being altered. If not set, their defaults will be used:
     // * TunnelWholeDevice
@@ -765,7 +887,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     config[@"UpgradeDownloadClientVersionHeader"] = nil;
     config[@"UpgradeDownloadFilename"] = nil;
 
-    config[@"SessionID"] = self.sessionID;
+    config[@"SessionID"] = sessionID;
 
     // Indicate whether UseNoticeFiles is set
     *usingNoticeFiles = (config[@"UseNoticeFiles"] != nil);
@@ -773,7 +895,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     NSString *finalConfigStr = [[[SBJson4Writer alloc] init] stringWithObject:config];
     
     if (finalConfigStr == nil) {
-        [self logMessage:@"Failed to convert config to JSON string"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"Failed to convert config to JSON string"}];
         return nil;
     }
 
@@ -1512,15 +1636,35 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     return @"US";
 }
 
+// RFC3339 formatter.
++ (NSDateFormatter*)rfc3339Formatter {
+
+    NSDateFormatter *rfc3339Formatter = [[NSDateFormatter alloc] init];
+    NSLocale *enUSPOSIXLocale = [NSLocale localeWithLocaleIdentifier:@"en_US_POSIX"];
+    [rfc3339Formatter setLocale:enUSPOSIXLocale];
+
+    // Example: notice time format from Go code: "2006-01-02T15:04:05.999Z07:00"
+    [rfc3339Formatter setDateFormat:@"yyyy'-'MM'-'dd'T'HH':'mm':'ss.SSSZZZZZ"];
+    [rfc3339Formatter setTimeZone:[NSTimeZone timeZoneForSecondsFromGMT:0]];
+
+    return rfc3339Formatter;
+}
+
 /*!
  generateSessionID generates a session ID suitable for use with the Psiphon API.
  */
-- (NSString *)generateSessionID {
++ (NSString *)generateSessionID:(NSError *_Nullable *_Nonnull)outError {
+
+    *outError = nil;
+
     const int sessionIDLen = 16;
     uint8_t sessionID[sessionIDLen];
     int result = SecRandomCopyBytes(kSecRandomDefault, sessionIDLen, sessionID);
     if (result != errSecSuccess) {
-        [self logMessage:[NSString stringWithFormat: @"Error generating session ID: %d", result]];
+        NSString *errorDescription = [NSString stringWithFormat:@"Error generating session ID: %d", result];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeGenerateSessionIDError
+                                    userInfo:@{NSLocalizedDescriptionKey:errorDescription}];
         return nil;
     }
     NSMutableString *hexEncodedSessionID = [NSMutableString stringWithCapacity:(sessionIDLen*2)];

MobileLibrary/psi/psi.go

@@ -291,8 +291,8 @@ func ImportExchangePayload(payload string) bool {
 }
 
 // Encrypt and upload feedback.
-func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer, uploadPath, uploadServerHeaders string) error {
-	return psiphon.SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer, uploadPath, uploadServerHeaders)
+func SendFeedback(configJson, diagnosticsJson, uploadPath string) error {
+	return psiphon.SendFeedback(configJson, diagnosticsJson, uploadPath)
 }
 
 // Get build info from tunnel-core

psiphon/common/parameters/clientParameters.go

@@ -237,6 +237,11 @@ const (
 	BPFServerTCPProbability                          = "BPFServerTCPProbability"
 	BPFClientTCPProgram                              = "BPFClientTCPProgram"
 	BPFClientTCPProbability                          = "BPFClientTCPProbability"
+	FeedbackUploadURLs                               = "FeedbackUploadURLs"
+	FeedbackTacticsWaitPeriod                        = "FeedbackTacticsWaitPeriod"
+	FeedbackUploadMaxRetries                         = "FeedbackUploadMaxRetries"
+	FeedbackUploadRetryDelaySeconds                  = "FeedbackUploadRetryDelaySeconds"
+	FeedbackUploadTimeoutSeconds                     = "FeedbackUploadTimeoutSeconds"
 )
 
 const (
@@ -491,6 +496,12 @@ var defaultClientParameters = map[string]struct {
 	BPFServerTCPProbability: {value: 0.5, minimum: 0.0},
 	BPFClientTCPProgram:     {value: (*BPFProgramSpec)(nil)},
 	BPFClientTCPProbability: {value: 0.5, minimum: 0.0},
+
+	FeedbackUploadURLs:              {value: SecureTransferURLs{}},
+	FeedbackTacticsWaitPeriod:       {value: 5 * time.Second, minimum: 0 * time.Second, flags: useNetworkLatencyMultiplier},
+	FeedbackUploadMaxRetries:        {value: 5, minimum: 0},
+	FeedbackUploadRetryDelaySeconds: {value: 300 * time.Second, minimum: 0 * time.Second, flags: useNetworkLatencyMultiplier},
+	FeedbackUploadTimeoutSeconds:    {value: 30 * time.Second, minimum: 0 * time.Second, flags: useNetworkLatencyMultiplier},
 }
 
 // IsServerSideOnly indicates if the parameter specified by name is used
@@ -663,6 +674,14 @@ func (p *ClientParameters) Set(
 					}
 					return nil, errors.Trace(err)
 				}
+			case SecureTransferURLs:
+				err := v.DecodeAndValidate()
+				if err != nil {
+					if skipOnError {
+						continue
+					}
+					return nil, errors.Trace(err)
+				}
 			case protocol.TunnelProtocols:
 				if skipOnError {
 					newValue = v.PruneInvalid()
@@ -1086,6 +1105,13 @@ func (p ClientParametersAccessor) TransferURLs(name string) TransferURLs {
 	return value
 }
 
+// SecureTransferURLs returns a SecureTransferURLs parameter value.
+func (p ClientParametersAccessor) SecureTransferURLs(name string) SecureTransferURLs {
+	value := SecureTransferURLs{}
+	p.snapshot.getValue(name, &value)
+	return value
+}
+
 // RateLimits returns a common.RateLimits parameter value.
 func (p ClientParametersAccessor) RateLimits(name string) common.RateLimits {
 	value := common.RateLimits{}

psiphon/common/parameters/transferURLs.go

@@ -34,7 +34,7 @@ type TransferURL struct {
 	// with base64 encoding to mitigate trivial binary executable string scanning.
 	URL string
 
-	// SkipVerify indicates whether to verify HTTPS certificates. It some
+	// SkipVerify indicates whether to verify HTTPS certificates. In some
 	// circumvention scenarios, verification is not possible. This must
 	// only be set to true when the resource has its own verification mechanism.
 	SkipVerify bool
@@ -49,7 +49,7 @@ type TransferURL struct {
 // TransferURLs is a list of transfer URLs.
 type TransferURLs []*TransferURL
 
-// DecodeAndValidate validates a list of download URLs.
+// DecodeAndValidate validates a list of transfer URLs.
 //
 // At least one TransferURL in the list must have OnlyAfterAttempts of 0,
 // or no TransferURL would be selected on the first attempt.
@@ -116,3 +116,67 @@ func (t TransferURLs) Select(attempt int) (string, string, bool) {
 
 	return transferURL.URL, canonicalURL, transferURL.SkipVerify
 }
+
+// SecureTransferURL specifies a URL for uploading or downloading a resource
+// along with a public key for encrypting the resource, when uploading, or
+// for verifying a signature of the resource, when downloading.
+type SecureTransferURL struct {
+	B64EncodedPublicKey string
+	RequestHeaders      map[string]string
+	TransferURL         TransferURL
+}
+
+// SecureTransferURLs is a list of secure transfer URLs.
+type SecureTransferURLs []*SecureTransferURL
+
+// TransferURLs returns the underlying TransferURLs with ordering preserved.
+func (t SecureTransferURLs) TransferURLs() (TransferURLs, error) {
+	transferURLs := make(TransferURLs, len(t))
+	for i, secureTransferURL := range t {
+		if secureTransferURL == nil {
+			return nil, errors.TraceNew("unexpected nil SecureTransferURL")
+		}
+		transferURLs[i] = &secureTransferURL.TransferURL
+	}
+	return transferURLs, nil
+}
+
+// DecodeAndValidate validates a list of secure transfer URLs.
+func (t SecureTransferURLs) DecodeAndValidate() error {
+	transferURLs, err := t.TransferURLs()
+	if err != nil {
+		return errors.Trace(err)
+	}
+	err = transferURLs.DecodeAndValidate()
+	if err != nil {
+		return errors.Trace(err)
+	}
+	return nil
+}
+
+// Select chooses a SecureTransferURL from the list.
+//
+// The secure transfer URL is selected based at random from the candidates
+// allowed in the specified attempt.
+func (t SecureTransferURLs) Select(attempt int) (*SecureTransferURL, error) {
+	candidates := make([]int, 0)
+	for index, secureTransferURL := range t {
+		if secureTransferURL == nil {
+			return nil, errors.TraceNew("unexpected nil SecureTransferURL")
+		}
+		if attempt >= secureTransferURL.TransferURL.OnlyAfterAttempts {
+			candidates = append(candidates, index)
+		}
+	}
+
+	if len(candidates) < 1 {
+		// This case is not expected, as DecodeAndValidate should reject configs
+		// that would have no candidates for 0 attempts.
+		return nil, errors.TraceNew("no candiates found")
+	}
+
+	selection := prng.Intn(len(candidates))
+	transferURL := t[candidates[selection]]
+
+	return transferURL, nil
+}

psiphon/common/parameters/transferURLs_test.go

@@ -164,7 +164,7 @@ func TestTransferURLs(t *testing.T) {
 					t.Fatalf("unexpected canonical URL: %s", canonicalURL)
 				}
 				if skipVerify {
-					t.Fatalf("expected skipVerify")
+					t.Fatalf("unexpected skipVerify")
 				}
 				attemptDistinctSelections[attempt][url] += 1
 				attempt = (attempt + 1) % testCase.attempts
@@ -186,3 +186,281 @@ func TestTransferURLs(t *testing.T) {
 	}
 
 }
+
+func TestSecureTransferURLs(t *testing.T) {
+
+	decodedA := "a.example.com"
+	encodedA := base64.StdEncoding.EncodeToString([]byte(decodedA))
+	decodedB := "b.example.com"
+	encodedB := base64.StdEncoding.EncodeToString([]byte(decodedB))
+	decodedC := "c.example.com"
+	encodedC := base64.StdEncoding.EncodeToString([]byte(decodedC))
+
+	type secureTransferURLSubTest struct {
+		secureTransferURL  *SecureTransferURL
+		expectedDecodedURL string
+	}
+
+	testCases := []struct {
+		description                string
+		secureTransferURLs         []secureTransferURLSubTest
+		attempts                   int
+		expectedValid              bool
+		expectedDistinctSelections int
+	}{
+		{
+			"missing OnlyAfterAttempts = 0",
+			[]secureTransferURLSubTest{
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedA,
+							OnlyAfterAttempts: 1,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedA,
+				},
+			},
+			1,
+			false,
+			0,
+		},
+		{
+			"contains nil SecureTransferURL",
+			[]secureTransferURLSubTest{
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedA,
+							OnlyAfterAttempts: 0,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedA,
+				},
+				{
+					nil,
+					decodedA,
+				},
+			},
+			1,
+			false,
+			0,
+		},
+		{
+			"single URL, multiple attempts",
+			[]secureTransferURLSubTest{
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedA,
+							OnlyAfterAttempts: 0,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedA,
+				},
+			},
+			2,
+			true,
+			1,
+		},
+		{
+			"multiple URLs, single attempt",
+			[]secureTransferURLSubTest{
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedA,
+							OnlyAfterAttempts: 0,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedA,
+				},
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedB,
+							OnlyAfterAttempts: 1,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedB,
+				},
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedC,
+							OnlyAfterAttempts: 1,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedC,
+				},
+			},
+			1,
+			true,
+			1,
+		},
+		{
+			"multiple URLs, multiple attempts",
+			[]secureTransferURLSubTest{
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedA,
+							OnlyAfterAttempts: 0,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedA,
+				},
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedB,
+							OnlyAfterAttempts: 1,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedB,
+				},
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedC,
+							OnlyAfterAttempts: 1,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedC,
+				},
+			},
+			2,
+			true,
+			3,
+		},
+		{
+			"multiple URLs, multiple attempts",
+			[]secureTransferURLSubTest{
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedA,
+							OnlyAfterAttempts: 0,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedA,
+				},
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedB,
+							OnlyAfterAttempts: 1,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedB,
+				},
+				{
+					&SecureTransferURL{
+						TransferURL: TransferURL{
+							URL:               encodedC,
+							OnlyAfterAttempts: 3,
+						},
+						RequestHeaders:      nil,
+						B64EncodedPublicKey: "",
+					},
+					decodedC,
+				},
+			},
+			4,
+			true,
+			3,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+
+			// Construct list of SecureTransferURLs.
+			secureTransferURLs := make(SecureTransferURLs, len(testCase.secureTransferURLs))
+			for i, subTest := range testCase.secureTransferURLs {
+				secureTransferURLs[i] = subTest.secureTransferURL
+			}
+
+			err := secureTransferURLs.DecodeAndValidate()
+
+			if testCase.expectedValid {
+				if err != nil {
+					t.Fatalf("unexpected validation error: %s", err)
+				}
+			} else {
+				if err == nil {
+					t.Fatalf("expected validation error")
+				}
+				return
+			}
+
+			// Check URLs are decoded as expected.
+			for _, subTest := range testCase.secureTransferURLs {
+				if subTest.expectedDecodedURL != subTest.secureTransferURL.TransferURL.URL {
+					t.Fatalf("unexpected URL: %s", subTest.secureTransferURL.TransferURL.URL)
+				}
+			}
+
+			// Track distinct selections for each attempt; the
+			// expected number of distinct should be for at least
+			// one particular attempt.
+			attemptDistinctSelections := make(map[int]map[string]int)
+			for i := 0; i < testCase.attempts; i++ {
+				attemptDistinctSelections[i] = make(map[string]int)
+			}
+
+			// Perform enough runs to account for random selection.
+			runs := 1000
+
+			attempt := 0
+			for i := 0; i < runs; i++ {
+				secureTransferURL, err := secureTransferURLs.Select(attempt)
+				if err != nil {
+					// Error should have been caught by DecodeAndVerify.
+					t.Fatalf("unexpected Select error")
+				}
+				if secureTransferURL.TransferURL.SkipVerify {
+					t.Fatalf("unexpected skipVerify")
+				}
+				attemptDistinctSelections[attempt][secureTransferURL.TransferURL.URL] += 1
+				attempt = (attempt + 1) % testCase.attempts
+			}
+
+			maxDistinctSelections := 0
+			for _, m := range attemptDistinctSelections {
+				if len(m) > maxDistinctSelections {
+					maxDistinctSelections = len(m)
+				}
+			}
+
+			if maxDistinctSelections != testCase.expectedDistinctSelections {
+				t.Fatalf("got %d distinct selections, expected %d",
+					maxDistinctSelections,
+					testCase.expectedDistinctSelections)
+			}
+		})
+	}
+
+}

psiphon/config.go

@@ -320,7 +320,7 @@ type Config struct {
 	// be established to known servers. This value is supplied by and depends
 	// on the Psiphon Network, and is typically embedded in the client binary.
 	// All URLs must point to the same entity with the same ETag. At least one
-	// DownloadURL must have OnlyAfterAttempts = 0.
+	// TransferURL must have OnlyAfterAttempts = 0.
 	RemoteServerListURLs parameters.TransferURLs
 
 	// RemoteServerListSignaturePublicKey specifies a public key that's used
@@ -703,6 +703,14 @@ type Config struct {
 	// nil, this parameter is ignored.
 	UpgradeDownloadUrl string
 
+	// FeedbackUploadURLs is a list of SecureTransferURLs which specify
+	// locations where feedback data can be uploaded, pairing with each
+	// location a public key with which to encrypt the feedback data. This
+	// value is supplied by and depends on the Psiphon Network, and is
+	// typically embedded in the client binary. At least one TransferURL must
+	// have OnlyAfterAttempts = 0.
+	FeedbackUploadURLs parameters.SecureTransferURLs
+
 	// clientParameters is the active ClientParameters with defaults, config
 	// values, and, optionally, tactics applied.
 	//
@@ -1160,7 +1168,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 	return nil
 }
 
-// GetClientParameters returns a the current client parameters.
+// GetClientParameters returns the current client parameters.
 func (config *Config) GetClientParameters() *parameters.ClientParameters {
 	return config.clientParameters
 }
@@ -1564,6 +1572,10 @@ func (config *Config) makeConfigParameters() map[string]interface{} {
 		applyParameters[parameters.ApplicationParameters] = config.ApplicationParameters
 	}
 
+	if len(config.FeedbackUploadURLs) > 0 {
+		applyParameters[parameters.FeedbackUploadURLs] = config.FeedbackUploadURLs
+	}
+
 	return applyParameters
 }
 

psiphon/feedback.go

@@ -33,20 +33,14 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"net/http"
-	"strings"
 	"time"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/errors"
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/parameters"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/prng"
 )
 
-const (
-	FEEDBACK_UPLOAD_MAX_RETRIES         = 5
-	FEEDBACK_UPLOAD_RETRY_DELAY_SECONDS = 300
-	FEEDBACK_UPLOAD_TIMEOUT_SECONDS     = 30
-)
-
 // Conforms to the format expected by the feedback decryptor.
 // https://bitbucket.org/psiphon/psiphon-circumvention-system/src/default/EmailResponder/FeedbackDecryptor/decryptor.py
 type secureFeedback struct {
@@ -100,8 +94,8 @@ func encryptFeedback(diagnosticsJson, b64EncodedPublicKey string) ([]byte, error
 }
 
 // Encrypt feedback and upload to server. If upload fails
-// the feedback thread will sleep and retry multiple times.
-func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer, uploadPath, uploadServerHeaders string) error {
+// the routine will sleep and retry multiple times.
+func SendFeedback(configJson, diagnosticsJson, uploadPath string) error {
 
 	config, err := LoadConfig([]byte(configJson))
 	if err != nil {
@@ -112,6 +106,24 @@ func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer
 		return errors.Trace(err)
 	}
 
+	// Get tactics, may update client parameters
+	p := config.GetClientParameters().Get()
+	timeout := p.Duration(parameters.FeedbackTacticsWaitPeriod)
+	p.Close()
+	ctx, cancelFunc := context.WithTimeout(context.Background(), timeout)
+	defer cancelFunc()
+	// Note: GetTactics will fail silently if the datastore used for retrieving
+	// and storing tactics is opened by another process.
+	GetTactics(ctx, config)
+
+	// Get the latest client parameters
+	p = config.GetClientParameters().Get()
+	feedbackUploadRetryDelay := p.Duration(parameters.FeedbackUploadRetryDelaySeconds)
+	feedbackUploadTimeout := p.Duration(parameters.FeedbackUploadTimeoutSeconds)
+	feedbackUploadMaxRetries := p.Int(parameters.FeedbackUploadMaxRetries)
+	secureTransferURLs := p.SecureTransferURLs(parameters.FeedbackUploadURLs)
+	p.Close()
+
 	untunneledDialConfig := &DialConfig{
 		UpstreamProxyURL:              config.UpstreamProxyURL,
 		CustomHeaders:                 config.CustomHeaders,
@@ -121,30 +133,53 @@ func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer
 		TrustedCACertificatesFilename: config.TrustedCACertificatesFilename,
 	}
 
-	secureFeedback, err := encryptFeedback(diagnosticsJson, b64EncodedPublicKey)
-	if err != nil {
-		return err
-	}
-
 	uploadId := prng.HexString(8)
 
-	url := "https://" + uploadServer + uploadPath + uploadId
-	headerPieces := strings.Split(uploadServerHeaders, ": ")
-	// Only a single header is expected.
-	if len(headerPieces) != 2 {
-		return errors.Tracef("expected 2 header pieces, got: %d", len(headerPieces))
-	}
+	for i := 0; i < feedbackUploadMaxRetries; i++ {
+
+		secureTransferURL, err := secureTransferURLs.Select(i)
+		if err != nil {
+			return errors.Tracef("Error selecting feedback transfer URL: %s", err)
+		}
+
+		secureFeedback, err := encryptFeedback(diagnosticsJson, secureTransferURL.B64EncodedPublicKey)
+		if err != nil {
+			return errors.Trace(err)
+		}
 
-	for i := 0; i < FEEDBACK_UPLOAD_MAX_RETRIES; i++ {
-		err = uploadFeedback(
+		ctx, cancelFunc := context.WithTimeout(
+			context.Background(),
+			feedbackUploadTimeout)
+		defer cancelFunc()
+
+		client, err := MakeUntunneledHTTPClient(
+			ctx,
 			config,
 			untunneledDialConfig,
-			secureFeedback,
-			url,
-			MakePsiphonUserAgent(config),
-			headerPieces)
+			nil,
+			secureTransferURL.TransferURL.SkipVerify)
 		if err != nil {
-			time.Sleep(FEEDBACK_UPLOAD_RETRY_DELAY_SECONDS * time.Second)
+			return errors.Trace(err)
+		}
+
+		url := secureTransferURL.TransferURL.URL + uploadPath + uploadId
+
+		req, err := http.NewRequest("PUT", url, bytes.NewBuffer(secureFeedback))
+		if err != nil {
+			return errors.Trace(err)
+		}
+
+		for k, v := range secureTransferURL.RequestHeaders {
+			req.Header.Set(k, v)
+		}
+		req.Header.Set("User-Agent", MakePsiphonUserAgent(config))
+
+		err = uploadFeedback(client, req)
+		if err != nil {
+			// Do not sleep after the last attempt
+			if i+1 < feedbackUploadMaxRetries {
+				time.Sleep(feedbackUploadRetryDelay)
+			}
 		} else {
 			break
 		}
@@ -155,31 +190,7 @@ func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer
 
 // Attempt to upload feedback data to server.
 func uploadFeedback(
-	config *Config, dialConfig *DialConfig, feedbackData []byte, url, userAgent string, headerPieces []string) error {
-
-	ctx, cancelFunc := context.WithTimeout(
-		context.Background(),
-		time.Duration(FEEDBACK_UPLOAD_TIMEOUT_SECONDS*time.Second))
-	defer cancelFunc()
-
-	client, err := MakeUntunneledHTTPClient(
-		ctx,
-		config,
-		dialConfig,
-		nil,
-		false)
-	if err != nil {
-		return err
-	}
-
-	req, err := http.NewRequest("PUT", url, bytes.NewBuffer(feedbackData))
-	if err != nil {
-		return errors.Trace(err)
-	}
-
-	req.Header.Set("User-Agent", userAgent)
-
-	req.Header.Set(headerPieces[0], headerPieces[1])
+	client *http.Client, req *http.Request) error {
 
 	resp, err := client.Do(req)
 	if err != nil {
@@ -188,7 +199,7 @@ func uploadFeedback(
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return errors.TraceNew("received HTTP status: " + resp.Status)
+		return errors.TraceNew("Unexpected HTTP status: " + resp.Status)
 	}
 
 	return nil

psiphon/feedback_test.go

@@ -73,7 +73,7 @@ func TestFeedbackUpload(t *testing.T) {
 		t.FailNow()
 	}
 
-	err = SendFeedback(string(configFileContents), string(diagnosticData), config["ENCRYPTION_PUBLIC_KEY"].(string), config["UPLOAD_SERVER"].(string), config["UPLOAD_PATH"].(string), config["UPLOAD_SERVER_HEADERS"].(string))
+	err = SendFeedback(string(configFileContents), string(diagnosticData), "")
 	if err != nil {
 		t.Error(err.Error())
 		t.FailNow()