Jelajahi Sumber

Merge pull request #562 from mirokuratczyk/feedback-upload-refactor

Refactor feedback upload mechanism
Miro 5 tahun lalu
induk
melakukan
814f04fbae

+ 2 - 2
.travis.yml

@@ -69,8 +69,8 @@ before_install:
 - git rev-parse --short HEAD > psiphon/git_rev
 - openssl aes-256-cbc -K $encrypted_bf83b4ab4874_key -iv $encrypted_bf83b4ab4874_iv
   -in psiphon/controller_test.config.enc -out psiphon/controller_test.config -d
-- openssl aes-256-cbc -K $encrypted_sq6sgjwvsppj_key -iv $encrypted_sq6sgjwvsppj_iv
-  -in psiphon/feedback_test.config.enc -out psiphon/feedback_test.config -d
+- openssl aes-256-cbc -K $encrypted_560fd0d04977_key -iv $encrypted_560fd0d04977_iv 
+  -in feedback_test.config.enc -out feedback_test.config -d
 notifications:
   slack:
     rooms:

+ 188 - 18
MobileLibrary/Android/PsiphonTunnel/PsiphonTunnel.java

@@ -63,13 +63,31 @@ import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
 import psi.Psi;
 import psi.PsiphonProvider;
+import psi.PsiphonProviderNoticeHandler;
+import psi.PsiphonProviderFeedbackHandler;
 
 public class PsiphonTunnel {
 
-    public interface HostService {
+    public interface HostLogger {
+        default public void onDiagnosticMessage(String message) {}
+    }
+
+    // Protocol used to communicate the outcome of feedback upload operations to the application
+    // using PsiphonTunnelFeedback.
+    public interface HostFeedbackHandler {
+        // Callback which is invoked once the feedback upload has completed.
+        // If the exception is non-null, then the upload failed.
+        default public void sendFeedbackCompleted(java.lang.Exception e) {}
+    }
+
+    public interface HostService extends HostLogger {
 
         public String getAppName();
         public Context getContext();
@@ -77,7 +95,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) {}
@@ -299,6 +316,151 @@ public class PsiphonTunnel {
         Psi.writeRuntimeProfiles(outputDirectory, cpuSampleDurationSeconnds, blockSampleDurationSeconds);
     }
 
+    // The interface for managing the Psiphon feedback upload operations.
+    // Warnings:
+    // - Should not be used in the same process as PsiphonTunnel.
+    // - Only a single instance of PsiphonTunnelFeedback should be used at a time. Using multiple
+    // instances in parallel, or concurrently, will result in undefined behavior.
+    public static class PsiphonTunnelFeedback {
+
+        final private ExecutorService workQueue;
+        final private ExecutorService callbackQueue;
+
+        public PsiphonTunnelFeedback() {
+            workQueue = Executors.newSingleThreadExecutor();
+            callbackQueue = Executors.newSingleThreadExecutor();
+        }
+
+        @Override
+        protected void finalize() throws Throwable {
+            // Ensure the queues are cleaned up.
+            shutdownAndAwaitTermination(callbackQueue);
+            shutdownAndAwaitTermination(workQueue);
+            super.finalize();
+        }
+
+        void shutdownAndAwaitTermination(ExecutorService pool) {
+            try {
+                // Wait a while for existing tasks to terminate
+                if (!pool.awaitTermination(5, TimeUnit.SECONDS)) {
+                    pool.shutdownNow(); // Cancel currently executing tasks
+                    // Wait a while for tasks to respond to being cancelled
+                    if (!pool.awaitTermination(5, TimeUnit.SECONDS)) {
+                        System.err.println("PsiphonTunnelFeedback: pool did not terminate");
+                        return;
+                    }
+                }
+            } catch (InterruptedException ie) {
+                // (Re-)Cancel if current thread also interrupted
+                pool.shutdownNow();
+                // Preserve interrupt status
+                Thread.currentThread().interrupt();
+            }
+        }
+
+        // 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. This call is asynchronous and returns before the upload completes. The
+        // operation has completed when sendFeedbackCompleted() is called on the provided
+        // HostFeedbackHandler. The provided HostLogger will be called to log informational notices,
+        // including warnings.
+        //
+        // Warnings:
+        // - Only one active upload is supported at a time. An ongoing upload will be cancelled if
+        // this function is called again before it completes.
+        // - An ongoing feedback upload started with startSendFeedback() should be stopped with
+        // stopSendFeedback() before the process exits. This ensures that any underlying resources
+        // are cleaned up; failing to do so may result in data store corruption or other undefined
+        // behavior.
+        // - PsiphonTunnel.startTunneling and startSendFeedback both make an attempt to migrate
+        // persistent files from legacy locations in a one-time operation. If these functions are
+        // called in parallel, then there is a chance that the migration attempts could execute at
+        // the same time and result in non-fatal errors in one, or both, of the migration
+        // operations.
+        public void startSendFeedback(Context context, HostFeedbackHandler feedbackHandler, HostLogger logger,
+                                      String feedbackConfigJson, String diagnosticsJson, String uploadPath,
+                                      String clientPlatformPrefix, String clientPlatformSuffix) {
+
+            workQueue.submit(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        // Adds fields used in feedback upload, e.g. client platform.
+                        String psiphonConfig = buildPsiphonConfig(context, logger, feedbackConfigJson,
+                                clientPlatformPrefix, clientPlatformSuffix, false, 0);
+
+                        Psi.startSendFeedback(psiphonConfig, diagnosticsJson, uploadPath,
+                                new PsiphonProviderFeedbackHandler() {
+                                    @Override
+                                    public void sendFeedbackCompleted(java.lang.Exception e) {
+                                        callbackQueue.submit(new Runnable() {
+                                            @Override
+                                            public void run() {
+                                                feedbackHandler.sendFeedbackCompleted(e);
+                                            }
+                                        });
+                                    }
+                                },
+                                new PsiphonProviderNoticeHandler() {
+                                    @Override
+                                    public void notice(String noticeJSON) {
+
+                                        try {
+                                            JSONObject notice = new JSONObject(noticeJSON);
+
+                                            String noticeType = notice.getString("noticeType");
+                                            if (noticeType == null) {
+                                                return;
+                                            }
+
+                                            JSONObject data = notice.getJSONObject("data");
+                                            if (data == null) {
+                                                return;
+                                            }
+
+                                            String diagnosticMessage = noticeType + ": " + data.toString();
+                                            callbackQueue.submit(new Runnable() {
+                                                @Override
+                                                public void run() {
+                                                    logger.onDiagnosticMessage(diagnosticMessage);
+                                                }
+                                            });
+                                        } catch (java.lang.Exception e) {
+                                            callbackQueue.submit(new Runnable() {
+                                                @Override
+                                                public void run() {
+                                                    logger.onDiagnosticMessage("Error handling notice " + e.toString());
+                                                }
+                                            });
+                                        }
+                                    }
+                                });
+                    } catch (java.lang.Exception e) {
+                        callbackQueue.submit(new Runnable() {
+                            @Override
+                            public void run() {
+                                feedbackHandler.sendFeedbackCompleted(new Exception("Error sending feedback", e));
+                            }
+                        });
+                    }
+                }
+            });
+        }
+
+        // Interrupt an in-progress feedback upload operation started with startSendFeedback(). This
+        // call is asynchronous and returns a future which is fulfilled when the underlying stop
+        // operation completes.
+        public Future<Void> stopSendFeedback() {
+            return workQueue.submit(new Runnable() {
+                @Override
+                public void run() {
+                    Psi.stopSendFeedback();
+                }
+            }, null);
+        }
+    }
+
     //----------------------------------------------------------------------------------------------
     // VPN Routing
     //----------------------------------------------------------------------------------------------
@@ -615,9 +777,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, HostLogger 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
@@ -658,46 +830,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\\-\\.]", "_"));
@@ -802,7 +972,7 @@ public class PsiphonTunnel {
         }
     }
 
