sync: enforce HTTP for loopback addresses only (#921)

Fixes SNT-386

Author: sharvilshah

Source/common/SNTConfigurator.h

@@ -704,6 +704,12 @@ extern NSString* _Nonnull const kEnableMenuItemUserOverride;
 ///
 @property(nullable, readonly, nonatomic) NSURL* syncBaseURL;
 
+///
+///  Whether a SyncBaseURL value exists in the configuration, without any parsing or validation.
+///  Use this to distinguish "no URL configured" from "URL configured but rejected by policy."
+///
+@property(readonly, nonatomic) BOOL syncBaseURLConfigured;
+
 ///
 ///  If enabled, syncing will use binary protobufs for transfer instead
 ///  of JSON. Defaults to NO.

Source/common/SNTConfigurator.mm

@@ -566,6 +566,10 @@ + (NSSet*)keyPathsForValuesAffectingSyncBaseURL {
   return [self configStateSet];
 }
 
++ (NSSet*)keyPathsForValuesAffectingSyncBaseURLConfigured {
+  return [self configStateSet];
+}
+
 + (NSSet*)keyPathsForValuesAffectingSyncEnableProtoTransfer {
   return [self configStateSet];
 }
@@ -1124,9 +1128,27 @@ - (NSURL*)syncBaseURL {
     urlString = [urlString stringByAppendingString:@"/"];
   }
   NSURL* url = [NSURL URLWithString:urlString];
+
+  // Only allow HTTP for loopback addresses. Everything else must use HTTPS.
+  if ([[url.scheme lowercaseString] isEqualToString:@"http"]) {
+    NSString* host = [url.host lowercaseString];
+    if (![host isEqualToString:@"localhost"] && ![host isEqualToString:@"127.0.0.1"] &&
+        ![host isEqualToString:@"::1"]) {
+      LOGW(@"Rejecting non-localhost HTTP SyncBaseURL: %@", urlString);
+      return nil;
+    }
+  }
+
   return url;
 }
 
+// Raw check: returns YES if a SyncBaseURL string is present in the config, without any
+// URL parsing or validation. This lets callers distinguish "not configured" from
+// "configured but rejected by syncBaseURL policy" (e.g. non-localhost HTTP).
+- (BOOL)syncBaseURLConfigured {
+  return [self.configState[kSyncBaseURLKey] length] > 0;
+}
+
 - (BOOL)syncEnableProtoTransfer {
   NSNumber* number = self.configState[kSyncEnableProtoTransfer];
   return number ? [number boolValue] : NO;

Source/common/SNTConfiguratorTest.mm

@@ -165,6 +165,64 @@ - (void)testInitMigratesRemovableMediaSyncStateKeys {
                               }];
 }
 
+- (void)testSyncBaseURLRejectsNonLocalhostHTTP {
+  SNTConfigurator* sut = [[SNTConfigurator alloc] init];
+
+  // HTTPS is always allowed.
+  sut.configState[@"SyncBaseURL"] = @"https://example.com/api";
+  XCTAssertNotNil(sut.syncBaseURL);
+  XCTAssertEqualObjects(sut.syncBaseURL.host, @"example.com");
+
+  // HTTP to localhost is allowed.
+  sut.configState[@"SyncBaseURL"] = @"http://localhost:8080/api";
+  XCTAssertNotNil(sut.syncBaseURL);
+  XCTAssertEqualObjects(sut.syncBaseURL.host, @"localhost");
+
+  // HTTP to 127.0.0.1 is allowed.
+  sut.configState[@"SyncBaseURL"] = @"http://127.0.0.1:8080/api";
+  XCTAssertNotNil(sut.syncBaseURL);
+  XCTAssertEqualObjects(sut.syncBaseURL.host, @"127.0.0.1");
+
+  // HTTP to ::1 is allowed.
+  sut.configState[@"SyncBaseURL"] = @"http://[::1]:8080/api";
+  XCTAssertNotNil(sut.syncBaseURL);
+  XCTAssertEqualObjects(sut.syncBaseURL.host, @"::1");
+
+  // HTTP to a non-localhost host is rejected.
+  sut.configState[@"SyncBaseURL"] = @"http://example.com/api";
+  XCTAssertNil(sut.syncBaseURL);
+
+  sut.configState[@"SyncBaseURL"] = @"http://10.0.0.1/api";
+  XCTAssertNil(sut.syncBaseURL);
+
+  // Empty and missing values return nil.
+  sut.configState[@"SyncBaseURL"] = @"";
+  XCTAssertNil(sut.syncBaseURL);
+
+  sut.configState[@"SyncBaseURL"] = nil;
+  XCTAssertNil(sut.syncBaseURL);
+}
+
+- (void)testSyncBaseURLConfigured {
+  SNTConfigurator* sut = [[SNTConfigurator alloc] init];
+
+  // A value is configured, even if syncBaseURL rejects it.
+  sut.configState[@"SyncBaseURL"] = @"http://example.com/api";
+  XCTAssertNil(sut.syncBaseURL);
+  XCTAssertTrue(sut.syncBaseURLConfigured);
+
+  // A valid value is also reported as configured.
+  sut.configState[@"SyncBaseURL"] = @"https://example.com/api";
+  XCTAssertTrue(sut.syncBaseURLConfigured);
+
+  // Empty and missing values are not configured.
+  sut.configState[@"SyncBaseURL"] = @"";
+  XCTAssertFalse(sut.syncBaseURLConfigured);
+
+  sut.configState[@"SyncBaseURL"] = nil;
+  XCTAssertFalse(sut.syncBaseURLConfigured);
+}
+
 - (void)testTelemetryFilterExpressions {
   SNTConfigurator* sut = [[SNTConfigurator alloc] init];
 

Source/santactl/Commands/SNTCommandDoctor.mm

@@ -199,10 +199,16 @@ - (BOOL)validateSync {
 
   NSURL* syncBaseURL = config.syncBaseURL;
   if (!syncBaseURL) {
+    if (config.syncBaseURLConfigured) {
+      print(@"[-] SyncBaseURL is configured but was rejected "
+            @"(HTTP is only allowed for localhost)");
+      print(@"");
+      return YES;
+    }
     print(@"[+] Sync is disabled");
     print(@"");
     // Don't treat this as an error.
-    return YES;
+    return NO;
   }
   print(@"[+] Sync is enabled");
   print(@"[+] Machine ID: %s", config.machineID.UTF8String ?: "(not set)");

docs/src/lib/santaconfig.ts

@@ -996,7 +996,7 @@ thousand static rules working correctly, but we don't recommend using StaticRule
   sync: [
     {
       key: "SyncBaseURL",
-      description: `The base URL of the sync server`,
+      description: `The base URL of the sync server. HTTPS is required unless the host is localhost (localhost, 127.0.0.1, or ::1).`,
       type: "string",
     },
     {