From f1db8fff419adfc564dfc78b63ea88285d13930d Mon Sep 17 00:00:00 2001 From: Timothy Qiu Date: Tue, 1 Aug 2017 19:31:12 +0800 Subject: [PATCH 1/5] Cleanup: Don't accept optional in init --- ShadowsocksX-NG/ServerProfile.swift | 10 ++++---- ShadowsocksX-NGTests/ServerProfileTests.swift | 24 +++++++------------ 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/ShadowsocksX-NG/ServerProfile.swift b/ShadowsocksX-NG/ServerProfile.swift index dfb1fa0..f5f01f5 100644 --- a/ShadowsocksX-NG/ServerProfile.swift +++ b/ShadowsocksX-NG/ServerProfile.swift @@ -31,7 +31,7 @@ class ServerProfile: NSObject, NSCopying { self.uuid = uuid } - convenience init?(url: URL?) { + convenience init?(url: URL) { self.init() func padBase64(string: String) -> String { @@ -44,14 +44,12 @@ class ServerProfile: NSObject, NSCopying { } } - func decodeUrl(url: URL?) -> String? { - guard let urlStr = url?.absoluteString else { - return nil - } + func decodeUrl(url: URL) -> String? { + let urlStr = url.absoluteString let index = urlStr.index(urlStr.startIndex, offsetBy: 5) let encodedStr = urlStr.substring(from: index) guard let data = Data(base64Encoded: padBase64(string: encodedStr)) else { - return url?.absoluteString + return url.absoluteString } guard let decoded = String(data: data, encoding: String.Encoding.utf8) else { return nil diff --git a/ShadowsocksX-NGTests/ServerProfileTests.swift b/ShadowsocksX-NGTests/ServerProfileTests.swift index d942895..8204f42 100644 --- a/ShadowsocksX-NGTests/ServerProfileTests.swift +++ b/ShadowsocksX-NGTests/ServerProfileTests.swift @@ -31,7 +31,7 @@ class ServerProfileTests: XCTestCase { } func testInitWithSelfGeneratedURL() { - let newProfile = ServerProfile.init(url: profile.URL()) + let newProfile = ServerProfile.init(url: profile.URL()!) XCTAssertEqual(newProfile?.serverHost, profile.serverHost) XCTAssertEqual(newProfile?.serverPort, profile.serverPort) @@ -42,7 +42,7 @@ class ServerProfileTests: XCTestCase { } func testInitWithPlainURL() { - let url = URL(string: "ss://aes-256-cfb:password@example.com:8388") + let url = URL(string: "ss://aes-256-cfb:password@example.com:8388")! let profile = ServerProfile(url: url) @@ -57,7 +57,7 @@ class ServerProfileTests: XCTestCase { } func testInitWithPlainURLandQuery() { - let url = URL(string: "ss://aes-256-cfb:password@example.com:8388?Remark=Prism&OTA=true") + let url = URL(string: "ss://aes-256-cfb:password@example.com:8388?Remark=Prism&OTA=true")! let profile = ServerProfile(url: url) @@ -72,7 +72,7 @@ class ServerProfileTests: XCTestCase { } func testInitWithPlainURLandAnotherQuery() { - let url = URL(string: "ss://aes-256-cfb:password@example.com:8388?Remark=Prism&OTA=0") + let url = URL(string: "ss://aes-256-cfb:password@example.com:8388?Remark=Prism&OTA=0")! let profile = ServerProfile(url: url) @@ -88,7 +88,7 @@ class ServerProfileTests: XCTestCase { func testInitWithBase64EncodedURL() { // "ss://aes-256-cfb:password@example.com:8388" - let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmRAZXhhbXBsZS5jb206ODM4OA") + let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmRAZXhhbXBsZS5jb206ODM4OA")! let profile = ServerProfile(url: url) @@ -104,7 +104,7 @@ class ServerProfileTests: XCTestCase { func testInitWithBase64EncodedURLandQuery() { // "ss://aes-256-cfb:password@example.com:8388?Remark=Prism&OTA=true" - let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmRAZXhhbXBsZS5jb206ODM4OD9SZW1hcms9UHJpc20mT1RBPXRydWU") + let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmRAZXhhbXBsZS5jb206ODM4OD9SZW1hcms9UHJpc20mT1RBPXRydWU")! let profile = ServerProfile(url: url) @@ -119,15 +119,7 @@ class ServerProfileTests: XCTestCase { } func testInitWithEmptyURL() { - let url = URL(string: "ss://") - - let profile = ServerProfile(url: url) - - XCTAssertNil(profile) - } - - func testInitWithInvalidURL() { - let url = URL(string: "ss://invalid url") + let url = URL(string: "ss://")! let profile = ServerProfile(url: url) @@ -136,7 +128,7 @@ class ServerProfileTests: XCTestCase { func testInitWithBase64EncodedInvalidURL() { // "ss://invalid url" - let url = URL(string: "ss://aW52YWxpZCB1cmw") + let url = URL(string: "ss://aW52YWxpZCB1cmw")! let profile = ServerProfile(url: url) From b1e95babaa9da0e29789cce85de6a65f21285b5a Mon Sep 17 00:00:00 2001 From: Timothy Qiu Date: Tue, 1 Aug 2017 19:39:46 +0800 Subject: [PATCH 2/5] Fixes possible crash if url is malicious Try scan a QR code with content `ss://sdf invalid`, an uncaught exception raises. --- ShadowsocksX-NG/Utils.m | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ShadowsocksX-NG/Utils.m b/ShadowsocksX-NG/Utils.m index c77a07a..92912c9 100644 --- a/ShadowsocksX-NG/Utils.m +++ b/ShadowsocksX-NG/Utils.m @@ -56,7 +56,10 @@ void ScanQRCodeOnScreen() { NSLog(@"%@", feature.messageString); if ( [feature.messageString hasPrefix:@"ss://"] ) { - [foundSSUrls addObject:[NSURL URLWithString:feature.messageString]]; + NSURL *url = [NSURL URLWithString:feature.messageString]; + if (url) { + [foundSSUrls addObject:url]; + } } } CGImageRelease(image); From 72c52047f46e9407c46002071b51a6dfdb6bc253 Mon Sep 17 00:00:00 2001 From: Timothy Qiu Date: Tue, 1 Aug 2017 20:02:58 +0800 Subject: [PATCH 3/5] Adds SIP002 URL for QR scaner --- ShadowsocksX-NG/ServerProfile.swift | 29 ++++++++++++++-- ShadowsocksX-NGTests/ServerProfileTests.swift | 34 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/ShadowsocksX-NG/ServerProfile.swift b/ShadowsocksX-NG/ServerProfile.swift index f5f01f5..2fde458 100644 --- a/ShadowsocksX-NG/ServerProfile.swift +++ b/ShadowsocksX-NG/ServerProfile.swift @@ -65,17 +65,40 @@ class ServerProfile: NSObject, NSCopying { return nil } guard let host = parsedUrl.host, let port = parsedUrl.port, - let method = parsedUrl.user, let password = parsedUrl.password else { + let user = parsedUrl.user else { return nil } self.serverHost = host self.serverPort = UInt16(port) - self.method = method.lowercased() - self.password = password + // This can be overriden by the fragment part of SIP002 URL remark = parsedUrl.queryItems? .filter({ $0.name == "Remark" }).first?.value ?? "" + + if let password = parsedUrl.password { + self.method = user.lowercased() + self.password = password + } else { + // SIP002 URL have no password section + guard let data = Data(base64Encoded: padBase64(string: user)), + let userInfo = String(data: data, encoding: .utf8) else { + return nil + } + + let parts = userInfo.characters.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false) + if parts.count != 2 { + return nil + } + self.method = String(parts[0]).lowercased() + self.password = String(parts[1]) + + // SIP002 defines where to put the profile name + if let profileName = parsedUrl.fragment { + self.remark = profileName + } + } + if let otaStr = parsedUrl.queryItems? .filter({ $0.name == "OTA" }).first?.value { ota = NSString(string: otaStr).boolValue diff --git a/ShadowsocksX-NGTests/ServerProfileTests.swift b/ShadowsocksX-NGTests/ServerProfileTests.swift index 8204f42..c3b4d42 100644 --- a/ShadowsocksX-NGTests/ServerProfileTests.swift +++ b/ShadowsocksX-NGTests/ServerProfileTests.swift @@ -135,6 +135,40 @@ class ServerProfileTests: XCTestCase { XCTAssertNil(profile) } + func testInitWithSIP002URL() { + // "ss://aes-256-cfb:password@example.com:8388?Remark=Prism&OTA=true" + let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmQ=@example.com:8388/?Remark=Prism&OTA=true")! + + let profile = ServerProfile(url: url) + + XCTAssertNotNil(profile) + + XCTAssertEqual(profile?.serverHost, "example.com") + XCTAssertEqual(profile?.serverPort, 8388) + XCTAssertEqual(profile?.method, "aes-256-cfb") + XCTAssertEqual(profile?.password, "password") + XCTAssertEqual(profile?.remark, "Prism") + XCTAssertEqual(profile?.ota, true) + } + + func testInitWithSIP002URLProfileName() { + let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmQ=@example.com:8388/#Name")! + + let profile = ServerProfile(url: url) + + XCTAssertNotNil(profile) + XCTAssertEqual(profile?.remark, "Name") + } + + func testInitWithSIP002URLProfileNameOverride() { + let url = URL(string: "ss://YWVzLTI1Ni1jZmI6cGFzc3dvcmQ=@example.com:8388/?Remark=Name#Overriden")! + + let profile = ServerProfile(url: url) + + XCTAssertNotNil(profile) + XCTAssertEqual(profile?.remark, "Overriden") + } + func testPerformanceExample() { // This is an example of a performance test case. self.measure { From e7cc26f81dd2b82aa2e5898d357b785e8d9938bf Mon Sep 17 00:00:00 2001 From: Timothy Qiu Date: Tue, 1 Aug 2017 21:40:09 +0800 Subject: [PATCH 4/5] Generates SIP002 QR code --- ShadowsocksX-NG/AppDelegate.swift | 1 + ShadowsocksX-NG/SWBQRCodeWindowController.h | 1 + ShadowsocksX-NG/SWBQRCodeWindowController.m | 9 ++++++ ShadowsocksX-NG/ServerProfile.swift | 34 ++++++++++++++++++++- 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/ShadowsocksX-NG/AppDelegate.swift b/ShadowsocksX-NG/AppDelegate.swift index 841c235..97883e6 100755 --- a/ShadowsocksX-NG/AppDelegate.swift +++ b/ShadowsocksX-NG/AppDelegate.swift @@ -241,6 +241,7 @@ class AppDelegate: NSObject, NSApplicationDelegate, NSUserNotificationCenterDele } qrcodeWinCtrl = SWBQRCodeWindowController(windowNibName: "SWBQRCodeWindowController") qrcodeWinCtrl.qrCode = profile.URL()!.absoluteString + qrcodeWinCtrl.legacyQRCode = profile.URL(legacy: true)!.absoluteString qrcodeWinCtrl.title = profile.title() qrcodeWinCtrl.showWindow(self) NSApp.activate(ignoringOtherApps: true) diff --git a/ShadowsocksX-NG/SWBQRCodeWindowController.h b/ShadowsocksX-NG/SWBQRCodeWindowController.h index 85a3e9a..ff9b194 100644 --- a/ShadowsocksX-NG/SWBQRCodeWindowController.h +++ b/ShadowsocksX-NG/SWBQRCodeWindowController.h @@ -11,6 +11,7 @@ @interface SWBQRCodeWindowController : NSWindowController +@property (nonatomic, copy) NSString *legacyQRCode; @property (nonatomic, copy) NSString *qrCode; @property (nonatomic, copy) NSString *title; diff --git a/ShadowsocksX-NG/SWBQRCodeWindowController.m b/ShadowsocksX-NG/SWBQRCodeWindowController.m index a7e97e9..e123be0 100644 --- a/ShadowsocksX-NG/SWBQRCodeWindowController.m +++ b/ShadowsocksX-NG/SWBQRCodeWindowController.m @@ -78,4 +78,13 @@ [pasteboard writeObjects:copiedObjects]; } +- (void)flagsChanged:(NSEvent *)event { + NSUInteger modifiers = event.modifierFlags & NSDeviceIndependentModifierFlagsMask; + if (modifiers & NSAlternateKeyMask) { + [self setQRCode:self.legacyQRCode]; + } else { + [self setQRCode:self.qrCode]; + } +} + @end diff --git a/ShadowsocksX-NG/ServerProfile.swift b/ShadowsocksX-NG/ServerProfile.swift index 2fde458..af86a89 100644 --- a/ShadowsocksX-NG/ServerProfile.swift +++ b/ShadowsocksX-NG/ServerProfile.swift @@ -246,7 +246,7 @@ class ServerProfile: NSObject, NSCopying { return true } - func URL() -> Foundation.URL? { + private func makeLegacyURL() -> URL? { var url = URLComponents() url.host = serverHost @@ -275,6 +275,38 @@ class ServerProfile: NSObject, NSCopying { } return nil } + + func URL(legacy: Bool = false) -> URL? { + // If you want the URL from <= 1.5.1 + if (legacy) { + return self.makeLegacyURL() + } + + guard let rawUserInfo = "\(method):\(password)".data(using: .utf8) else { + return nil + } + let paddings = CharacterSet(charactersIn: "=") + let userInfo = rawUserInfo.base64EncodedString().trimmingCharacters(in: paddings) + + var items = [URLQueryItem(name: "OTA", value: ota.description)] + if enabledKcptun { + items.append(URLQueryItem(name: "Kcptun", value: enabledKcptun.description)) + items.append(contentsOf: kcptunProfile.urlQueryItems()) + } + + var comps = URLComponents() + + comps.scheme = "ss" + comps.host = serverHost + comps.port = Int(serverPort) + comps.user = userInfo + comps.fragment = remark + comps.queryItems = items + + let url = try? comps.asURL() + + return url + } func title() -> String { if remark.isEmpty { From f54a108d7ec8acb5b24c332e693682becb4a0e4c Mon Sep 17 00:00:00 2001 From: Timothy Qiu Date: Tue, 1 Aug 2017 23:22:33 +0800 Subject: [PATCH 5/5] Sets path for URL to meet SIP002 requirement --- ShadowsocksX-NG/ServerProfile.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/ShadowsocksX-NG/ServerProfile.swift b/ShadowsocksX-NG/ServerProfile.swift index af86a89..aff8b4e 100644 --- a/ShadowsocksX-NG/ServerProfile.swift +++ b/ShadowsocksX-NG/ServerProfile.swift @@ -300,6 +300,7 @@ class ServerProfile: NSObject, NSCopying { comps.host = serverHost comps.port = Int(serverPort) comps.user = userInfo + comps.path = "/" // This is required by SIP0002 for URLs with fragment or query comps.fragment = remark comps.queryItems = items