-    private String setupTrustedCertificates(Context context) throws Exception {
+    private static String setupTrustedCertificates(Context context, HostLogger logger) throws Exception {
 
         // Copy the Android system CA store to a local, private cert bundle file.
         //
@@ -863,7 +1033,7 @@ public class PsiphonTunnel {
                     output.println("-----END CERTIFICATE-----");
                 }
 
-                mHostService.onDiagnosticMessage("prepared PsiphonCAStore");
+                logger.onDiagnosticMessage("prepared PsiphonCAStore");
 
                 return file.getAbsolutePath();
 

+ 27 - 3
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel.xcodeproj/project.pbxproj

@@ -42,6 +42,10 @@
 		66BDB0681DC26CCC0079384C /* SBJson4Writer.m in Sources */ = {isa = PBXBuildFile; fileRef = 66BDB0591DC26CCC0079384C /* SBJson4Writer.m */; };
 		CE3D1DA523906003009A4AF6 /* Backups.h in Headers */ = {isa = PBXBuildFile; fileRef = CE3D1DA323906003009A4AF6 /* Backups.h */; };
 		CE3D1DA623906003009A4AF6 /* Backups.m in Sources */ = {isa = PBXBuildFile; fileRef = CE3D1DA423906003009A4AF6 /* Backups.m */; };
+		CEC229FC24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h in Headers */ = {isa = PBXBuildFile; fileRef = CEC229FA24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h */; };
+		CEC229FD24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m in Sources */ = {isa = PBXBuildFile; fileRef = CEC229FB24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m */; };
+		CEDE547924EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.h in Headers */ = {isa = PBXBuildFile; fileRef = CEDE547724EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.h */; };
+		CEDE547A24EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.m in Sources */ = {isa = PBXBuildFile; fileRef = CEDE547824EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.m */; };
 		EFED7EBF1F587F6E0078980F /* libresolv.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = EFED7EBE1F587F6E0078980F /* libresolv.tbd */; };
 /* End PBXBuildFile section */
 
@@ -106,6 +110,10 @@
 		66BDB0591DC26CCC0079384C /* SBJson4Writer.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SBJson4Writer.m; sourceTree = "<group>"; };
 		CE3D1DA323906003009A4AF6 /* Backups.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Backups.h; sourceTree = "<group>"; };
 		CE3D1DA423906003009A4AF6 /* Backups.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = Backups.m; sourceTree = "<group>"; };
+		CEC229FA24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = PsiphonProviderNoticeHandlerShim.h; sourceTree = "<group>"; };
+		CEC229FB24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = PsiphonProviderNoticeHandlerShim.m; sourceTree = "<group>"; };
+		CEDE547724EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = PsiphonProviderFeedbackHandlerShim.h; path = ../PsiphonProviderFeedbackHandlerShim.h; sourceTree = "<group>"; };
+		CEDE547824EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = PsiphonProviderFeedbackHandlerShim.m; path = ../PsiphonProviderFeedbackHandlerShim.m; sourceTree = "<group>"; };
 		EFED7EBE1F587F6E0078980F /* libresolv.tbd */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.text-based-dylib-definition"; name = libresolv.tbd; path = usr/lib/libresolv.tbd; sourceTree = SDKROOT; };
 /* End PBXFileReference section */
 
@@ -187,10 +195,11 @@
 		66BDB0221DA6BFCC0079384C /* PsiphonTunnel */ = {
 			isa = PBXGroup;
 			children = (
-				CE3D1D9E239056E4009A4AF6 /* Files */,
 				66BAD3321E525FBC00CD06DE /* JailbreakCheck */,
 				66BDB04A1DC26CCC0079384C /* json-framework */,
+				CEDE547B24EBF5A00053566E /* Psiphon */,
 				662659241DD270E900872F6C /* Reachability */,
+				CEC22A0624F0689000534D04 /* Utils */,
 				66BDB0231DA6BFCC0079384C /* PsiphonTunnel.h */,
 				66BDB0431DA6C7DD0079384C /* PsiphonTunnel.m */,
 				66BDB0241DA6BFCC0079384C /* Info.plist */,
@@ -238,13 +247,24 @@
 			path = "json-framework";
 			sourceTree = "<group>";
 		};
-		CE3D1D9E239056E4009A4AF6 /* Files */ = {
+		CEC22A0624F0689000534D04 /* Utils */ = {
 			isa = PBXGroup;
 			children = (
 				CE3D1DA323906003009A4AF6 /* Backups.h */,
 				CE3D1DA423906003009A4AF6 /* Backups.m */,
 			);
-			path = Files;
+			path = Utils;
+			sourceTree = "<group>";
+		};
+		CEDE547B24EBF5A00053566E /* Psiphon */ = {
+			isa = PBXGroup;
+			children = (
+				CEDE547724EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.h */,
+				CEDE547824EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.m */,
+				CEC229FA24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h */,
+				CEC229FB24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m */,
+			);
+			path = Psiphon;
 			sourceTree = "<group>";
 		};
 		EFED7EBD1F587F6E0078980F /* Frameworks */ = {
@@ -275,9 +295,11 @@
 				6685BDCA1E2E882800F0E414 /* Psi.h in Headers */,
 				66BDB0651DC26CCC0079384C /* SBJson4StreamWriterState.h in Headers */,
 				66BDB05B1DC26CCC0079384C /* SBJson4Parser.h in Headers */,
+				CEDE547924EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.h in Headers */,
 				6685BDCD1E2E88A200F0E414 /* Psi-meta.h in Headers */,
 				66BDB05A1DC26CCC0079384C /* SBJson4.h in Headers */,
 				66BDB0611DC26CCC0079384C /* SBJson4StreamTokeniser.h in Headers */,
+				CEC229FC24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h in Headers */,
 				66BDB0631DC26CCC0079384C /* SBJson4StreamWriter.h in Headers */,
 				66BDB0671DC26CCC0079384C /* SBJson4Writer.h in Headers */,
 			);
@@ -406,6 +428,7 @@
 			files = (
 				66BDB05E1DC26CCC0079384C /* SBJson4StreamParser.m in Sources */,
 				66BDB0641DC26CCC0079384C /* SBJson4StreamWriter.m in Sources */,
+				CEDE547A24EBF5980053566E /* PsiphonProviderFeedbackHandlerShim.m in Sources */,
 				66BDB0661DC26CCC0079384C /* SBJson4StreamWriterState.m in Sources */,
 				66BDB05C1DC26CCC0079384C /* SBJson4Parser.m in Sources */,
 				4E89F7FE1E2ED3CE00005F4C /* LookupIPv6.c in Sources */,
@@ -413,6 +436,7 @@
 				66BDB0681DC26CCC0079384C /* SBJson4Writer.m in Sources */,
 				66BDB0621DC26CCC0079384C /* SBJson4StreamTokeniser.m in Sources */,
 				66BDB0441DA6C7DD0079384C /* PsiphonTunnel.m in Sources */,
+				CEC229FD24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m in Sources */,
 				662659281DD270E900872F6C /* Reachability.m in Sources */,
 				66BDB0601DC26CCC0079384C /* SBJson4StreamParserState.m in Sources */,
 				CE3D1DA623906003009A4AF6 /* Backups.m in Sources */,

+ 0 - 34
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Files/Backups.h

@@ -1,34 +0,0 @@
-/*
-* Copyright (c) 2019, Psiphon Inc.
-* All rights reserved.
-*
-* This program is free software: you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation, either version 3 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-* GNU General Public License for more details.
-*
-* You should have received a copy of the GNU General Public License
-* along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*
-*/
-
-#import <Foundation/Foundation.h>
-
-NS_ASSUME_NONNULL_BEGIN
-
-@interface Backups : NSObject
-
-/// Excludes the target file from application backups made by iCloud and iTunes.
-/// If NO is returned, the file was not successfully excluded from backup and the error is populated.
-/// @param filePath Path at which the file exists.
-/// @param err If non-nil, contains the error encountered when attempting to exclude the file from backup.
-+ (BOOL)excludeFileFromBackup:(NSString*)filePath err:(NSError**)err;
-
-@end
-
-NS_ASSUME_NONNULL_END

+ 0 - 38
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Files/Backups.m

@@ -1,38 +0,0 @@
-/*
-* Copyright (c) 2019, Psiphon Inc.
-* All rights reserved.
-*
-* This program is free software: you can redistribute it and/or modify
-* it under the terms of the GNU General Public License as published by
-* the Free Software Foundation, either version 3 of the License, or
-* (at your option) any later version.
-*
-* This program is distributed in the hope that it will be useful,
-* but WITHOUT ANY WARRANTY; without even the implied warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-* GNU General Public License for more details.
-*
-* You should have received a copy of the GNU General Public License
-* along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*
-*/
-
-#import "Backups.h"
-
-@implementation Backups
-
-// See comment in header
-+ (BOOL)excludeFileFromBackup:(NSString*)filePath err:(NSError**)err {
-    *err = nil;
-
-    // The URL must be of the file scheme ("file://"), otherwise the `setResourceValue:forKey:error`
-    // operation will silently fail with: "CFURLCopyResourcePropertyForKey failed because passed URL
-    // no scheme".
-    NSURL *urlWithScheme = [NSURL fileURLWithPath:filePath];
-
-    return [urlWithScheme setResourceValue:[NSNumber numberWithBool:YES]
-                                    forKey:NSURLIsExcludedFromBackupKey
-                                     error:err];
-}
-
-@end

+ 35 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Psiphon/PsiphonProviderNoticeHandlerShim.h

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2020, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#import <Foundation/Foundation.h>
+#import "Psi-meta.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+/// PsiphonProviderNoticeHandlerShim provides a shim between GoPsiPsiphonProviderNoticeHandler and a block which logs notices.
+/// @note This indirection is required because gomobile does not support Objective-C blocks.
+@interface PsiphonProviderNoticeHandlerShim : NSObject <GoPsiPsiphonProviderNoticeHandler>
+
+/// Initialize the notice handler with the given logger.
+/// @param logger Logger which will receive notices.
+- (id)initWithLogger:(void (^__nonnull)(NSString *_Nonnull))logger;
+
+@end
+
+NS_ASSUME_NONNULL_END

+ 42 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Psiphon/PsiphonProviderNoticeHandlerShim.m

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2020, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#import "PsiphonProviderNoticeHandlerShim.h"
+
+@implementation PsiphonProviderNoticeHandlerShim {
+    void (^logger) (NSString *_Nonnull);
+}
+
+- (id)initWithLogger:(void (^__nonnull)(NSString *_Nonnull))logger {
+    self = [super init];
+    if (self != nil) {
+        self->logger = logger;
+    }
+    return self;
+}
+
+#pragma mark - GoPsiPsiphonProviderNoticeHandler implementation
+
+- (void)notice:(NSString *)noticeJSON {
+    if (self->logger != nil) {
+        self->logger(noticeJSON);
+    }
+}
+
+@end

+ 36 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonProviderFeedbackHandlerShim.h

@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2020, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#import <Foundation/Foundation.h>
+#import "PsiphonTunnel.h"
+#import "Psi-meta.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+/// PsiphonProviderFeedbackHandlerShim provides a shim between the internal GoPsiPsiphonProviderFeedbackHandler and exported
+/// PsiphonTunnelFeedbackDelegate interfaces.
+@interface PsiphonProviderFeedbackHandlerShim : NSObject <GoPsiPsiphonProviderFeedbackHandler>
+
+/// Initialize the shim with the given feedback delegate.
+/// @param handler Callback which is invoked when the feedback upload completes.
+- (id)initWithHandler:(void (^__nonnull)(NSError *_Nonnull))handler;
+
+@end
+
+NS_ASSUME_NONNULL_END

+ 42 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonProviderFeedbackHandlerShim.m

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2020, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#import "PsiphonProviderFeedbackHandlerShim.h"
+
+@implementation PsiphonProviderFeedbackHandlerShim {
+    void (^handler) (NSError *_Nonnull);
+}
+
+- (id)initWithHandler:(void (^__nonnull)(NSError *_Nonnull))handler; {
+    self = [super init];
+    if (self != nil) {
+        self->handler = handler;
+    }
+    return self;
+}
+
+#pragma mark - GoPsiPsiphonProviderFeedbackHandler implementation
+
+- (void)sendFeedbackCompleted:(NSError *)err {
+    if (self->handler) {
+        self->handler(err);
+    }
+}
+
+@end

+ 78 - 75
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.h

@@ -36,15 +36,6 @@ FOUNDATION_EXPORT const unsigned char PsiphonTunnelVersionString[];
 
 /*!
  The set of possible connection states the tunnel can be in.
- Swift:
- @code
- public enum PsiphonConnectionState : Int {
-     case disconnected
-     case connecting
-     case connected
-     case waitingForNetwork
- }
- @endcode
  */
 typedef NS_ENUM(NSInteger, PsiphonConnectionState)
 {
@@ -54,6 +45,22 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
     PsiphonConnectionStateWaitingForNetwork
 };
 
+/*!
+ @protocol PsiphonTunnelLoggerDelegate
+ Used to communicate diagnostic logs to the application that is using the PsiphonTunnel framework.
+ */
+@protocol PsiphonTunnelLoggerDelegate <NSObject>
+
+@optional
+
+/*!
+ Gets runtime errors info that may be useful for debugging.
+ @param message  The diagnostic message string.
+ @param timestamp RFC3339 encoded timestamp.
+ */
+- (void)onDiagnosticMessage:(NSString * _Nonnull)message withTimestamp:(NSString * _Nonnull)timestamp;
+
+@end
 
 /*!
  @protocol TunneledAppDelegate
@@ -62,7 +69,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, PsiphonTunnelLoggerDelegate>
 
 //
 // Required delegate methods
@@ -114,11 +121,15 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
  @return Either JSON NSString with config that should be used to run the Psiphon tunnel,
          or return already parsed JSON as NSDictionary,
          or nil on error.
-
- Swift: @code func getPsiphonConfig() -> Any? @endcode
  */
 - (id _Nullable)getPsiphonConfig;
 
+//
+// Optional delegate methods. Note that some of these are probably necessary for
+// for a functioning app to implement, for example `onConnected`.
+//
+@optional
+
 /*!
  Called when the tunnel is starting to get the initial server entries (typically embedded in the app) that will be used to bootstrap the Psiphon tunnel connection. This value is in a particular format and will be supplied by Psiphon Inc.
  If getEmbeddedServerEntriesPath is also implemented, it will take precedence over this method, unless getEmbeddedServerEntriesPath returns NULL or an empty string.
@@ -126,12 +137,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
  */
 - (NSString * _Nullable)getEmbeddedServerEntries;
 
-//
-// Optional delegate methods. Note that some of these are probably necessary for
-// for a functioning app to implement, for example `onConnected`.
-//
-@optional
-
 /*!
   Called when the tunnel is starting to get the initial server entries (typically embedded in the app) that will be used to bootstrap the Psiphon tunnel connection. This value is in a particular format and will be supplied by Psiphon Inc.
   If this method is implemented, it takes precedence over getEmbeddedServerEntries, and getEmbeddedServerEntries will not be called unless this method returns NULL or an empty string.
@@ -139,22 +144,12 @@ 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
  */
 - (void)onConnecting;
 /*!
  Called when the tunnel has successfully connected.
- Swift: @code func onConnected() @endcode
  */
 - (void)onConnected;
 
@@ -162,7 +157,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
  Called when the tunnel notices that the device has no network connectivity and
  begins waiting to regain it. When connecitvity is regained, `onConnecting`
  will be called.
- Swift: @code func onStartedWaitingForNetworkConnectivity @endcode
  */
 - (void)onStartedWaitingForNetworkConnectivity;
 
@@ -173,7 +167,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
  (since it didn't change from anything).
  @param oldState  The previous connection state.
  @param newState  The new connection state.
- Swift: @code func onConnectionStateChanged(from oldState: PsiphonConnectionState, to newState: PsiphonConnectionState) @endcode
  */
 - (void)onConnectionStateChangedFrom:(PsiphonConnectionState)oldState to:(PsiphonConnectionState)newState;
 
@@ -181,7 +174,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
  Called to indicate that tunnel-core is exiting imminently (usually due to
  a `stop()` call, but could be due to an unexpected error).
  onExiting may be called before or after `stop()` returns.
- Swift: @code func onExiting() @endcode
  */
 - (void)onExiting;
 
@@ -189,7 +181,6 @@ typedef NS_ENUM(NSInteger, PsiphonConnectionState)
 Called when the device's Internet connection state has changed.
 This may mean that it had connectivity and now doesn't, or went from Wi-Fi to
 WWAN or vice versa or VPN state changed
-Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachability) @endcode
 */
 - (void)onInternetReachabilityChanged:(Reachability * _Nonnull)currentReachability;
 
@@ -198,7 +189,6 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  for use. This can be used for updating the UI which provides the options to
  the user.
  @param regions  A string array containing the available egress region country codes.
- Swift: @code func onAvailableEgressRegions(_ regions: [Any]) @endcode
  */
 - (void)onAvailableEgressRegions:(NSArray * _Nonnull)regions;
 
@@ -206,48 +196,41 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  If the tunnel is started with a fixed SOCKS proxy port, and that port is
  already in use, this will be called.
  @param port  The port number.
- Swift: @code func onSocksProxyPort(inUse port: Int) @endcode
  */
 - (void)onSocksProxyPortInUse:(NSInteger)port;
 /*!
  If the tunnel is started with a fixed HTTP proxy port, and that port is
  already in use, this will be called.
  @param port  The port number.
- Swift: @code func onHttpProxyPort(inUse port: Int) @endcode
  */
 - (void)onHttpProxyPortInUse:(NSInteger)port;
 
 /*!
  Called when tunnel-core determines what port will be used for the local SOCKS proxy.
  @param port  The port number.
- Swift: @code func onListeningSocksProxyPort(_ port: Int) @endcode
  */
 - (void)onListeningSocksProxyPort:(NSInteger)port;
 /*!
  Called when tunnel-core determines what port will be used for the local HTTP proxy.
  @param port  The port number.
- Swift: @code func onListeningHttpProxyPort(_ port: Int) @endcode
  */
 - (void)onListeningHttpProxyPort:(NSInteger)port;
 
 /*!
  Called when a error occurs when trying to utilize a configured upstream proxy.
  @param message  A message giving additional info about the error.
- Swift: @code func onUpstreamProxyError(_ message: String) @endcode
  */
 - (void)onUpstreamProxyError:(NSString * _Nonnull)message;
 
 /*!
  Called after the handshake with the Psiphon server, with the client region as determined by the server.
  @param region  The country code of the client, as determined by the server.
- Swift: @code func onClientRegion(_ region: String) @endcode
  */
 - (void)onClientRegion:(NSString * _Nonnull)region;
 
 /*!
  Called to report that split tunnel is on for the given region.
  @param region  The region split tunnel is on for.
- Swift: @code func onSplitTunnelRegion(_ region: String) @endcode
  */
 - (void)onSplitTunnelRegion:(NSString * _Nonnull)region;
 
@@ -257,7 +240,6 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  Note: `address` should remain private; this notice should be used for alerting
  users, not for diagnotics logs.
  @param address  The IP or hostname that is not being tunneled.
- Swift: @code func onUntunneledAddress(_ address: String) @endcode
  */
 - (void)onUntunneledAddress:(NSString * _Nonnull)address;
 
@@ -268,7 +250,6 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  EmitBytesTransferred to true in the Psiphon config.
  @param sent  The number of bytes sent.
  @param received  The number of bytes received.
- Swift: @code func onBytesTransferred(_ sent: Int64, _ received: Int64) @endcode
  */
 - (void)onBytesTransferred:(int64_t)sent :(int64_t)received;
 
@@ -278,21 +259,18 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  once, for multiple home pages.
  Note: This is probably only applicable to Psiphon Inc.'s apps.
  @param url  The URL of the home page.
- Swift: @code func onHomepage(_ url: String) @endcode
  */
 - (void)onHomepage:(NSString * _Nonnull)url;
 
 /*!
  Called when tunnel-core receives server timetamp in the handshake
  @param timestamp  The server timestamp in RFC3339 format.
- Swift: @code func onServerTimestamp(_ timestamp: String) @endcode
  */
 - (void)onServerTimestamp:(NSString * _Nonnull)timestamp;
 
 /*!
  Called when tunnel-core receives an array of active authorization IDs in the handshake
  @param authorizations  A string array containing active authorization IDs.
- Swift: @code func onActiveAuthorizationIDs(_ authorizations: [Any]) @endcode
  */
 - (void)onActiveAuthorizationIDs:(NSArray * _Nonnull)authorizations;
 
@@ -300,7 +278,6 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  Called when tunnel-core receives traffic rate limit information in the handshake
  @param upstreamBytesPerSecond  upstream rate limit; 0 for no limit
  @param downstreamBytesPerSecond  downstream rate limit; 0 for no limit
- Swift: @code func onTrafficRateLimits(_ upstreamBytesPerSecond: Int64, _ downstreamBytesPerSecond: Int64) @endcode
  */
 - (void)onTrafficRateLimits:(int64_t)upstreamBytesPerSecond :(int64_t)downstreamBytesPerSecond;
 
@@ -308,7 +285,6 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  Called when tunnel-core receives an alert from the server.
  @param reason The reason for the alert.
  @param subject Additional context or classification of the reason; blank for none.
- Swift: @code func onServerAlert(_ reason: String, _ subject: String) @endcode
  */
 - (void)onServerAlert:(NSString * _Nonnull)reason :(NSString * _Nonnull)subject;
 
@@ -323,7 +299,6 @@ Swift: @code func onInternetReachabilityChanged(_ currentReachability: Reachabil
  Returns an instance of PsiphonTunnel. This is either a new instance or the pre-existing singleton. If an instance already exists, it will be stopped when this function is called.
  @param tunneledAppDelegate  The delegate implementation to use for callbacks.
  @return  The PsiphonTunnel instance.
- Swift: @code class func newPsiphonTunnel(_ tunneledAppDelegate: TunneledAppDelegate) -> Self @endcode
  */
 + (PsiphonTunnel * _Nonnull)newPsiphonTunnel:(id<TunneledAppDelegate> _Nonnull)tunneledAppDelegate;
 
@@ -366,7 +341,6 @@ Returns the path where the rotated notices file will be created.
  Start connecting the PsiphonTunnel. Returns before connection is complete -- delegate callbacks (such as `onConnected` and `onConnectionStateChanged`) are used to indicate progress and state.
  @param ifNeeded  If TRUE, the tunnel will only be started if it's not already connected and healthy. If FALSE, the tunnel will be forced to stop and reconnect.
  @return TRUE if the connection start was successful, FALSE otherwise.
- Swift: @code func start(_ ifNeeded: Bool) -> Bool @endcode
  */
 - (BOOL)start:(BOOL)ifNeeded;
 
@@ -384,26 +358,22 @@ Returns the path where the rotated notices file will be created.
  session ID.
 
  @return TRUE if the connection start was successful, FALSE otherwise.
- Swift: @code func startWithCurrentSessionID() @endcode
  */
 - (BOOL)stopAndReconnectWithCurrentSessionID;
 
 /*!
  Stop the tunnel (regardless of its current connection state).
- Swift: @code func stop() @endcode
  */
 - (void)stop;
 
 /*!
  Indicate if the device is sleeping. This logs a diagnostic message and forces hasNetworkConnectivity to false when sleeping.
- Swift: @code func setSleeping(_ isSleeping: Bool) @endcode
  */
 - (void)setSleeping:(BOOL)isSleeping;
 
 /*!
  Returns the current tunnel connection state.
  @return  The current connection state.
- Swift: @code func getConnectionState() -> PsiphonConnectionState @endcode
  */
 - (PsiphonConnectionState)getConnectionState;
 
@@ -411,63 +381,42 @@ Returns the path where the rotated notices file will be created.
  Returns the current network reachability status, if Psiphon tunnel is not in a
  disconnected state.
  @return The current reachability status.
- Swift: @code func getNetworkReachabilityStatus(_ status: UnsafeMutablePointer<NetworkStatus>!) -> Bool  @endcode
  */
 - (BOOL)getNetworkReachabilityStatus:(NetworkStatus * _Nonnull)status;
 
 /*!
  Provides the port number of the local SOCKS proxy. Only valid when currently connected (will return 0 otherwise).
  @return  The current local SOCKS proxy port number.
- Swift: @code func getLocalSocksProxyPort() -> Int @endcode
  */
 - (NSInteger)getLocalSocksProxyPort;
 
 /*!
  Provides the port number of the local HTTP proxy. Only valid when currently connected (will return 0 otherwise).
  @return  The current local HTTP proxy port number.
- Swift: @code func getLocalHttpProxyPort() -> Int @endcode
  */
 - (NSInteger)getLocalHttpProxyPort;
 
 /*!
  Only valid in whole device mode. Provides the MTU the packet tunnel requires the device to use.
  @return  The MTU size.
- Swift: @code func getPacketTunnelMTU() -> Int @endcode
  */
 - (long)getPacketTunnelMTU;
 
 /*!
  Only valid in whole device mode. Provides the DNS resolver IP address that is provided by the packet tunnel to the device.
   @return  The IP address of the DNS resolver as a string.
-  Swift: @code func getPacketTunnelDNSResolverIPv4Address() -> String @endcode
  */
 - (NSString * _Nonnull)getPacketTunnelDNSResolverIPv4Address;
 
 /*!
  Only valid in whole device mode. Provides the DNS resolver IP address that is provided by the packet tunnel to the device.
  @return  The IP address of the DNS resolver as a string.
-  Swift: @code func getPacketTunnelDNSResolverIPv6Address() -> String @endcode
  */
 - (NSString * _Nonnull)getPacketTunnelDNSResolverIPv6Address;
 
-/*!
- 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;
-
 /*!
  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.
  @return  The build info json as a string.
- Swift: @code func getBuildInfo() -> String @endcode
  */
 + (NSString * _Nonnull)getBuildInfo;
 
@@ -477,8 +426,62 @@ Returns the path where the rotated notices file will be created.
  Writes Go runtime profile information to a set of files in the specifiec output directory.
  @param cpuSampleDurationSeconds determines how to long to wait and sample profiles that require active sampling. When set to 0, these profiles are skipped.
  @param blockSampleDurationSeconds determines how to long to wait and sample profiles that require active sampling. When set to 0, these profiles are skipped.
- Swift: @code func writeRuntimeProfilesTo(_ outputDirectory: String, withCPUSampleDurationSeconds cpuSampleDurationSecond: Int, withBlockSampleDurationSeconds blockSampleDurationSeconds: Int) @endcode
  */
 - (void)writeRuntimeProfilesTo:(NSString * _Nonnull)outputDirectory withCPUSampleDurationSeconds:(int)cpuSampleDurationSeconds withBlockSampleDurationSeconds:(int)blockSampleDurationSeconds;
 
  @end
+
+/*!
+ @protocol PsiphonTunnelFeedbackDelegate
+ Used to communicate the outcome of feedback upload operations to the application using the PsiphonTunnel framework.
+ */
+@protocol PsiphonTunnelFeedbackDelegate <NSObject>
+
+/// Called once the feedback upload has completed.
+/// @param err If non-nil, then the upload failed.
+- (void)sendFeedbackCompleted:(NSError * _Nullable)err;
+
+@end
+
+/*!
+ The interface for managing the Psiphon tunnel feedback upload operations.
+ @warning Should not be used in the same process as PsiphonTunnel.
+ @warning Only a single instance of PsiphonTunnelFeedback should be used at a time. Using multiple instances in parallel, or
+ concurrently, will result in undefined behavior.
+ */
+@interface PsiphonTunnelFeedback : NSObject
+
+/*!
+ Upload a feedback package to Psiphon Inc. The app collects feedback and diagnostics information in a particular format and then calls this
+ function to upload it for later investigation. This call is asynchronous and returns before the upload completes. The operation has
+ completed when `sendFeedbackCompleted:` is called on the provided `PsiphonTunnelFeedbackDelegate`.
+ @param feedbackJson The feedback data to upload.
+ @param feedbackConfigJson The feedback compatible config. Must be an NSDictionary or NSString. Config must be provided by
+ Psiphon Inc.
+ @param uploadPath The path at which to upload the diagnostic data. Must be provided by Psiphon Inc.
+ @param loggerDelegate Optional delegate which will be called to log informational notices, including warnings. Stored as a weak
+ reference; the caller is responsible for holding a strong reference.
+ @param feedbackDelegate Delegate which `sendFeedbackCompleted(error)` is called on once when the operation completes; if
+ error is non-nil, then the operation failed. Stored as a weak reference; the caller is responsible for holding a strong reference.
+ @warning Only one active upload is supported at a time. An ongoing upload will be cancelled if this function is called again before it
+ completes.
+ @warning An ongoing feedback upload started with `startSendFeedback:` should be stopped with `stopSendFeedback` before the
+ process exits. This ensures that any underlying resources are cleaned up; failing to do so may result in data store corruption or other
+ undefined behavior.
+ @warning `PsiphonTunnel.start:` and `startSendFeedback:`  both make an attempt to migrate persistent files from legacy locations in a
+ one-time operation. If these functions are called in parallel, then there is a chance that the migration attempts could execute at the same
+ time and result in non-fatal errors in one, or both, of the migration operations.
+ */
+- (void)startSendFeedback:(NSString * _Nonnull)feedbackJson
+       feedbackConfigJson:(id _Nonnull)feedbackConfigJson
+               uploadPath:(NSString * _Nonnull)uploadPath
+           loggerDelegate:(id<PsiphonTunnelLoggerDelegate> _Nullable)loggerDelegate
+         feedbackDelegate:(id<PsiphonTunnelFeedbackDelegate> _Nonnull)feedbackDelegate;
+
+/*!
+ Interrupt an in-progress feedback upload operation started with `startSendFeedback:`. This call is synchronous and returns once the
+ upload has been cancelled.
+ */
+- (void)stopSendFeedback;
+
+@end

+ 370 - 83
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -25,6 +25,8 @@
 #import <SystemConfiguration/CaptiveNetwork.h>
 #import "LookupIPv6.h"
 #import "Psi-meta.h"
+#import "PsiphonProviderFeedbackHandlerShim.h"
+#import "PsiphonProviderNoticeHandlerShim.h"
 #import "PsiphonTunnel.h"
 #import "Backups.h"
 #import "json-framework/SBJson4.h"
@@ -47,13 +49,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 +154,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 +211,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 +276,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,18 +301,20 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
                 }
             });
         }
-        
+
         // If getEmbeddedServerEntriesPath returns an empty string,
         // call getEmbeddedServerEntries
         if ([embeddedServerEntriesPath length] == 0) {
-            
-            dispatch_sync(self->callbackQueue, ^{
-                embeddedServerEntries = [self.tunneledAppDelegate getEmbeddedServerEntries];
-            });
-            
-            if (embeddedServerEntries == nil) {
-                [self logMessage:@"Error getting embedded server entries from delegate"];
-                return FALSE;
+            // getEmbeddedServerEntries is optional in the protocol
+            if ([self.tunneledAppDelegate respondsToSelector:@selector(getEmbeddedServerEntries)]) {
+                dispatch_sync(self->callbackQueue, ^{
+                    embeddedServerEntries = [self.tunneledAppDelegate getEmbeddedServerEntries];
+                });
+
+                if (embeddedServerEntries == nil) {
+                    [self logMessage:@"Error getting embedded server entries from delegate"];
+                    return FALSE;
+                }
             }
         }
 
@@ -425,32 +458,6 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     return GoPsiGetPacketTunnelDNSResolverIPv6Address();
 }
 
-// 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"];
-        }
-
-        NSError *e;
-
-        GoPsiSendFeedback(connectionConfigJson, feedbackJson, b64EncodedPublicKey, uploadServer, @"", uploadServerHeaders, &e);
-
-        if (e != nil) {
-            [self logMessage:[NSString stringWithFormat: @"Feedback upload error: %@", e.localizedDescription]];
-        } else {
-            [self logMessage:@"Feedback upload successful"];
-        }
-    });
-}
-
 // See comment in header.
 + (NSString * _Nonnull)getBuildInfo {
     return GoPsiGetBuildInfo();
@@ -490,7 +497,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 +512,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 +565,10 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         
         id eh = ^(NSError *err) {
             initialConfig = nil;
-            [self logMessage:[NSString stringWithFormat: @"Config JSON parse failed: %@", err.description]];
+            NSString *s = [NSString stringWithFormat:@"Config JSON parse failed: %@", err.description];
+            *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                            code:PsiphonTunnelErrorCodeConfigError
+                                        userInfo:@{NSLocalizedDescriptionKey:s}];
         };
         
         id parser = [SBJson4Parser parserWithBlock:block allowMultiRoot:NO unwrapRootArray:NO errorHandler:eh];
