Просмотр исходного кода

Do not log file paths

Make an effort to not log file paths because they may contain
sensitive values, such as the account name of the user on the
system.
mirokuratczyk 5 лет назад
Родитель
Сommit
d5133108c6

+ 12 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel.xcodeproj/project.pbxproj

@@ -45,6 +45,9 @@
 		CE3D1DA623906003009A4AF6 /* Backups.m in Sources */ = {isa = PBXBuildFile; fileRef = CE3D1DA423906003009A4AF6 /* Backups.m */; };
 		CE4616BF2539493600D1243E /* Reachability+HasNetworkConnectivity.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4616BD2539493600D1243E /* Reachability+HasNetworkConnectivity.h */; };
 		CE4616C02539493600D1243E /* Reachability+HasNetworkConnectivity.m in Sources */ = {isa = PBXBuildFile; fileRef = CE4616BE2539493600D1243E /* Reachability+HasNetworkConnectivity.m */; };
+		CE9549F525C8AAEE00F9AF86 /* Redactor.h in Headers */ = {isa = PBXBuildFile; fileRef = CE9549F325C8AAEE00F9AF86 /* Redactor.h */; };
+		CE9549F625C8AAEE00F9AF86 /* Redactor.m in Sources */ = {isa = PBXBuildFile; fileRef = CE9549F425C8AAEE00F9AF86 /* Redactor.m */; };
+		CE9549F925C98CA100F9AF86 /* RedactorTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CE9549F825C98CA100F9AF86 /* RedactorTests.m */; };
 		CEC229FC24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h in Headers */ = {isa = PBXBuildFile; fileRef = CEC229FA24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.h */; };
 		CEC229FD24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m in Sources */ = {isa = PBXBuildFile; fileRef = CEC229FB24F047E700534D04 /* PsiphonProviderNoticeHandlerShim.m */; };
 		CECF01442538D34100CD3E5C /* IPv6Synthesizer.h in Headers */ = {isa = PBXBuildFile; fileRef = CECF01422538D34100CD3E5C /* IPv6Synthesizer.h */; };
@@ -126,6 +129,9 @@
 		CE3D1DA423906003009A4AF6 /* Backups.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = Backups.m; sourceTree = "<group>"; };
 		CE4616BD2539493600D1243E /* Reachability+HasNetworkConnectivity.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "Reachability+HasNetworkConnectivity.h"; sourceTree = "<group>"; };
 		CE4616BE2539493600D1243E /* Reachability+HasNetworkConnectivity.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "Reachability+HasNetworkConnectivity.m"; sourceTree = "<group>"; };
+		CE9549F325C8AAEE00F9AF86 /* Redactor.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Redactor.h; sourceTree = "<group>"; };
+		CE9549F425C8AAEE00F9AF86 /* Redactor.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = Redactor.m; sourceTree = "<group>"; };
+		CE9549F825C98CA100F9AF86 /* RedactorTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RedactorTests.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>"; };
 		CECF01422538D34100CD3E5C /* IPv6Synthesizer.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = IPv6Synthesizer.h; sourceTree = "<group>"; };
@@ -244,6 +250,7 @@
 			isa = PBXGroup;
 			children = (
 				66BDB02E1DA6BFCC0079384C /* PsiphonTunnelTests.m */,
+				CE9549F825C98CA100F9AF86 /* RedactorTests.m */,
 				66BDB0301DA6BFCC0079384C /* Info.plist */,
 			);
 			path = PsiphonTunnelTests;
@@ -287,6 +294,8 @@
 				CE3D1DA423906003009A4AF6 /* Backups.m */,
 				52BE676725B8A615002DB553 /* PsiphonClientPlatform.h */,
 				52BE676B25B8A635002DB553 /* PsiphonClientPlatform.m */,
+				CE9549F325C8AAEE00F9AF86 /* Redactor.h */,
+				CE9549F425C8AAEE00F9AF86 /* Redactor.m */,
 			);
 			path = Utils;
 			sourceTree = "<group>";
@@ -342,6 +351,7 @@
 				CECF01492538DD0B00CD3E5C /* PsiphonProviderNetwork.h in Headers */,
 				66BDB05F1DC26CCC0079384C /* SBJson4StreamParserState.h in Headers */,
 				66BDB0311DA6BFCC0079384C /* PsiphonTunnel.h in Headers */,
