From 9a6f3d638c7378e1d7510789fba8c5c0da0e475f Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 19 Mar 2019 23:08:38 +0100 Subject: [PATCH 1/6] Recognize "--compress lzo" option as legal --- TunnelKit/Sources/Core/ConfigurationParser.swift | 15 +++++++++++---- .../Sources/Core/SessionProxy+PushReply.swift | 14 ++++++++------ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/TunnelKit/Sources/Core/ConfigurationParser.swift b/TunnelKit/Sources/Core/ConfigurationParser.swift index 0a3ef19..0790ca1 100644 --- a/TunnelKit/Sources/Core/ConfigurationParser.swift +++ b/TunnelKit/Sources/Core/ConfigurationParser.swift @@ -315,10 +315,17 @@ public class ConfigurationParser { isHandled = true compressionFraming = .compress - guard $0.isEmpty else { - compressionAlgorithm = .other - unsupportedError = .unsupportedConfiguration(option: line) - return + if !LZOIsSupported() { + guard $0.isEmpty else { + unsupportedError = .unsupportedConfiguration(option: line) + return + } + } else { + if let arg = $0.first { + compressionAlgorithm = (arg == "lzo") ? .LZO : .other + } else { + compressionAlgorithm = .disabled + } } } Regex.keyDirection.enumerateArguments(in: line) { diff --git a/TunnelKit/Sources/Core/SessionProxy+PushReply.swift b/TunnelKit/Sources/Core/SessionProxy+PushReply.swift index 8d4baa0..af2282d 100644 --- a/TunnelKit/Sources/Core/SessionProxy+PushReply.swift +++ b/TunnelKit/Sources/Core/SessionProxy+PushReply.swift @@ -395,18 +395,20 @@ extension SessionProxy { switch $0[0] { case "comp-lzo": compressionFraming = .compLZO - if !(($0.count == 2) && ($0[1] == "no")) { - compressionAlgorithm = .LZO - } else { + if ($0.count == 2) && ($0[1] == "no") { compressionAlgorithm = .disabled + } else { + compressionAlgorithm = .LZO } case "compress": compressionFraming = .compress - if $0.count > 1 { - compressionAlgorithm = .other - } else { + if $0.count == 1 { compressionAlgorithm = .disabled + } else if ($0.count == 2) && ($0[1] == "lzo") { + compressionAlgorithm = .LZO + } else { + compressionAlgorithm = .other } default: From 4b9ffcfb4e8cd43419a657eb5f6a7193728354ed Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 19 Mar 2019 23:17:04 +0100 Subject: [PATCH 2/6] Accept LZO regardless of framing --- TunnelKit/Sources/Core/DataPath.m | 2 +- TunnelKit/Sources/Core/SessionProxy.swift | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/TunnelKit/Sources/Core/DataPath.m b/TunnelKit/Sources/Core/DataPath.m index 2af19c9..5f64e7c 100644 --- a/TunnelKit/Sources/Core/DataPath.m +++ b/TunnelKit/Sources/Core/DataPath.m @@ -117,7 +117,7 @@ [self.decrypter setPeerId:peerId]; [self setCompressionFraming:compressionFraming]; - if (LZOIsSupported() && (compressionFraming == CompressionFramingNativeCompLZO) && (compressionAlgorithm == CompressionAlgorithmNativeLZO)) { + if (LZOIsSupported() && (compressionAlgorithm == CompressionAlgorithmNativeLZO)) { self.lzo = LZOCreate(); } } diff --git a/TunnelKit/Sources/Core/SessionProxy.swift b/TunnelKit/Sources/Core/SessionProxy.swift index 59358ee..366daa0 100644 --- a/TunnelKit/Sources/Core/SessionProxy.swift +++ b/TunnelKit/Sources/Core/SessionProxy.swift @@ -912,20 +912,20 @@ public class SessionProxy { reply = optionalReply log.debug("Received PUSH_REPLY: \"\(reply.maskedDescription)\"") - if let framing = reply.compressionFraming, let compression = reply.compressionAlgorithm, compression != .disabled { - switch framing { - case .compress: - log.error("Server has new compression enabled and this is currently unsupported (\(framing))") - throw SessionError.serverCompression + if let framing = reply.compressionFraming, let compression = reply.compressionAlgorithm { + switch compression { + case .disabled: + break - case .compLZO: + case .LZO: if !LZOIsSupported() { - log.error("Server has legacy LZO compression enabled and this was not built into the library (\(framing))") + log.error("Server has LZO compression enabled and this was not built into the library (framing=\(framing))") throw SessionError.serverCompression } - default: - break + case .other: + log.error("Server has non-LZO compression enabled and this is currently unsupported (framing=\(framing))") + throw SessionError.serverCompression } } } catch let e { From 9d479a9aba05bef815b4a20b193ef5a01c88d209 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 19 Mar 2019 23:19:16 +0100 Subject: [PATCH 3/6] Handle LZO compression in --compress framing Share parse block between comp-lzo and compress. It seems that --compress sends NO_COMPRESS w/o swapping. Also suppress redundant LZOIsSupported(), implied by non-nil value of self.lzo. --- TunnelKit/Sources/Core/DataPath.m | 98 ++++++++++++++++++------------- 1 file changed, 56 insertions(+), 42 deletions(-) diff --git a/TunnelKit/Sources/Core/DataPath.m b/TunnelKit/Sources/Core/DataPath.m index 5f64e7c..51063d7 100644 --- a/TunnelKit/Sources/Core/DataPath.m +++ b/TunnelKit/Sources/Core/DataPath.m @@ -168,6 +168,42 @@ - (void)setCompressionFraming:(CompressionFramingNative)compressionFraming { + __weak DataPath *weakSelf = self; + + DataPathParseBlock parseCompressedBlock = ^BOOL(uint8_t * _Nonnull payload, NSInteger * _Nonnull payloadOffset, uint8_t * _Nonnull compressionHeader, NSInteger * _Nonnull headerLength, const uint8_t * _Nonnull packet, NSInteger packetLength, NSError * _Nullable __autoreleasing * _Nullable error) { + *compressionHeader = payload[0]; + *headerLength = 1; + + switch (*compressionHeader) { + case DataPacketNoCompress: + *payloadOffset = 1; + break; + + case DataPacketNoCompressSwap: + payload[0] = packet[packetLength - 1]; + *payloadOffset = 0; + break; + + case DataPacketLZOCompress: + if (!weakSelf.lzo) { // compressed packet unexpected + if (error) { + *error = TunnelKitErrorWithCode(TunnelKitErrorCodeDataPathCompression); + } + return NO; + } + *payloadOffset = 1; + break; + + default: + // @"Expected NO_COMPRESS (found %X != %X)", payload[0], DataPacketNoCompress); + if (error) { + *error = TunnelKitErrorWithCode(TunnelKitErrorCodeDataPathCompression); + } + return NO; + } + return YES; + }; + switch (compressionFraming) { case CompressionFramingNativeDisabled: { self.assemblePayloadBlock = ^(uint8_t * packetDest, NSInteger * packetLengthOffset, NSData * payload) { @@ -184,27 +220,30 @@ } case CompressionFramingNativeCompress: { self.assemblePayloadBlock = ^(uint8_t * packetDest, NSInteger * packetLengthOffset, NSData * payload) { - memcpy(packetDest, payload.bytes, payload.length); - packetDest[payload.length] = packetDest[0]; - packetDest[0] = DataPacketNoCompressSwap; - *packetLengthOffset = 1; - }; - self.parsePayloadBlock = ^BOOL(uint8_t * _Nonnull payload, NSInteger * _Nonnull payloadOffset, uint8_t * _Nonnull compressionHeader, NSInteger * _Nonnull headerLength, const uint8_t * _Nonnull packet, NSInteger packetLength, NSError * _Nullable __autoreleasing * _Nullable error) { - *compressionHeader = payload[0]; - if (*compressionHeader != DataPacketNoCompressSwap) { - // @"Expected NO_COMPRESS_SWAP (found %X != %X)", payload[0], DataPacketNoCompressSwap); - *error = TunnelKitErrorWithCode(TunnelKitErrorCodeDataPathCompression); - return NO; + NSData *compressedPayload = [weakSelf.lzo compressedDataWithData:payload error:NULL]; + if (compressedPayload) { + packetDest[0] = DataPacketLZOCompress; + *packetLengthOffset = 1 - (payload.length - compressedPayload.length); + payload = compressedPayload; + memcpy(packetDest + 1, payload.bytes, payload.length); + } else { + *packetLengthOffset = 1; + + // do not byte swap if compression enabled + if (weakSelf.lzo) { + packetDest[0] = DataPacketNoCompress; + memcpy(packetDest + 1, payload.bytes, payload.length); + } else { + memcpy(packetDest, payload.bytes, payload.length); + packetDest[payload.length] = packetDest[0]; + packetDest[0] = DataPacketNoCompressSwap; + } } - payload[0] = packet[packetLength - 1]; - *payloadOffset = 0; - *headerLength = 1; - return YES; }; + self.parsePayloadBlock = parseCompressedBlock; break; } case CompressionFramingNativeCompLZO: { - __weak DataPath *weakSelf = self; self.assemblePayloadBlock = ^(uint8_t * packetDest, NSInteger * packetLengthOffset, NSData * payload) { NSData *compressedPayload = [weakSelf.lzo compressedDataWithData:payload error:NULL]; if (compressedPayload) { @@ -217,32 +256,7 @@ } memcpy(packetDest + 1, payload.bytes, payload.length); }; - self.parsePayloadBlock = ^BOOL(uint8_t * _Nonnull payload, NSInteger * _Nonnull payloadOffset, uint8_t * _Nonnull compressionHeader, NSInteger * _Nonnull headerLength, const uint8_t * _Nonnull packet, NSInteger packetLength, NSError * _Nullable __autoreleasing * _Nullable error) { - *compressionHeader = payload[0]; - switch (*compressionHeader) { - case DataPacketNoCompress: - break; - - case DataPacketLZOCompress: - if (!LZOIsSupported() || !weakSelf.lzo) { // compressed packet unexpected - if (error) { - *error = TunnelKitErrorWithCode(TunnelKitErrorCodeDataPathCompression); - } - return NO; - } - break; - - default: - // @"Expected NO_COMPRESS (found %X != %X)", payload[0], DataPacketNoCompress); - if (error) { - *error = TunnelKitErrorWithCode(TunnelKitErrorCodeDataPathCompression); - } - return NO; - } - *payloadOffset = 1; - *headerLength = 1; - return YES; - }; + self.parsePayloadBlock = parseCompressedBlock; break; } } From 40458ebf5f08816fc108f724221dd2200065603f Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 19 Mar 2019 23:55:24 +0100 Subject: [PATCH 4/6] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9f64ee..576cbef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Support for legacy `--comp-lzo` compression. [#69](https://github.com/keeshux/tunnelkit/pull/69) +- Support for newer `--compress lzo` option. [#70](https://github.com/keeshux/tunnelkit/pull/70) ## 1.4.3 (2019-03-18) From 2f17e30fb9f2bba478c927a59338a96ae9891e13 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Wed, 20 Mar 2019 09:13:01 +0100 Subject: [PATCH 5/6] Exclude testmini.c from LZO spec --- TunnelKit.podspec | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TunnelKit.podspec b/TunnelKit.podspec index e08b871..43dd818 100644 --- a/TunnelKit.podspec +++ b/TunnelKit.podspec @@ -37,9 +37,9 @@ Pod::Spec.new do |s| s.subspec "LZO" do |p| p.source_files = "TunnelKit/Sources/Core/LZO.h", "TunnelKit/Sources/Core/Errors.{h,m}", - "TunnelKit/Sources/LZO/**/*.{h,m,c}" + "TunnelKit/Sources/LZO/**/*lzo*.{h,m,c}" p.private_header_files = "TunnelKit/Sources/Core/LZO.h", - "TunnelKit/Sources/LZO/lib/*.h" + "TunnelKit/Sources/LZO/lib/*lzo*.h" p.pod_target_xcconfig = { "APPLICATION_EXTENSION_API_ONLY" => "YES" } end end From 0cd5b2f6af001e19f0900236ffe08f360f7163a9 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Wed, 20 Mar 2019 16:39:02 +0100 Subject: [PATCH 6/6] Update README with full LZO support --- README.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index adff863..edb9559 100644 --- a/README.md +++ b/README.md @@ -29,11 +29,10 @@ The client is known to work with [OpenVPN®][openvpn] 2.3+ servers. - Authentication (`--tls-auth`) - Encryption (`--tls-crypt`) - [x] Compression framing - - Disabled - - Compress (2.4) - - LZO (deprecated in 2.4) + - Via `--comp-lzo` (deprecated in 2.4) + - Via `--compress` - [x] Compression algorithms - - LZO (`--comp-lzo` only) + - LZO (via `--comp-lzo` or `--compress lzo`) - [x] Key renegotiation - [x] Replay protection (hardcoded window) @@ -46,8 +45,7 @@ TunnelKit can parse .ovpn configuration files. Below are a few limitations worth Unsupported: - UDP fragmentation, i.e. `--fragment` -- Compression - - `--compress` other than empty +- Compression via `--compress` other than empty or `lzo` - Proxy - External file references (inline `` only) - Encrypted client certificate keys