@@ -528,16 +576,27 @@ 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"];
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:
+                                                   @"configObject should reference either an "
+                                                    "NSDictionary object or an NSString object"}];
+        return nil;
     }
-    
-    if (initialConfig == nil) {
+
+    if (*outError != nil) {
+        return nil;
+    } else if (initialConfig == nil) {
+        // Should never happen.
+        *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                        code:PsiphonTunnelErrorCodeConfigError
+                                    userInfo:@{NSLocalizedDescriptionKey:@"initialConfig nil"}];
         return nil;
     }
-    
+
     NSMutableDictionary *config = [NSMutableDictionary dictionaryWithDictionary:initialConfig];
     
     //
@@ -545,12 +604,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 +640,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 +657,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 +676,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 +687,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 +706,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 +716,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 +731,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 +741,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 +764,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 +788,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 +848,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 +856,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;
     }
 
@@ -1076,6 +1161,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         
         NSString *dataStr = [[[SBJson4Writer alloc] init] stringWithObject:data];
         NSString *timestampStr = notice[@"timestamp"];
+        if (timestampStr == nil) {
+            return;
+        }
 
         NSString *diagnosticMessage = [NSString stringWithFormat:@"%@: %@", noticeType, dataStr];
         [self postDiagnosticMessage:diagnosticMessage withTimestamp:timestampStr];