+				CE9549F525C8AAEE00F9AF86 /* Redactor.h in Headers */,
 				6685BDCA1E2E882800F0E414 /* Psi.h in Headers */,
 				66BDB0651DC26CCC0079384C /* SBJson4StreamWriterState.h in Headers */,
 				CECF01502538E14B00CD3E5C /* NetworkID.h in Headers */,
@@ -498,6 +508,7 @@
 				662659281DD270E900872F6C /* Reachability.m in Sources */,
 				66BDB0601DC26CCC0079384C /* SBJson4StreamParserState.m in Sources */,
 				CE3D1DA623906003009A4AF6 /* Backups.m in Sources */,
+				CE9549F625C8AAEE00F9AF86 /* Redactor.m in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -506,6 +517,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				66BDB02F1DA6BFCC0079384C /* PsiphonTunnelTests.m in Sources */,
+				CE9549F925C98CA100F9AF86 /* RedactorTests.m in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

+ 16 - 11
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/PsiphonTunnel.m

@@ -36,6 +36,7 @@
 #import <resolv.h>
 #import <netdb.h>
 #import "PsiphonClientPlatform.h"
+#import "Redactor.h"
 
 #define GOOGLE_DNS_1 @"8.8.4.4"
 #define GOOGLE_DNS_2 @"8.8.8.8"
@@ -632,8 +633,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
     // Note: this deprecates the "DataStoreDirectory" config field.
     NSURL *defaultDataRootDirectoryURL = [PsiphonTunnel defaultDataRootDirectoryWithError:&err];
     if (err != nil) {
+        NSString *redactedErr = [Redactor stripFilePaths:err.localizedDescription];
         NSString *s = [NSString stringWithFormat:@"Unable to get defaultDataRootDirectoryURL: %@",
-                       err.localizedDescription];
+                       redactedErr];
         *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
                                         code:PsiphonTunnelErrorCodeConfigError
                                     userInfo:@{NSLocalizedDescriptionKey:s}];
@@ -649,8 +651,10 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
                                attributes:nil
                                     error:&err];
         if (err != nil) {
-            NSString *s = [NSString stringWithFormat: @"Unable to create defaultRootDirectoryURL '%@': %@",
-                           defaultDataRootDirectoryURL, err.localizedDescription];
+            NSString *redactedErr = [Redactor stripFilePaths:err.localizedDescription
+                                               withFilePaths:@[defaultDataRootDirectoryURL.path]];
+            NSString *s = [NSString stringWithFormat:@"Unable to create defaultRootDirectoryURL: %@",
+                           redactedErr];
             *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
                                             code:PsiphonTunnelErrorCodeConfigError
                                         userInfo:@{NSLocalizedDescriptionKey:s}];
@@ -660,7 +664,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         config[@"DataRootDirectory"] = defaultDataRootDirectoryURL.path;
     }
     else {
-        logMessage([NSString stringWithFormat:@"DataRootDirectory overridden from '%@' to '%@'", defaultDataRootDirectoryURL.path, config[@"DataRootDirectory"]]);
+        logMessage(@"DataRootDirectory overridden");
     }
 
     // Ensure that the configured data root directory is not backed up to iCloud or iTunes.
@@ -668,7 +672,9 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 
     BOOL succeeded = [Backups excludeFileFromBackup:dataRootDirectory.path err:&err];
     if (!succeeded) {
-        logMessage([NSString stringWithFormat:@"Failed to exclude data root directory from backup: %@", err.localizedDescription]);
+        NSString *redactedErr = [Redactor stripFilePaths:err.localizedDescription
+                                           withFilePaths:@[dataRootDirectory.path]];
+        logMessage([NSString stringWithFormat:@"Failed to exclude data root directory from backup: %@", redactedErr]);
     } else {
         logMessage(@"Excluded data root directory from backup");
     }
@@ -679,7 +685,8 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
 
     NSURL *libraryURL = [PsiphonTunnel libraryURLWithError:&err];
     if (err != nil) {
-        NSString *s = [NSString stringWithFormat: @"Unable to get Library URL: %@", err.localizedDescription];
+        NSString *redactedErr = [Redactor stripFilePaths:err.localizedDescription];
+        NSString *s = [NSString stringWithFormat:@"Unable to get Library URL: %@", redactedErr];
         *outError = [NSError errorWithDomain:PsiphonTunnelErrorDomain
                                         code:PsiphonTunnelErrorCodeConfigError
                                     userInfo:@{NSLocalizedDescriptionKey:s}];
@@ -708,7 +715,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         config[@"MigrateDataStoreDirectory"] = defaultDataStoreDirectoryURL.path;
     }
     else {
-        logMessage([NSString stringWithFormat: @"DataStoreDirectory overridden from '%@' to '%@'", [defaultDataStoreDirectoryURL path], config[@"DataStoreDirectory"]]);
+        logMessage(@"DataStoreDirectory overridden from default");
     }
 
     //
@@ -733,8 +740,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         config[@"MigrateRemoteServerListDownloadFilename"] = defaultRemoteServerListFilename;
     }
     else {
-        logMessage([NSString stringWithFormat: @"RemoteServerListDownloadFilename overridden from '%@' to '%@'",
-                defaultRemoteServerListFilename, config[@"RemoteServerListDownloadFilename"]]);
+        logMessage(@"RemoteServerListDownloadFilename overridden from default");
     }
     
     // If RemoteServerListUrl/RemoteServerListURLs and RemoteServerListSignaturePublicKey
@@ -765,8 +771,7 @@ typedef NS_ERROR_ENUM(PsiphonTunnelErrorDomain, PsiphonTunnelErrorCode) {
         config[@"MigrateObfuscatedServerListDownloadDirectory"] = defaultOSLDirectoryURL.path;
     }
     else {
-        logMessage([NSString stringWithFormat: @"ObfuscatedServerListDownloadDirectory overridden from '%@' to '%@'",
-                [defaultOSLDirectoryURL path], config[@"ObfuscatedServerListDownloadDirectory"]]);
+        logMessage(@"ObfuscatedServerListDownloadDirectory overridden from default");
     }
     
     // If ObfuscatedServerListRootURL/ObfuscatedServerListRootURLs is absent,

+ 3 - 1
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Utils/Backups.h

@@ -24,9 +24,11 @@ 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.
+/// If false 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.
+/// @return If true, then the operation succeeded. If false, then the file was not successfully excluded from
+/// backup and the error is populated.
 + (BOOL)excludeFileFromBackup:(NSString*)filePath err:(NSError * _Nullable *)err;
 
 @end

+ 46 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Utils/Redactor.h

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2021, 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
+
+/// Redactor implements a set of functions to redact sensitive values from data.
+@interface Redactor : NSObject
+
+/// Returns a redacted copy of the provided string where any file paths are replaced with "[redacted]". For example,
+/// the input "prefix /a/b/c suffix" will result in the return value "prefix [redacted] suffix".
+/// @warning An attempt is made to redact file paths, but there is no guarantee that all file path schemes on the
+/// given system will be caught.
+/// @param s The string to redact.
+/// @return A copy of the provided string with any file paths redacted.
++ (NSString*)stripFilePaths:(NSString*)s;
+
+/// Version of stripFilePaths which first replaces any occurrences of the provided file paths in the input string with
+/// "[redacted]".
+/// @warning An attempt is made to redact file paths, but there is no guarantee that all file path schemes on the given
+/// system will be caught.
+/// @param s The string to redact.
+/// @param filePaths File paths to redact directly.
+/// @return A copy of the provided string with any file paths redacted.
++ (NSString*)stripFilePaths:(NSString*)s withFilePaths:(NSArray<NSString*>*_Nullable)filePaths;
+
+@end
+
+NS_ASSUME_NONNULL_END

+ 63 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnel/Utils/Redactor.m

@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2021, 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 "Redactor.h"
+
+@implementation Redactor
+
++ (NSString*)stripFilePaths:(NSString*)s {
+    return [Redactor stripFilePaths:s withFilePaths:nil];
+}
+
++ (NSString*)stripFilePaths:(NSString*)s withFilePaths:(NSArray<NSString*>*)filePaths {
+
+    NSMutableString *ret = [s mutableCopy];
+    NSRange replaceRange = NSMakeRange(0, [ret length]);
+
+    for (NSString *filePath in filePaths) {
+        [ret replaceOccurrencesOfString:filePath withString:@"[redacted]"
+                                options:kNilOptions range:replaceRange];
+    }
+
+    NSString *filePathRegex =
+        // File path
+        @"("
+            // Leading characters
+            @"[^ ]*"
+            // At least one path separator
+            @"/"
+            // Path component; take until next space
+            @"[^ ]*"
+        @")+";
+
+    NSError *err = nil;
+    NSRegularExpression *regex = [NSRegularExpression regularExpressionWithPattern:filePathRegex
+                                                                           options:kNilOptions
+                                                                             error:&err];
+    if (err != nil) {
+        [NSException raise:@"Regex compile failed" format:@"failed to compile %@", filePathRegex];
+    }
+
+    NSRange searchRange = NSMakeRange(0, [ret length]);
+    [regex replaceMatchesInString:ret options:kNilOptions range:searchRange withTemplate:@"[redacted]"];
+
+    return ret;
+}
+
+@end

+ 18 - 7
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnelTests/PsiphonTunnelTests.m

@@ -1,10 +1,21 @@
-//
-//  PsiphonTunnelTests.m
-//  PsiphonTunnelTests
-//
-//  Created by Adam Pritchard on 2016-10-06.
-//  Copyright © 2016 Psiphon Inc. All rights reserved.
-//
+/*
+ * Copyright (c) 2021, 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 <XCTest/XCTest.h>
 

+ 59 - 0
MobileLibrary/iOS/PsiphonTunnel/PsiphonTunnelTests/RedactorTests.m

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2021, 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 <XCTest/XCTest.h>
+
+#import "Redactor.h"
+
+@interface RedactorTests : XCTestCase
+
+@end
+
+@implementation RedactorTests
+
+- (void)testRedactor {
+    [self run:@"prefix /a suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+    [self run:@"prefix /a/b/c/d suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+    [self run:@"prefix ./a/b/c/d suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+    [self run:@"prefix a/b/c/d suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+    [self run:@"prefix ../a/b/c/d/../ suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+    [self run:@"prefix ~/a/b/c/d suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+    [self run:@"prefix /a/b c/d suffix" filePaths:nil expect:@"prefix [redacted] [redacted] suffix"];
+    [self run:@"prefix /a/b%20c/d suffix" filePaths:nil expect:@"prefix [redacted] suffix"];
+
+    // Unhandled case
+    [self run:@"prefix /a/file name with spaces /e/f/g/ suffix"
+    filePaths:nil
+       expect:@"prefix [redacted] name with spaces [redacted] suffix"];
+
+    // Handle unhandled case
+    [self run:@"prefix /a/file name with spaces /e/f/g/ suffix"
+    filePaths:@[@"/a/file name with spaces"]
+       expect:@"prefix [redacted] [redacted] suffix"];
+}
+
+- (void)run:(NSString*)input filePaths:(NSArray<NSString*>*)filePaths expect:(NSString*)expect {
+    NSString *redacted = [Redactor stripFilePaths:input withFilePaths:filePaths];
+    if ([redacted isEqualToString:expect] == false) {
+        XCTFail(@"Error: \"%@\" not equal to expect value \"%@\"", redacted, expect);
+    }
+}
+
+@end

+ 0 - 47
psiphon/common/utils.go

@@ -193,53 +193,6 @@ func FileExists(filePath string) bool {
 	return true
 }
 
-// FileMigration represents the action of moving a file, or directory, to a new
-// location.
-type FileMigration struct {
-
-	// OldPath is the current location of the file.
-	OldPath string
-
-	// NewPath is the location that the file should be moved to.
-	NewPath string
-
-	// IsDir should be set to true if the file is a directory.
-	IsDir bool
-}
-
-// DoFileMigration performs the specified file move operation. An error will be
-// returned and the operation will not performed if: a file is expected, but a
-// directory is found; a directory is expected, but a file is found; or a file,
-// or directory, already exists at the target path of the move operation.
-func DoFileMigration(migration FileMigration) error {
-	if !FileExists(migration.OldPath) {
-		return errors.Tracef("%s does not exist", migration.OldPath)
-	}
-	info, err := os.Stat(migration.OldPath)
-	if err != nil {
-		return errors.Tracef("error getting file info of %s: %s", migration.OldPath, err.Error())
-	}
-	if info.IsDir() != migration.IsDir {
-		if migration.IsDir {
-			return errors.Tracef("expected directory %s to be directory but found file", migration.OldPath)
-		}
-
-		return errors.Tracef("expected %s to be file but found directory",
-			migration.OldPath)
-	}
-
-	if FileExists(migration.NewPath) {
-		return errors.Tracef("%s already exists, will not overwrite", migration.NewPath)
-	}
-
-	err = os.Rename(migration.OldPath, migration.NewPath)
-	if err != nil {
-		return errors.Tracef("renaming %s as %s failed with error %s", migration.OldPath, migration.NewPath, err.Error())
-	}
-
-	return nil
-}
-
 // SafeParseURL wraps url.Parse, stripping the input URL from any error
 // message. This allows logging url.Parse errors without unintentially logging
 // PII that may appear in the input URL.

+ 41 - 25
psiphon/config.go

@@ -852,7 +852,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 	if config.DataRootDirectory == "" {
 		wd, err := os.Getwd()
 		if err != nil {
-			return errors.Trace(err)
+			return errors.Trace(StripFilePathsError(err))
 		}
 		config.DataRootDirectory = wd
 	}
@@ -862,7 +862,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 	if !common.FileExists(dataDirectoryPath) {
 		err := os.Mkdir(dataDirectoryPath, os.ModePerm)
 		if err != nil {
-			return errors.Tracef("failed to create datastore directory %s with error: %s", dataDirectoryPath, err.Error())
+			return errors.Tracef("failed to create datastore directory with error: %s", StripFilePathsError(err, dataDirectoryPath))
 		}
 	}
 
@@ -889,16 +889,19 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 			noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, "Config migration: need migration")
 			noticeMigrations := migrationsFromLegacyNoticeFilePaths(config)
 
+			successfulMigrations := 0
+
 			for _, migration := range noticeMigrations {
-				err := common.DoFileMigration(migration)
+				err := DoFileMigration(migration)
 				if err != nil {
 					alertMsg := fmt.Sprintf("Config migration: %s", errors.Trace(err))
 					noticeMigrationAlertMsgs = append(noticeMigrationAlertMsgs, alertMsg)
 				} else {
-					infoMsg := fmt.Sprintf("Config migration: moved %s to %s", migration.OldPath, migration.NewPath)
-					noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, infoMsg)
+					successfulMigrations += 1
 				}
 			}
+			infoMsg := fmt.Sprintf("Config migration: %d/%d notice files successfully migrated", successfulMigrations, len(noticeMigrations))
+			noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, infoMsg)
 		} else {
 			noticeMigrationInfoMsgs = append(noticeMigrationInfoMsgs, "Config migration: migration already completed")
 		}
@@ -966,7 +969,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 	if !common.FileExists(dataStoreDirectoryPath) {
 		err := os.Mkdir(dataStoreDirectoryPath, os.ModePerm)
 		if err != nil {
-			return errors.Tracef("failed to create datastore directory %s with error: %s", dataStoreDirectoryPath, err.Error())
+			return errors.Tracef("failed to create datastore directory with error: %s", StripFilePathsError(err, dataStoreDirectoryPath))
 		}
 	}
 
@@ -975,7 +978,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 	if !common.FileExists(oslDirectoryPath) {
 		err := os.Mkdir(oslDirectoryPath, os.ModePerm)
 		if err != nil {
-			return errors.Tracef("failed to create osl directory %s with error: %s", oslDirectoryPath, err.Error())
+			return errors.Tracef("failed to create osl directory with error: %s", StripFilePathsError(err, oslDirectoryPath))
 		}
 	}
 
@@ -990,7 +993,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 	// Validate config fields.
 
 	if !common.FileExists(config.DataRootDirectory) {
-		return errors.Tracef("DataRootDirectory does not exist: %s", config.DataRootDirectory)
+		return errors.TraceNew("DataRootDirectory does not exist")
 	}
 
 	if config.PropagationChannelId == "" {
@@ -1140,7 +1143,7 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 			if err != nil {
 				return errors.Trace(err)
 			}
-			NoticeInfo("MigrateDataStoreDirectory unset, using working directory %s", wd)
+			NoticeInfo("MigrateDataStoreDirectory unset, using working directory")
 			config.MigrateDataStoreDirectory = wd
 		}
 
@@ -1154,31 +1157,33 @@ func (config *Config) Commit(migrateFromLegacyFields bool) error {
 
 		// Do migrations
 
+		successfulMigrations := 0
 		for _, migration := range migrations {
-			err := common.DoFileMigration(migration)
+			err := DoFileMigration(migration)
 			if err != nil {
 				NoticeWarning("Config migration: %s", errors.Trace(err))
 			} else {
-				NoticeInfo("Config migration: moved %s to %s", migration.OldPath, migration.NewPath)
+				successfulMigrations += 1
 			}
 		}
+		NoticeInfo(fmt.Sprintf("Config migration: %d/%d legacy files successfully migrated", successfulMigrations, len(migrations)))
 
 		// Remove OSL directory if empty
 		if config.MigrateObfuscatedServerListDownloadDirectory != "" {
 			files, err := ioutil.ReadDir(config.MigrateObfuscatedServerListDownloadDirectory)
 			if err != nil {
-				NoticeWarning("Error reading OSL directory %s: %s", config.MigrateObfuscatedServerListDownloadDirectory, errors.Trace(err))
+				NoticeWarning("Error reading OSL directory: %s", errors.Trace(StripFilePathsError(err, config.MigrateObfuscatedServerListDownloadDirectory)))
 			} else if len(files) == 0 {
 				err := os.Remove(config.MigrateObfuscatedServerListDownloadDirectory)
 				if err != nil {
-					NoticeWarning("Error deleting empty OSL directory %s: %s", config.MigrateObfuscatedServerListDownloadDirectory, errors.Trace(err))
+					NoticeWarning("Error deleting empty OSL directory: %s", errors.Trace(StripFilePathsError(err, config.MigrateObfuscatedServerListDownloadDirectory)))
 				}
 			}
 		}
 
 		f, err := os.Create(migrationCompleteFilePath)
 		if err != nil {
-			NoticeWarning("Config migration: failed to create %s with error %s", migrationCompleteFilePath, errors.Trace(err))
+			NoticeWarning("Config migration: failed to create migration completed file with error %s", errors.Trace(StripFilePathsError(err, migrationCompleteFilePath)))
 		} else {
 			NoticeInfo("Config migration: completed")
 			f.Close()
@@ -1845,24 +1850,27 @@ func (n *loggingNetworkIDGetter) GetNetworkID() string {
 // with the legacy config fields HomepageNoticesFilename and
 // RotatingNoticesFilename, to the new file paths used by Psiphon which exist
 // under the data root directory.
-func migrationsFromLegacyNoticeFilePaths(config *Config) []common.FileMigration {
-	var noticeMigrations []common.FileMigration
+func migrationsFromLegacyNoticeFilePaths(config *Config) []FileMigration {
+	var noticeMigrations []FileMigration
 
 	if config.MigrateHomepageNoticesFilename != "" {
-		noticeMigrations = append(noticeMigrations, common.FileMigration{
+		noticeMigrations = append(noticeMigrations, FileMigration{
+			Name:    "hompage",
 			OldPath: config.MigrateHomepageNoticesFilename,
 			NewPath: config.GetHomePageFilename(),
 		})
 	}
 
 	if config.MigrateRotatingNoticesFilename != "" {
-		migrations := []common.FileMigration{
+		migrations := []FileMigration{
 			{
+				Name:    "notices",
 				OldPath: config.MigrateRotatingNoticesFilename,
 				NewPath: config.GetNoticesFilename(),
 				IsDir:   false,
 			},
 			{
+				Name:    "notices.1",
 				OldPath: config.MigrateRotatingNoticesFilename + ".1",
 				NewPath: config.GetNoticesFilename() + ".1",
 			},
@@ -1877,14 +1885,17 @@ func migrationsFromLegacyNoticeFilePaths(config *Config) []common.FileMigration
 // performed to move files from legacy file paths, which were configured with
 // legacy config fields, to the new file paths used by Psiphon which exist
 // under the data root directory.
-func migrationsFromLegacyFilePaths(config *Config) ([]common.FileMigration, error) {
+// Note: an attempt is made to redact any file paths from the returned error.
+func migrationsFromLegacyFilePaths(config *Config) ([]FileMigration, error) {
 
-	migrations := []common.FileMigration{
+	migrations := []FileMigration{
 		{
+			Name:    "psiphon.boltdb",
 			OldPath: filepath.Join(config.MigrateDataStoreDirectory, "psiphon.boltdb"),
 			NewPath: filepath.Join(config.GetDataStoreDirectory(), "psiphon.boltdb"),
 		},
 		{
+			Name:    "psiphon.boltdb.lock",
 			OldPath: filepath.Join(config.MigrateDataStoreDirectory, "psiphon.boltdb.lock"),
 			NewPath: filepath.Join(config.GetDataStoreDirectory(), "psiphon.boltdb.lock"),
 		},
@@ -1894,16 +1905,19 @@ func migrationsFromLegacyFilePaths(config *Config) ([]common.FileMigration, erro
 
 		// Migrate remote server list files
 
-		rslMigrations := []common.FileMigration{
+		rslMigrations := []FileMigration{
 			{
+				Name:    "remote_server_list",
 				OldPath: config.MigrateRemoteServerListDownloadFilename,
 				NewPath: config.GetRemoteServerListDownloadFilename(),
 			},
 			{
+				Name:    "remote_server_list.part",
 				OldPath: config.MigrateRemoteServerListDownloadFilename + ".part",
 				NewPath: config.GetRemoteServerListDownloadFilename() + ".part",
 			},
 			{
+				Name:    "remote_server_list.part.etag",
 				OldPath: config.MigrateRemoteServerListDownloadFilename + ".part.etag",
 				NewPath: config.GetRemoteServerListDownloadFilename() + ".part.etag",
 			},
@@ -1923,11 +1937,12 @@ func migrationsFromLegacyFilePaths(config *Config) ([]common.FileMigration, erro
 
 		files, err := ioutil.ReadDir(config.MigrateObfuscatedServerListDownloadDirectory)
 		if err != nil {
-			NoticeWarning("Migration: failed to read directory %s with error %s", config.MigrateObfuscatedServerListDownloadDirectory, err)
+			NoticeWarning("Migration: failed to read OSL download directory with error %s", StripFilePathsError(err, config.MigrateObfuscatedServerListDownloadDirectory))
 		} else {
 			for _, file := range files {
 				if oslFileRegex.MatchString(file.Name()) {
-					fileMigration := common.FileMigration{
+					fileMigration := FileMigration{
+						Name:    "osl",
 						OldPath: filepath.Join(config.MigrateObfuscatedServerListDownloadDirectory, file.Name()),
 						NewPath: filepath.Join(config.GetObfuscatedServerListDownloadDirectory(), file.Name()),
 					}
@@ -1957,7 +1972,7 @@ func migrationsFromLegacyFilePaths(config *Config) ([]common.FileMigration, erro
 
 		files, err := ioutil.ReadDir(upgradeDownloadDir)
 		if err != nil {
-			NoticeWarning("Migration: failed to read directory %s with error %s", upgradeDownloadDir, err)
+			NoticeWarning("Migration: failed to read upgrade download directory with error %s", StripFilePathsError(err, upgradeDownloadDir))
 		} else {
 
 			for _, file := range files {
@@ -1966,7 +1981,8 @@ func migrationsFromLegacyFilePaths(config *Config) ([]common.FileMigration, erro
 
 					oldFileSuffix := strings.TrimPrefix(file.Name(), oldUpgradeDownloadFilename)
 
-					fileMigration := common.FileMigration{
+					fileMigration := FileMigration{
+						Name:    "upgrade",
 						OldPath: filepath.Join(upgradeDownloadDir, file.Name()),
 						NewPath: config.GetUpgradeDownloadFilename() + oldFileSuffix,
 					}

+ 4 - 0
psiphon/notice.go

@@ -242,6 +242,10 @@ func (nl *noticeLogger) outputNotice(noticeType string, noticeFlags uint32, args
 	// in I/O error messages.
 	output = StripIPAddresses(output)
 
+	// Don't call StripFilePaths here, as the file path redaction can
+	// potentially match many non-path strings. Instead, StripFilePaths should
+	// be applied in specific cases.
+
 	nl.mutex.Lock()
 	defer nl.mutex.Unlock()
 

+ 87 - 0
psiphon/utils.go

@@ -27,6 +27,7 @@ import (
 	"net"
 	"net/url"
 	"os"
+	"path/filepath"
 	"regexp"
 	"runtime"
 	"runtime/debug"
@@ -164,6 +165,34 @@ func StripIPAddressesString(s string) string {
 	return stripIPAddressAndPortRegex.ReplaceAllString(s, "[redacted]")
 }
 
+var stripFilePathRegex = regexp.MustCompile(
+	// File path
+	`(` +
+		// Leading characters
+		`[^ ]*` +
+		// At least one path separator
+		`/` +
+		// Path component; take until next space
+		`[^ ]*` +
+		`)+`)
+
+// StripFilePaths returns a copy of the input with all file paths
+// replaced by "[redacted]". First any occurrences of the provided file paths
+// are replaced and then an attempt is made to replace any other file paths by
+// searching with a heuristic. The latter is a best effort attempt it is not
+// guaranteed that it will catch every file path.
+func StripFilePaths(s string, filePaths ...string) string {
+	for _, filePath := range filePaths {
+		s = strings.ReplaceAll(s, filePath, "[redacted]")
+	}
+	return stripFilePathRegex.ReplaceAllLiteralString(filepath.ToSlash(s), "[redacted]")
+}
+
+// StripFilePathsError is StripFilePaths for errors.
+func StripFilePathsError(err error, filePaths ...string) error {
+	return std_errors.New(StripFilePaths(err.Error(), filePaths...))
+}
+
 // RedactNetError removes network address information from a "net" package
 // error message. Addresses may be domains or IP addresses.
 //
@@ -305,3 +334,61 @@ func (c conditionallyEnabledComponents) MarionetteEnabled() bool {
 func (c conditionallyEnabledComponents) RefractionNetworkingEnabled() bool {
 	return refraction.Enabled()
 }
+
+// FileMigration represents the action of moving a file, or directory, to a new
+// location.
+type FileMigration struct {
+
+	// Name is the name of the migration for logging because file paths are not
+	// logged as they may contain sensitive information.
+	Name string
+
+	// OldPath is the current location of the file.
+	OldPath string
+
+	// NewPath is the location that the file should be moved to.
+	NewPath string
+
+	// IsDir should be set to true if the file is a directory.
+	IsDir bool
+}
+
+// DoFileMigration performs the specified file move operation. An error will be
+// returned and the operation will not performed if: a file is expected, but a
+// directory is found; a directory is expected, but a file is found; or a file,
+// or directory, already exists at the target path of the move operation.
+// Note: an attempt is made to redact any file paths from the returned error.
+func DoFileMigration(migration FileMigration) error {
+
+	// Prefix string added to any errors for debug purposes.
+	errPrefix := ""
+	if len(migration.Name) > 0 {
+		errPrefix = fmt.Sprintf("(%s) ", migration.Name)
+	}
+
+	if !common.FileExists(migration.OldPath) {
+		return errors.TraceNew(errPrefix + "old path does not exist")
+	}
+	info, err := os.Stat(migration.OldPath)
+	if err != nil {
+		return errors.Tracef(errPrefix+"error getting file info: %s", StripFilePathsError(err, migration.OldPath))
+	}
+	if info.IsDir() != migration.IsDir {
+		if migration.IsDir {
+			return errors.TraceNew(errPrefix + "expected directory but found file")
+		}
+
+		return errors.TraceNew(errPrefix + "expected but found directory")
+	}
+
+	if common.FileExists(migration.NewPath) {
+		return errors.TraceNew(errPrefix + "file already exists, will not overwrite")
+	}
+
+	err = os.Rename(migration.OldPath, migration.NewPath)
+	if err != nil {
+		return errors.Tracef(errPrefix+"renaming file failed with error %s", StripFilePathsError(err, migration.OldPath, migration.NewPath))
+	}
+
+	return nil
+}

+ 90 - 0
psiphon/utils_test.go

@@ -20,6 +20,8 @@
 package psiphon
 
 import (
+	"os"
+	"strings"
 	"testing"
 )
 
@@ -101,3 +103,91 @@ func TestStripIPAddresses(t *testing.T) {
 		})
 	}
 }
+
+func TestStripFilePaths(t *testing.T) {
+
+	testCases := []struct {
+		description    string
+		input          string
+		expectedOutput string
+		filePaths      []string
+	}{
+		{
+			"Absolute path",
+			"prefix /a suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"Absolute path with directories",
+			"prefix /a/b/c/d suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"Relative path 1",
+			"prefix ./a/b/c/d suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"Relative path 2",
+			"prefix a/b/c/d suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"Relative path 3",
+			"prefix ../a/b/c/d/../ suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"File path with home directory tilde",
+			"prefix ~/a/b/c/d suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"Multiple file paths",
+			"prefix /a/b c/d suffix",
+			"prefix [redacted] [redacted] suffix",
+			nil,
+		},
+		{
+			"File path with percent encoded spaces",
+			"prefix /a/b%20c/d suffix",
+			"prefix [redacted] suffix",
+			nil,
+		},
+		{
+			"Strip file paths unhandled case",
+			"prefix /a/file name with spaces /e/f/g/ suffix",
+			"prefix [redacted] name with spaces [redacted] suffix",
+			nil,
+		},
+		{
+			"Strip file paths catch unhandled case with provided path",
+			"prefix /a/file name with spaces /e/f/g/ suffix",
+			"prefix [redacted] [redacted] suffix",
+			[]string{"/a/file name with spaces"},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			// For convenience replace separators in input string and
+			// file paths with the OS-specific path separator here instead
+			// constructing the test input strings with os.PathSeparator.
+			input := strings.ReplaceAll(testCase.input, "/", string(os.PathSeparator))
+			var filePaths []string
+			for _, filePath := range testCase.filePaths {
+				filePaths = append(filePaths, strings.ReplaceAll(filePath, "/", string(os.PathSeparator)))
+			}
+			output := StripFilePaths(input, filePaths...)
+			if output != testCase.expectedOutput {
+				t.Errorf("unexpected output: %s", output)
+			}
+		})
+	}
+}