@@ -1512,15 +1600,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)];
@@ -1531,3 +1639,182 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 }
 
 @end
+
+// See comment in header.
+@implementation PsiphonTunnelFeedback {
+    dispatch_queue_t workQueue;
+    dispatch_queue_t callbackQueue;
+}
+
+- (id)init {
+    self = [super init];
+    if (self) {
+        self->workQueue = dispatch_queue_create("com.psiphon3.library.feedback.WorkQueue", DISPATCH_QUEUE_SERIAL);
+        self->callbackQueue = dispatch_queue_create("com.psiphon3.library.feedback.CallbackQueue", DISPATCH_QUEUE_SERIAL);
+    }
+    return self;
+}
+
+// See comment in header.
+- (void)startSendFeedback:(NSString * _Nonnull)feedbackJson
+       feedbackConfigJson:(id _Nonnull)feedbackConfigJson
+               uploadPath:(NSString * _Nonnull)uploadPath
+           loggerDelegate:(id<PsiphonTunnelLoggerDelegate> _Nullable)loggerDelegate
+         feedbackDelegate:(id<PsiphonTunnelFeedbackDelegate> _Nonnull)feedbackDelegate {
+
+    dispatch_async(self->workQueue, ^{
+
+        __weak PsiphonTunnelFeedback *weakSelf = self;
+        __weak id<PsiphonTunnelLoggerDelegate> weakLogger = loggerDelegate;
+        __weak id<PsiphonTunnelFeedbackDelegate> weakFeedbackDelegate = feedbackDelegate;
+
+        void (^logMessage)(NSString * _Nonnull) = ^void(NSString * _Nonnull message) {
+            __strong PsiphonTunnelFeedback *strongSelf = weakSelf;
+            if (strongSelf == nil) {
+                return;
+            }
+            __strong id<PsiphonTunnelLoggerDelegate> strongLogger = weakLogger;
+            if (strongLogger == nil) {
+                return;
+            }
+            if ([strongLogger respondsToSelector:@selector(onDiagnosticMessage:withTimestamp:)]) {
+                NSString *timestamp = [[PsiphonTunnel rfc3339Formatter] stringFromDate:[NSDate date]];
+                dispatch_sync(strongSelf->callbackQueue, ^{
+                    [strongLogger onDiagnosticMessage:message withTimestamp:timestamp];
+                });
+            }
+        };
+
+        NSError *err;
+        NSString *sessionID = [PsiphonTunnel generateSessionID:&err];
+        if (err != nil) {
+            [feedbackDelegate sendFeedbackCompleted:err];
+            return;
+        }
+
+        BOOL usingNoticeFiles = FALSE;
+        BOOL tunnelWholeDevice = FALSE;
+
+        NSString *psiphonConfig = [PsiphonTunnel buildPsiphonConfig:feedbackConfigJson
+                                                   usingNoticeFiles:&usingNoticeFiles
+                                                  tunnelWholeDevice:&tunnelWholeDevice
+                                                          sessionID:sessionID
+                                                         logMessage:logMessage
+                                                              error:&err];
+        if (err != nil) {
+            NSError *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                                    code:PsiphonTunnelErrorCodeConfigError
+                                                userInfo:@{NSLocalizedDescriptionKey:@"Error building config",
+                                                           NSUnderlyingErrorKey:err}];
+            dispatch_sync(self->callbackQueue, ^{
+                [feedbackDelegate sendFeedbackCompleted:outError];
+            });
+            return;
+        } else if (psiphonConfig == nil) {
+            // Should never happen.
+            NSError *err = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                               code:PsiphonTunnelErrorCodeConfigError
+                                           userInfo:@{NSLocalizedDescriptionKey:@"Error built config nil"}];
+            dispatch_sync(self->callbackQueue, ^{
+                [feedbackDelegate sendFeedbackCompleted:err];
+            });
+            return;
+        }
+
+        void (^sendFeedbackCompleted)(NSError * _Nonnull) = ^void(NSError * _Nonnull err) {
+            __strong PsiphonTunnelFeedback *strongSelf = weakSelf;
+            if (strongSelf == nil) {
+                return;
+            }
+            __strong id<PsiphonTunnelFeedbackDelegate> strongFeedbackDelegate = weakFeedbackDelegate;
+            if (strongFeedbackDelegate == nil) {
+                return;
+            }
+
+            NSError *outError = nil;
+
+            if (err != nil) {
+                outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
+                                               code:PsiphonTunnelErrorCodeSendFeedbackError
+                                           userInfo:@{NSLocalizedDescriptionKey:@"Error sending feedback",
+                                                      NSUnderlyingErrorKey:err}];
+            }
+            dispatch_sync(strongSelf->callbackQueue, ^{
+                [strongFeedbackDelegate sendFeedbackCompleted:outError];
+            });
+        };
+
+        PsiphonProviderFeedbackHandlerShim *innerFeedbackHandler =
+            [[PsiphonProviderFeedbackHandlerShim alloc] initWithHandler:sendFeedbackCompleted];
+
+        // Convert notice to a diagnostic message and then log it.
+        void (^logNotice)(NSString * _Nonnull) = ^void(NSString * _Nonnull noticeJSON) {
+            __strong PsiphonTunnelFeedback *strongSelf = weakSelf;
+            if (strongSelf == nil) {
+                return;
+            }
+            __strong id<PsiphonTunnelLoggerDelegate> strongLogger = weakLogger;
+            if (strongLogger == nil) {
+                return;
+            }
+            if ([strongLogger respondsToSelector:@selector(onDiagnosticMessage:withTimestamp:)]) {
+
+                __block NSDictionary *notice = nil;
+                id block = ^(id obj, BOOL *ignored) {
+                    if (ignored == nil || *ignored == YES) {
+                        return;
+                    }
+                    notice = (NSDictionary *)obj;
+                };
+
+                id eh = ^(NSError *err) {
+                    notice = nil;
+                    logMessage([NSString stringWithFormat: @"Notice JSON parse failed: %@", err.description]);
+                };
+
+                id parser = [SBJson4Parser parserWithBlock:block allowMultiRoot:NO unwrapRootArray:NO errorHandler:eh];
+                [parser parse:[noticeJSON dataUsingEncoding:NSUTF8StringEncoding]];
+
+                if (notice == nil) {
+                    return;
+                }
+
+                NSString *noticeType = notice[@"noticeType"];
+                if (noticeType == nil) {
+                    logMessage(@"Notice missing noticeType");
+                    return;
+                }
+
+                NSDictionary *data = notice[@"data"];
+                if (data == nil) {
+                    return;
+                }
+
+                NSString *dataStr = [[[SBJson4Writer alloc] init] stringWithObject:data];
+                NSString *timestampStr = notice[@"timestamp"];
+                if (timestampStr == nil) {
+                    return;
+                }
+
+                NSString *diagnosticMessage = [NSString stringWithFormat:@"%@: %@", noticeType, dataStr];
+                dispatch_sync(strongSelf->callbackQueue, ^{
+                    [strongLogger onDiagnosticMessage:diagnosticMessage withTimestamp:timestampStr];
+                });
+            }
+        };
+
+        PsiphonProviderNoticeHandlerShim *noticeHandler =
+            [[PsiphonProviderNoticeHandlerShim alloc] initWithLogger:logNotice];
+
+        GoPsiStartSendFeedback(psiphonConfig, feedbackJson, uploadPath, innerFeedbackHandler, noticeHandler);
+    });
+}
+
+// See comment in header.
+- (void)stopSendFeedback {
+    dispatch_sync(self->workQueue, ^{
+        GoPsiStopSendFeedback();
+    });
+}
+
+@end

+ 34 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Utils/Backups.h

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2019, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#import <Foundation/Foundation.h>
+
+NS_ASSUME_NONNULL_BEGIN
+
+@interface Backups : NSObject
+
+/// Excludes the target file from application backups made by iCloud and iTunes.
+/// If NO is returned, the file was not successfully excluded from backup and the error is populated.
+/// @param filePath Path at which the file exists.
+/// @param err If non-nil, contains the error encountered when attempting to exclude the file from backup.
++ (BOOL)excludeFileFromBackup:(NSString*)filePath err:(NSError * _Nullable *)err;
+
+@end
+
+NS_ASSUME_NONNULL_END

+ 38 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Utils/Backups.m

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2019, Psiphon Inc.
+ * All rights reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#import "Backups.h"
+
+@implementation Backups
+
+// See comment in header
++ (BOOL)excludeFileFromBackup:(NSString*)filePath err:(NSError**)err {
+    *err = nil;
+
+    // The URL must be of the file scheme ("file://"), otherwise the `setResourceValue:forKey:error`
+    // operation will silently fail with: "CFURLCopyResourcePropertyForKey failed because passed URL
+    // no scheme".
+    NSURL *urlWithScheme = [NSURL fileURLWithPath:filePath];
+
+    return [urlWithScheme setResourceValue:[NSNumber numberWithBool:YES]
+                                    forKey:NSURLIsExcludedFromBackupKey
+                                     error:err];
+}
+
+@end

+ 94 - 4
MobileLibrary/psi/psi.go

@@ -40,8 +40,12 @@ import (
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/tun"
 )
 
-type PsiphonProvider interface {
+type PsiphonProviderNoticeHandler interface {
 	Notice(noticeJSON string)
+}
+
+type PsiphonProvider interface {
+	PsiphonProviderNoticeHandler
 	HasNetworkConnectivity() int
 	BindToDevice(fileDescriptor int) (string, error)
 	IPv6Synthesize(IPv4Addr string) string
@@ -50,6 +54,10 @@ type PsiphonProvider interface {
 	GetNetworkID() string
 }
 
+type PsiphonProviderFeedbackHandler interface {
+	SendFeedbackCompleted(err error)
+}
+
 func NoticeUserLog(message string) {
 	psiphon.NoticeUserLog(message)
 }
@@ -290,9 +298,91 @@ func ImportExchangePayload(payload string) bool {
 	return controller.ImportExchangePayload(payload)
 }
 
-// Encrypt and upload feedback.
-func SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer, uploadPath, uploadServerHeaders string) error {
-	return psiphon.SendFeedback(configJson, diagnosticsJson, b64EncodedPublicKey, uploadServer, uploadPath, uploadServerHeaders)
+var sendFeedbackMutex sync.Mutex
+var sendFeedbackCtx context.Context
+var stopSendFeedback context.CancelFunc
+var sendFeedbackWaitGroup *sync.WaitGroup
+
+// StartSendFeedback encrypts the provided diagnostics and then attempts to
+// upload the encrypted diagnostics to one of the feedback upload locations
+// supplied by the provided config or tactics.
+//
+// Returns immediately after starting the operation in a goroutine. The
+// operation has completed when SendFeedbackCompleted(error) is called on the
+// provided PsiphonProviderFeedbackHandler; if error is non-nil, then the
+// operation failed.
+//
+// Only one active upload is supported at a time. An ongoing upload will be
+// cancelled if this function is called again before it completes.
+//
+// Warnings:
+// - Should not be used with Start concurrently in the same process
+// - An ongoing feedback upload started with StartSendFeedback should be
+//   stopped with StopSendFeedback before the process exists. This ensures that
+//   any underlying resources are cleaned up; failing to do so may result in
+//   data store corruption or other undefined behavior.
+// - Start and StartSendFeedback both make an attempt to migrate persistent
+//   files from legacy locations in a one-time operation. If these functions
+//   are called in parallel, then there is a chance that the migration attempts
+//   could execute at the same time and result in non-fatal errors in one, or
+//   both, of the migration operations.
+// - Calling StartSendFeedback or StopSendFeedback on the same call stack
+//   that the PsiphonProviderFeedbackHandler.SendFeedbackCompleted() callback
+//   is delivered on can cause a deadlock. I.E. the callback code must return
+//   so the wait group can complete and the lock acquired in StopSendFeedback
+//   can be released.
+func StartSendFeedback(
+	configJson,
+	diagnosticsJson,
+	uploadPath string,
+	feedbackHandler PsiphonProviderFeedbackHandler,
+	noticeHandler PsiphonProviderNoticeHandler) {
+
+	// Cancel any ongoing uploads.
+	StopSendFeedback()
+
+	sendFeedbackMutex.Lock()
+	defer sendFeedbackMutex.Unlock()
+
+	sendFeedbackCtx, stopSendFeedback = context.WithCancel(context.Background())
+
+	// Unlike in Start, the provider is not wrapped in a newMutexPsiphonProvider
+	// or equivilent, as SendFeedback is not expected to be used in a memory
+	// constrained environment.
+
+	psiphon.SetNoticeWriter(psiphon.NewNoticeReceiver(
+		func(notice []byte) {
+			noticeHandler.Notice(string(notice))
+		}))
+
+	sendFeedbackWaitGroup = new(sync.WaitGroup)
+	sendFeedbackWaitGroup.Add(1)
+
+	go func() {
+		defer sendFeedbackWaitGroup.Done()
+		err := psiphon.SendFeedback(sendFeedbackCtx, configJson, diagnosticsJson, uploadPath)
+		feedbackHandler.SendFeedbackCompleted(err)
+	}()
+}
+
+// StopSendFeedback interrupts an in-progress feedback upload operation
+// started with `StartSendFeedback`.
+//
+// Warning: should not be used with Start concurrently in the same process.
+func StopSendFeedback() {
+
+	sendFeedbackMutex.Lock()
+	defer sendFeedbackMutex.Unlock()
+
+	if stopSendFeedback != nil {
+		stopSendFeedback()
+		sendFeedbackWaitGroup.Wait()
+		sendFeedbackCtx = nil
+		stopSendFeedback = nil
+		sendFeedbackWaitGroup = nil
+		// Allow the notice receiver to be deallocated.
+		psiphon.SetNoticeWriter(os.Stderr)
+	}
 }
 
 // Get build info from tunnel-core

+ 15 - 0
psiphon/common/parameters/clientParameters.go

@@ -242,6 +242,13 @@ const (
 	ServerPacketManipulationSpecs                    = "ServerPacketManipulationSpecs"
 	ServerProtocolPacketManipulations                = "ServerProtocolPacketManipulations"
 	ServerPacketManipulationProbability              = "ServerPacketManipulationProbability"
+	FeedbackUploadURLs                               = "FeedbackUploadURLs"
+	FeedbackEncryptionPublicKey                      = "FeedbackEncryptionPublicKey"
+	FeedbackTacticsWaitPeriod                        = "FeedbackTacticsWaitPeriod"
+	FeedbackUploadMaxAttempts                        = "FeedbackUploadMaxAttempts"
+	FeedbackUploadRetryMinDelaySeconds               = "FeedbackUploadRetryMinDelaySeconds"
+	FeedbackUploadRetryMaxDelaySeconds               = "FeedbackUploadRetryMaxDelaySeconds"
+	FeedbackUploadTimeoutSeconds                     = "FeedbackUploadTimeoutSeconds"
 )
 
 const (
@@ -502,6 +509,14 @@ var defaultClientParameters = map[string]struct {
 	ServerPacketManipulationSpecs:       {value: PacketManipulationSpecs{}, flags: serverSideOnly},
 	ServerProtocolPacketManipulations:   {value: make(ProtocolPacketManipulations), flags: serverSideOnly},
 	ServerPacketManipulationProbability: {value: 0.5, minimum: 0.0, flags: serverSideOnly},
+
+	FeedbackUploadURLs:                 {value: TransferURLs{}},
+	FeedbackEncryptionPublicKey:        {value: ""},
+	FeedbackTacticsWaitPeriod:          {value: 5 * time.Second, minimum: 0 * time.Second, flags: useNetworkLatencyMultiplier},
+	FeedbackUploadMaxAttempts:          {value: 5, minimum: 0},
+	FeedbackUploadRetryMinDelaySeconds: {value: 1 * time.Minute, minimum: time.Duration(0), flags: useNetworkLatencyMultiplier},
+	FeedbackUploadRetryMaxDelaySeconds: {value: 5 * time.Minute, minimum: 1 * 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

+ 27 - 17
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
@@ -44,12 +44,22 @@ type TransferURL struct {
 	// candidate locations. For a value of N, this URL is only a candidate
 	// after N rounds of attempting the transfer to or from other URLs.
 	OnlyAfterAttempts int
+
+	// B64EncodedPublicKey is a base64-encoded RSA public key to be used for
+	// encrypting the resource, when uploading, or for verifying a signature of
+	// the resource, when downloading. Required by some operations, such as
+	// uploading feedback.
+	B64EncodedPublicKey string `json:",omitempty"`
+
+	// RequestHeaders are optional HTTP headers to set on any requests made to
+	// the destination.
+	RequestHeaders map[string]string `json:",omitempty"`
 }
 
 // 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.
@@ -75,29 +85,29 @@ func (t TransferURLs) DecodeAndValidate() error {
 	return nil
 }
 
-// Select chooses a TransferURL from the list.
-//
-// The first return value is the canonical URL, to be used
-// as a key when storing information related to the TransferURLs,
-// such as an ETag.
-//
-// The second return value is the chosen transfer URL, which is
-// selected based at random from the candidates allowed in the
-// specified attempt.
-func (t TransferURLs) Select(attempt int) (string, string, bool) {
+// CanonicalURL returns the canonical URL, to be used as a key when storing
+// information related to the TransferURLs, such as an ETag.
+func (t TransferURLs) CanonicalURL() string {
 
 	// The first OnlyAfterAttempts = 0 URL is the canonical URL. This
 	// is the value used as the key for SetUrlETag when multiple download
 	// URLs can be used to fetch a single entity.
 
-	canonicalURL := ""
 	for _, transferURL := range t {
 		if transferURL.OnlyAfterAttempts == 0 {
-			canonicalURL = transferURL.URL
-			break
+			return transferURL.URL
 		}
 	}
 
+	return ""
+}
+
+// Select chooses a TransferURL from the list.
+//
+// The TransferURL is selected based at random from the candidates allowed in
+// the specified attempt.
+func (t TransferURLs) Select(attempt int) *TransferURL {
+
 	candidates := make([]int, 0)
 	for index, URL := range t {
 		if attempt >= URL.OnlyAfterAttempts {
@@ -108,11 +118,11 @@ func (t TransferURLs) Select(attempt int) (string, string, bool) {
 	if len(candidates) < 1 {
 		// This case is not expected, as DecodeAndValidate should reject configs
 		// that would have no candidates for 0 attempts.
-		return "", "", true
+		return nil
 	}
 
 	selection := prng.Intn(len(candidates))
 	transferURL := t[candidates[selection]]
 
-	return transferURL.URL, canonicalURL, transferURL.SkipVerify
+	return transferURL
 }

+ 5 - 4
psiphon/common/parameters/transferURLs_test.go

@@ -159,14 +159,15 @@ func TestTransferURLs(t *testing.T) {
 
 			attempt := 0
 			for i := 0; i < runs; i++ {
-				url, canonicalURL, skipVerify := testCase.transferURLs.Select(attempt)
+				canonicalURL := testCase.transferURLs.CanonicalURL()
 				if canonicalURL != testCase.expectedCanonicalURL {
 					t.Fatalf("unexpected canonical URL: %s", canonicalURL)
 				}
-				if skipVerify {
-					t.Fatalf("expected skipVerify")
+				transferUrl := testCase.transferURLs.Select(attempt)
+				if transferUrl.SkipVerify {
+					t.Fatalf("unexpected skipVerify")
 				}
-				attemptDistinctSelections[attempt][url] += 1
+				attemptDistinctSelections[attempt][transferUrl.URL] += 1
 				attempt = (attempt + 1) % testCase.attempts
 			}
 

+ 30 - 2
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,20 @@ 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.TransferURLs
+
+	// FeedbackEncryptionPublicKey is a default base64-encoded, RSA public key
+	// value used to encrypt feedback data. Used when uploading feedback with a
+	// TransferURL which has no public key value configured, i.e.
+	// B64EncodedPublicKey = "".
+	FeedbackEncryptionPublicKey string
+
 	// clientParameters is the active ClientParameters with defaults, config
 	// values, and, optionally, tactics applied.
 	//
@@ -1019,6 +1033,12 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 		}
 	}
 
+	if config.FeedbackUploadURLs != nil {
+		if config.FeedbackEncryptionPublicKey == "" {
+			return errors.TraceNew("missing FeedbackEncryptionPublicKey")
+		}
+	}
+
 	// This constraint is expected by logic in Controller.runTunnels().
 
 	if config.PacketTunnelTunFileDescriptor > 0 && config.TunnelPoolSize != 1 {
@@ -1160,7 +1180,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 +1584,14 @@ func (config *Config) makeConfigParameters() map[string]interface{} {
 		applyParameters[parameters.ApplicationParameters] = config.ApplicationParameters
 	}
 
+	if len(config.FeedbackUploadURLs) > 0 {
+		applyParameters[parameters.FeedbackUploadURLs] = config.FeedbackUploadURLs
+	}
+
+	if config.FeedbackEncryptionPublicKey != "" {
+		applyParameters[parameters.FeedbackEncryptionPublicKey] = config.FeedbackEncryptionPublicKey
+	}
+
 	return applyParameters
 }
 

+ 101 - 54
psiphon/feedback.go

@@ -32,21 +32,18 @@ import (
 	"crypto/x509"
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"net/http"
-	"strings"
+	"net/url"
+	"path"
 	"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 +97,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(ctx context.Context, configJson, diagnosticsJson, uploadPath string) error {
 
 	config, err := LoadConfig([]byte(configJson))
 	if err != nil {
@@ -112,6 +109,25 @@ 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()
+	getTacticsCtx, cancelFunc := context.WithTimeout(ctx, timeout)
+	defer cancelFunc()
+	// Note: GetTactics will fail silently if the datastore used for retrieving
+	// and storing tactics is opened by another process.
+	GetTactics(getTacticsCtx, config)
+
+	// Get the latest client parameters
+	p = config.GetClientParameters().Get()
+	feedbackUploadMinRetryDelay := p.Duration(parameters.FeedbackUploadRetryMinDelaySeconds)
+	feedbackUploadMaxRetryDelay := p.Duration(parameters.FeedbackUploadRetryMaxDelaySeconds)
+	feedbackUploadTimeout := p.Duration(parameters.FeedbackUploadTimeoutSeconds)
+	feedbackUploadMaxAttempts := p.Int(parameters.FeedbackUploadMaxAttempts)
+	transferURLs := p.TransferURLs(parameters.FeedbackUploadURLs)
+	p.Close()
+
 	untunneledDialConfig := &DialConfig{
 		UpstreamProxyURL:              config.UpstreamProxyURL,
 		CustomHeaders:                 config.CustomHeaders,
@@ -121,65 +137,96 @@ 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 < feedbackUploadMaxAttempts; i++ {
+
+		uploadURL := transferURLs.Select(i)
+		if uploadURL == nil {
+			return errors.TraceNew("error no feedback upload URL selected")
+		}
+
+		b64PublicKey := uploadURL.B64EncodedPublicKey
+		if b64PublicKey == "" {
+			if config.FeedbackEncryptionPublicKey == "" {
+				return errors.TraceNew("error no default encryption key")
+			}
+			b64PublicKey = config.FeedbackEncryptionPublicKey
+		}
 
-	for i := 0; i < FEEDBACK_UPLOAD_MAX_RETRIES; i++ {
-		err = uploadFeedback(
+		secureFeedback, err := encryptFeedback(diagnosticsJson, b64PublicKey)
+		if err != nil {
+			return errors.Trace(err)
+		}
+
+		feedbackUploadCtx, cancelFunc := context.WithTimeout(
+			ctx,
+			feedbackUploadTimeout)
+		defer cancelFunc()
+
+		client, err := MakeUntunneledHTTPClient(
+			feedbackUploadCtx,
 			config,
 			untunneledDialConfig,
-			secureFeedback,
-			url,
-			MakePsiphonUserAgent(config),
-			headerPieces)
+			nil,
+			uploadURL.SkipVerify)
 		if err != nil {
-			time.Sleep(FEEDBACK_UPLOAD_RETRY_DELAY_SECONDS * time.Second)
-		} else {
-			break
+			return errors.Trace(err)
 		}
-	}
 
-	return err
-}
+		parsedURL, err := url.Parse(uploadURL.URL)
+		if err != nil {
+			return errors.TraceMsg(err, "failed to parse feedback upload URL")
+		}
 
-// Attempt to upload feedback data to server.
-func uploadFeedback(
-	config *Config, dialConfig *DialConfig, feedbackData []byte, url, userAgent string, headerPieces []string) error {
+		parsedURL.Path = path.Join(parsedURL.Path, uploadPath, uploadId)
 
-	ctx, cancelFunc := context.WithTimeout(
-		context.Background(),
-		time.Duration(FEEDBACK_UPLOAD_TIMEOUT_SECONDS*time.Second))
-	defer cancelFunc()
+		request, err := http.NewRequestWithContext(feedbackUploadCtx, "PUT", parsedURL.String(), bytes.NewBuffer(secureFeedback))
+		if err != nil {
+			return errors.Trace(err)
+		}
 
-	client, err := MakeUntunneledHTTPClient(
-		ctx,
-		config,
-		dialConfig,
-		nil,
-		false)
-	if err != nil {
-		return err
-	}
+		for k, v := range uploadURL.RequestHeaders {
+			request.Header.Set(k, v)
+		}
+		request.Header.Set("User-Agent", MakePsiphonUserAgent(config))
 
-	req, err := http.NewRequest("PUT", url, bytes.NewBuffer(feedbackData))
-	if err != nil {
-		return errors.Trace(err)
+		err = uploadFeedback(client, request)
+		cancelFunc()
+		if err != nil {
+			if ctx.Err() != nil {
+				// Input context has completed
+				return errors.TraceMsg(err,
+					fmt.Sprintf("feedback upload attempt %d/%d cancelled", i+1, feedbackUploadMaxAttempts))
+			}
+			// Do not sleep after the last attempt
+			if i+1 < feedbackUploadMaxAttempts {
+				// Log error, sleep and then retry
+				timeUntilRetry := prng.Period(feedbackUploadMinRetryDelay, feedbackUploadMaxRetryDelay)
+				NoticeWarning(
+					"feedback upload attempt %d/%d failed (retry in %.0fs): %s",
+					i+1, feedbackUploadMaxAttempts, timeUntilRetry.Seconds(), errors.Trace(err))
+				select {
+				case <-ctx.Done():
+					return errors.TraceNew(
+						fmt.Sprintf("feedback upload attempt %d/%d cancelled before attempt",
+							i+2, feedbackUploadMaxAttempts))
+				case <-time.After(timeUntilRetry):
+				}
+				continue
+			}
+			return errors.TraceMsg(err,
+				fmt.Sprintf("feedback upload failed after %d attempts", i+1))
+		}
+		return nil
 	}
 
-	req.Header.Set("User-Agent", userAgent)
+	return nil
+}
 
-	req.Header.Set(headerPieces[0], headerPieces[1])
+// Attempt to upload feedback data to server.
+func uploadFeedback(
+	client *http.Client, req *http.Request) error {
 
 	resp, err := client.Do(req)
 	if err != nil {
@@ -188,7 +235,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

TEMPAT SAMPAH
psiphon/feedback_test.config.enc


+ 28 - 14
psiphon/feedback_test.go

@@ -20,6 +20,7 @@
 package psiphon
 
 import (
+	"context"
 	"encoding/json"
 	"io/ioutil"
 	"testing"
@@ -46,36 +47,49 @@ func TestFeedbackUpload(t *testing.T) {
 		t.Skipf("error loading configuration file: %s", err)
 	}
 
-	shortRevHash, err := ioutil.ReadFile("git_rev")
-	if err != nil {
-		// Skip, don't fail, if git rev file is not present
-		t.Skipf("error loading git revision file: %s", err)
-	}
+	// Unmarshal config, configure data root directory and re-marshal
 
 	var config map[string]interface{}
 
 	err = json.Unmarshal(configFileContents, &config)
 	if err != nil {
-		t.Error(err.Error())
-		t.FailNow()
+		t.Fatalf("Unmarshal failed: %s", err)
+	}
+
+	testDataDirName, err := ioutil.TempDir("", "psiphon-feedback-test")
+	if err != nil {
+		t.Fatalf("TempDir failed: %s", err)
+	}
+	config["DataRootDirectory"] = testDataDirName
+
+	configFileContents, err = json.Marshal(config)
+	if err != nil {
+		t.Fatalf("Marshal failed: %s", err)
+	}
+
+	// git_rev is a file which contains the shortened hash of the latest commit
+	// pointed to by HEAD, i.e. git rev-parse --short HEAD.
+
+	shortRevHash, err := ioutil.ReadFile("git_rev")
+	if err != nil {
+		// Skip, don't fail, if git rev file is not present
+		t.Skipf("error loading git revision file: %s", err)
 	}
 
-	// Form dummy feedback data which can be verified later
+	// Construct feedback data which can be verified later
 	diagnostics := Diagnostics{}
-	diagnostics.Feedback.Message.Text = "Travis test feedback. Revision " + string(shortRevHash)
+	diagnostics.Feedback.Message.Text = "Test feedback from feedback_test.go, revision: " + string(shortRevHash)
 	diagnostics.Metadata.Id = "0000000000000000"
 	diagnostics.Metadata.Platform = "android"
 	diagnostics.Metadata.Version = 4
 
 	diagnosticData, err := json.Marshal(diagnostics)
 	if err != nil {
-		t.Error(err.Error())
-		t.FailNow()
+		t.Fatalf("Marshal failed: %s", err)
 	}
 
-	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(context.Background(), string(configFileContents), string(diagnosticData), "")
 	if err != nil {
-		t.Error(err.Error())
-		t.FailNow()
+		t.Fatalf("SendFeedback failed: %s", err)
 	}
 }

+ 10 - 8
psiphon/remoteServerList.go

@@ -59,7 +59,8 @@ func FetchCommonRemoteServerList(
 	downloadTimeout := p.Duration(parameters.FetchRemoteServerListTimeout)
 	p.Close()
 
-	downloadURL, canonicalURL, skipVerify := urls.Select(attempt)
+	downloadURL := urls.Select(attempt)
+	canonicalURL := urls.CanonicalURL()
 
 	newETag, downloadStatRecorder, err := downloadRemoteServerListFile(
 		ctx,
@@ -67,9 +68,9 @@ func FetchCommonRemoteServerList(
 		tunnel,
 		untunneledDialConfig,
 		downloadTimeout,
-		downloadURL,
+		downloadURL.URL,
 		canonicalURL,
-		skipVerify,
+		downloadURL.SkipVerify,
 		"",
 		config.GetRemoteServerListDownloadFilename())
 	if err != nil {
@@ -150,8 +151,9 @@ func FetchObfuscatedServerLists(
 	downloadTimeout := p.Duration(parameters.FetchRemoteServerListTimeout)
 	p.Close()
 
-	rootURL, canonicalRootURL, skipVerify := urls.Select(attempt)
-	downloadURL := osl.GetOSLRegistryURL(rootURL)
+	rootURL := urls.Select(attempt)
+	canonicalRootURL := urls.CanonicalURL()
+	downloadURL := osl.GetOSLRegistryURL(rootURL.URL)
 	canonicalURL := osl.GetOSLRegistryURL(canonicalRootURL)
 
 	downloadFilename := osl.GetOSLRegistryFilename(config.GetObfuscatedServerListDownloadDirectory())
@@ -185,7 +187,7 @@ func FetchObfuscatedServerLists(
 		downloadTimeout,
 		downloadURL,
 		canonicalURL,
-		skipVerify,
+		rootURL.SkipVerify,
 		"",
 		downloadFilename)
 	if err != nil {
@@ -261,9 +263,9 @@ func FetchObfuscatedServerLists(
 			tunnel,
 			untunneledDialConfig,
 			downloadTimeout,
-			rootURL,
+			rootURL.URL,
 			canonicalRootURL,
-			skipVerify,
+			rootURL.SkipVerify,
 			publicKey,
 			lookupSLOKs,
 			oslFileSpec) {

+ 4 - 4
psiphon/upgradeDownload.go

@@ -84,14 +84,14 @@ func DownloadUpgrade(
 
 	// Select tunneled or untunneled configuration
 
-	downloadURL, _, skipVerify := urls.Select(attempt)
+	downloadURL := urls.Select(attempt)
 
 	httpClient, _, err := MakeDownloadHTTPClient(
 		ctx,
 		config,
 		tunnel,
 		untunneledDialConfig,
-		skipVerify)
+		downloadURL.SkipVerify)
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -102,7 +102,7 @@ func DownloadUpgrade(
 	availableClientVersion := handshakeVersion
 	if availableClientVersion == "" {
 
-		request, err := http.NewRequest("HEAD", downloadURL, nil)
+		request, err := http.NewRequest("HEAD", downloadURL.URL, nil)
 		if err != nil {
 			return errors.Trace(err)
 		}
@@ -159,7 +159,7 @@ func DownloadUpgrade(
 	n, _, err := ResumeDownload(
 		ctx,
 		httpClient,
-		downloadURL,
+		downloadURL.URL,
 		MakePsiphonUserAgent(config),
 		downloadFilename,
 		"")