Merge pull request #2 from keeshux/remove-deprecated-lzo-framing

Deprecate LZO framing
This commit is contained in:
Davide De Rosa 2018-08-23 23:49:29 +01:00 committed by GitHub
commit 684b3b6c3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 96 additions and 42 deletions

View File

@ -36,6 +36,7 @@ extension ViewController {
builder.cipher = .aes128cbc
builder.digest = .sha1
builder.mtu = 1350
builder.LZOFraming = true
builder.renegotiatesAfterSeconds = nil
builder.shouldDebug = true
builder.debugLogKey = "Log"
@ -76,8 +77,6 @@ class ViewController: UIViewController, URLSessionDataDelegate {
textServer.text = "germany"
textDomain.text = "privateinternetaccess.com"
// textServer.text = "159.122.133.238"
// textDomain.text = ""
textPort.text = "1198"
switchTCP.isOn = false
textUsername.text = "myusername"
@ -121,9 +120,9 @@ class ViewController: UIViewController, URLSessionDataDelegate {
@IBAction func tcpClicked(_ sender: Any) {
if switchTCP.isOn {
textPort.text = "443"
textPort.text = "502"
} else {
textPort.text = "8080"
textPort.text = "1198"
}
}

View File

@ -37,6 +37,7 @@ extension ViewController {
builder.cipher = .aes128cbc
builder.digest = .sha1
builder.mtu = 1350
builder.LZOFraming = true
builder.renegotiatesAfterSeconds = nil
builder.shouldDebug = true
builder.debugLogKey = "Log"
@ -71,10 +72,7 @@ class ViewController: NSViewController {
textServer.stringValue = "germany"
textDomain.stringValue = "privateinternetaccess.com"
// textServer.text = "159.122.133.238"
// textDomain.text = ""
textPort.stringValue = "1198"
// textPort.text = "8080"
textUsername.stringValue = "myusername"
textPassword.stringValue = "mypassword"

View File

@ -23,7 +23,7 @@ The client is known to work with [OpenVPN®][openvpn] 2.3+ servers. Key renegoti
- SHA-256
- [x] TLS CA validation
Today the library does not support compression, yet requires legacy compression framing. You must set the `comp-lzo no` option server-side (deprecated in OpenVPN 2.4) in order to avoid a confusing loss of data packets. Compression behavior will be properly patched in future versions.
The library does not currently support compression, so you must disable it server-side in order to avoid a confusing loss of data packets. The `TunnelKitProvider.Configuration.LZOFraming` option is deprecated and only provided for interoperability with `comp-lzo no`.
## Installation

View File

@ -128,6 +128,10 @@ extension TunnelKitProvider {
/// The MTU of the tunnel.
public var mtu: NSNumber
/// Enables LZO framing (deprecated).
// @available(*, deprecated)
public var LZOFraming: Bool
/// The number of seconds after which a renegotiation is started. Set to `nil` to disable renegotiation (default).
public var renegotiatesAfterSeconds: Int?
@ -158,6 +162,7 @@ extension TunnelKitProvider {
digest = .sha1
ca = nil
mtu = 1500
LZOFraming = false
renegotiatesAfterSeconds = nil
shouldDebug = false
debugLogKey = nil
@ -210,6 +215,7 @@ extension TunnelKitProvider {
self.digest = digest
self.ca = ca
mtu = providerConfiguration[S.mtu] as? NSNumber ?? 1500
LZOFraming = providerConfiguration[S.LZOFraming] as? Bool ?? false
renegotiatesAfterSeconds = providerConfiguration[S.renegotiatesAfter] as? Int
shouldDebug = providerConfiguration[S.debug] as? Bool ?? false
@ -243,6 +249,7 @@ extension TunnelKitProvider {
digest: digest,
ca: ca,
mtu: mtu,
LZOFraming: LZOFraming,
renegotiatesAfterSeconds: renegotiatesAfterSeconds,
shouldDebug: shouldDebug,
debugLogKey: shouldDebug ? debugLogKey : nil,
@ -270,6 +277,8 @@ extension TunnelKitProvider {
static let mtu = "MTU"
static let LZOFraming = "LZOFraming"
static let renegotiatesAfter = "RenegotiatesAfter"
static let debug = "Debug"
@ -303,6 +312,9 @@ extension TunnelKitProvider {
/// - Seealso: `TunnelKitProvider.ConfigurationBuilder.mtu`
public let mtu: NSNumber
/// - Seealso: `TunnelKitProvider.ConfigurationBuilder.LZOFraming`
public let LZOFraming: Bool
/// - Seealso: `TunnelKitProvider.ConfigurationBuilder.renegotiatesAfterSeconds`
public let renegotiatesAfterSeconds: Int?
@ -367,6 +379,9 @@ extension TunnelKitProvider {
if let resolvedAddresses = resolvedAddresses {
dict[S.resolvedAddresses] = resolvedAddresses
}
if LZOFraming {
dict[S.LZOFraming] = LZOFraming
}
if let renegotiatesAfterSeconds = renegotiatesAfterSeconds {
dict[S.renegotiatesAfter] = renegotiatesAfterSeconds
}
@ -421,6 +436,7 @@ extension TunnelKitProvider {
log.info("CA verification: disabled")
}
log.info("MTU: \(mtu)")
log.info("LZO framing: \(LZOFraming ? "enabled" : "disabled")")
if let renegotiatesAfterSeconds = renegotiatesAfterSeconds {
log.info("Renegotiation: \(renegotiatesAfterSeconds) seconds")
} else {
@ -461,6 +477,7 @@ extension TunnelKitProvider.Configuration: Equatable {
(lhs.digest == rhs.digest) &&
(lhs.ca == rhs.ca) &&
(lhs.mtu == rhs.mtu) &&
(lhs.LZOFraming == rhs.LZOFraming) &&
(lhs.renegotiatesAfterSeconds == rhs.renegotiatesAfterSeconds)
)
}

View File

@ -159,6 +159,7 @@ open class TunnelKitProvider: NEPacketTunnelProvider {
sessionConfiguration.cipher = cfg.cipher
sessionConfiguration.digest = cfg.digest
sessionConfiguration.caPath = caPath
sessionConfiguration.LZOFraming = cfg.LZOFraming
if let renegotiatesAfterSeconds = cfg.renegotiatesAfterSeconds {
sessionConfiguration.renegotiatesAfter = Double(renegotiatesAfterSeconds)
}

View File

@ -24,6 +24,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface DataPathCryptoAEAD : NSObject <DataPathEncrypter, DataPathDecrypter>
@property (nonatomic, assign) uint32_t peerId;
@property (nonatomic, assign) BOOL LZOFraming;// DEPRECATED_ATTRIBUTE;
- (instancetype)initWithCrypto:(nonnull CryptoAEAD *)crypto;

View File

@ -227,6 +227,8 @@ const NSInteger CryptoAEADTagLength = 16;
return self;
}
#pragma mark DataPathChannel
- (int)overheadLength
{
return self.crypto.overheadLength;
@ -259,11 +261,13 @@ const NSInteger CryptoAEADTagLength = 16;
#pragma mark DataPathEncrypter
- (void)assembleDataPacketWithPacketId:(uint32_t)packetId compression:(uint8_t)compression payload:(NSData *)payload into:(uint8_t *)dest length:(NSInteger *)length
- (void)assembleDataPacketWithPacketId:(uint32_t)packetId payload:(NSData *)payload into:(uint8_t *)dest length:(NSInteger *)length
{
uint8_t *ptr = dest;
*ptr = compression;
ptr += sizeof(uint8_t);
if (self.LZOFraming) {
*ptr = DataPacketLZONoCompress;
ptr += sizeof(uint8_t);
}
memcpy(ptr, payload.bytes, payload.length);
*length = (int)(ptr - dest + payload.length);
}
@ -329,11 +333,14 @@ const NSInteger CryptoAEADTagLength = 16;
return YES;
}
- (const uint8_t *)parsePayloadWithDataPacket:(const uint8_t *)packet packetLength:(NSInteger)packetLength length:(NSInteger *)length compression:(uint8_t *)compression
- (const uint8_t *)parsePayloadWithDataPacket:(const uint8_t *)packet packetLength:(NSInteger)packetLength length:(NSInteger *)length
{
const uint8_t *ptr = packet;
*compression = *ptr;
ptr += sizeof(uint8_t); // compression byte
if (self.LZOFraming) {
NSAssert(*ptr == DataPacketLZONoCompress, @"Expected LZO NO_COMPRESS");
// *compression = *ptr;
ptr += sizeof(uint8_t); // compression byte
}
*length = packetLength - (int)(ptr - packet);
return ptr;
}

View File

@ -23,6 +23,7 @@ NS_ASSUME_NONNULL_BEGIN
@interface DataPathCryptoCBC : NSObject <DataPathEncrypter, DataPathDecrypter>
@property (nonatomic, assign) uint32_t peerId;
@property (nonatomic, assign) BOOL LZOFraming;// DEPRECATED_ATTRIBUTE;
- (instancetype)initWithCrypto:(nonnull CryptoCBC *)crypto;

View File

@ -229,6 +229,8 @@ const NSInteger CryptoCBCMaxHMACLength = 100;
return self;
}
#pragma mark DataPathChannel
- (int)overheadLength
{
return self.crypto.overheadLength;
@ -257,13 +259,15 @@ const NSInteger CryptoCBCMaxHMACLength = 100;
#pragma mark DataPathEncrypter
- (void)assembleDataPacketWithPacketId:(uint32_t)packetId compression:(uint8_t)compression payload:(NSData *)payload into:(uint8_t *)dest length:(NSInteger *)length
- (void)assembleDataPacketWithPacketId:(uint32_t)packetId payload:(NSData *)payload into:(uint8_t *)dest length:(NSInteger *)length
{
uint8_t *ptr = dest;
*(uint32_t *)ptr = htonl(packetId);
ptr += sizeof(uint32_t);
*ptr = compression;
ptr += sizeof(uint8_t);
if (self.LZOFraming) {
*ptr = DataPacketLZONoCompress;
ptr += sizeof(uint8_t);
}
memcpy(ptr, payload.bytes, payload.length);
*length = (int)(ptr - dest + payload.length);
}
@ -316,12 +320,15 @@ const NSInteger CryptoCBCMaxHMACLength = 100;
return YES;
}
- (const uint8_t *)parsePayloadWithDataPacket:(const uint8_t *)packet packetLength:(NSInteger)packetLength length:(NSInteger *)length compression:(uint8_t *)compression
- (const uint8_t *)parsePayloadWithDataPacket:(const uint8_t *)packet packetLength:(NSInteger)packetLength length:(NSInteger *)length
{
const uint8_t *ptr = packet;
ptr += sizeof(uint32_t); // packet id
*compression = *ptr;
ptr += sizeof(uint8_t); // compression byte
if (self.LZOFraming) {
NSAssert(*ptr == DataPacketLZONoCompress, @"Expected LZO NO_COMPRESS");
// *compression = *ptr;
ptr += sizeof(uint8_t); // compression byte
}
*length = packetLength - (int)(ptr - packet);
return ptr;
}

View File

@ -23,6 +23,7 @@
usesReplayProtection:(BOOL)usesReplayProtection;
- (void)setPeerId:(uint32_t)peerId; // 24-bit, discard most significant byte
- (void)setLZOFraming:(BOOL)LZOFraming;// DEPRECATED_ATTRIBUTE;
- (NSArray<NSData *> *)encryptPackets:(nonnull NSArray<NSData *> *)packets key:(uint8_t)key error:(NSError **)error;
- (NSArray<NSData *> *)decryptPackets:(nonnull NSArray<NSData *> *)packets keepAlive:(nullable bool *)keepAlive error:(NSError **)error;

View File

@ -128,8 +128,17 @@
NSAssert(self.encrypter, @"Setting peer-id to nil encrypter");
NSAssert(self.decrypter, @"Setting peer-id to nil decrypter");
[self.encrypter setPeerId:peerId];
[self.decrypter setPeerId:peerId];
self.encrypter.peerId = peerId;
self.decrypter.peerId = peerId;
}
- (void)setLZOFraming:(BOOL)LZOFraming
{
NSAssert(self.encrypter, @"Setting LZOFraming to nil encrypter");
NSAssert(self.decrypter, @"Setting LZOFraming to nil decrypter");
self.encrypter.LZOFraming = LZOFraming;
self.decrypter.LZOFraming = LZOFraming;
}
#pragma mark DataPath
@ -156,7 +165,6 @@
uint8_t *payload = self.encBufferAligned;
NSInteger payloadLength;
[self.encrypter assembleDataPacketWithPacketId:self.outPacketId
compression:DataPacketCompressNone
payload:raw
into:payload
length:&payloadLength];
@ -211,11 +219,9 @@
}
NSInteger payloadLength;
uint8_t compression;
const uint8_t *payload = [self.decrypter parsePayloadWithDataPacket:packet
packetLength:packetLength
length:&payloadLength
compression:&compression];
length:&payloadLength];
if ((payloadLength == sizeof(DataPacketPingData)) && !memcmp(payload, DataPacketPingData, payloadLength)) {
if (keepAlive) {

View File

@ -8,22 +8,26 @@
#import <Foundation/Foundation.h>
@protocol DataPathEncrypter
@protocol DataPathChannel
- (int)overheadLength;
- (uint32_t)peerId;
- (void)setPeerId:(uint32_t)peerId;
- (void)assembleDataPacketWithPacketId:(uint32_t)packetId compression:(uint8_t)compression payload:(NSData *)payload into:(nonnull uint8_t *)dest length:(nonnull NSInteger *)length;
- (BOOL)LZOFraming;// DEPRECATED_ATTRIBUTE;
- (void)setLZOFraming:(BOOL)LZOFraming;// DEPRECATED_ATTRIBUTE;
@end
@protocol DataPathEncrypter <DataPathChannel>
- (void)assembleDataPacketWithPacketId:(uint32_t)packetId payload:(NSData *)payload into:(nonnull uint8_t *)dest length:(nonnull NSInteger *)length;
- (NSData *)encryptedDataPacketWithKey:(uint8_t)key packetId:(uint32_t)packetId payload:(const uint8_t *)payload payloadLength:(NSInteger)payloadLength error:(NSError **)error;
@end
@protocol DataPathDecrypter
@protocol DataPathDecrypter <DataPathChannel>
- (int)overheadLength;
- (uint32_t)peerId;
- (void)setPeerId:(uint32_t)peerId;
- (BOOL)decryptDataPacket:(nonnull NSData *)packet into:(nonnull uint8_t *)dest length:(nonnull NSInteger *)length packetId:(nonnull uint32_t *)packetId error:(NSError **)error;
- (nonnull const uint8_t *)parsePayloadWithDataPacket:(nonnull const uint8_t *)packet packetLength:(NSInteger)packetLength length:(nonnull NSInteger *)length compression:(nonnull uint8_t *)compression;
- (nonnull const uint8_t *)parsePayloadWithDataPacket:(nonnull const uint8_t *)packet packetLength:(NSInteger)packetLength length:(nonnull NSInteger *)length;
@end

View File

@ -22,7 +22,7 @@ typedef NS_ENUM(uint8_t, PacketCode) {
PacketCodeUnknown = 0xff
};
extern const uint8_t DataPacketCompressNone;
extern const uint8_t DataPacketLZONoCompress;
extern const uint8_t DataPacketPingData[16];
static inline int PacketHeaderSet(uint8_t *_Nonnull to, PacketCode code, uint8_t key)

View File

@ -8,5 +8,5 @@
#import "PacketMacros.h"
const uint8_t DataPacketCompressNone = 0xfa;
const uint8_t DataPacketLZONoCompress = 0xfa;
const uint8_t DataPacketPingData[] = { 0x2a, 0x18, 0x7b, 0xf3, 0x64, 0x1e, 0xb4, 0xcb, 0x07, 0xed, 0x2d, 0x0a, 0x98, 0x1f, 0xc7, 0x48 };

View File

@ -57,6 +57,10 @@ extension SessionProxy {
/// The path to the optional CA for TLS negotiation (PEM format).
public var caPath: String?
/// Enables LZO compression framing (deprecated in OpenVPN 2.4).
// @available(*, deprecated)
public var LZOFraming: Bool
/// Sends periodical keep-alive packets if set.
public var keepAliveInterval: TimeInterval?
@ -71,6 +75,7 @@ extension SessionProxy {
cipher = .aes128cbc
digest = .sha1
caPath = nil
LZOFraming = false
keepAliveInterval = nil
renegotiatesAfter = nil
}
@ -87,6 +92,7 @@ extension SessionProxy {
cipher: cipher,
digest: digest,
caPath: caPath,
LZOFraming: LZOFraming,
keepAliveInterval: keepAliveInterval,
renegotiatesAfter: renegotiatesAfter
)
@ -110,6 +116,9 @@ extension SessionProxy {
/// - Seealso: `SessionProxy.ConfigurationBuilder.caPath`
public let caPath: String?
/// - Seealso: `SessionProxy.ConfigurationBuilder.LZOFraming`
public let LZOFraming: Bool
/// - Seealso: `SessionProxy.ConfigurationBuilder.keepAliveInterval`
public let keepAliveInterval: TimeInterval?

View File

@ -80,8 +80,9 @@ extension SessionProxy {
return isTLSConnected
}
func startHandlingPackets(withPeerId peerId: UInt32? = nil) {
func startHandlingPackets(withPeerId peerId: UInt32? = nil, LZOFraming: Bool = false) {
dataPath?.setPeerId(peerId ?? PacketPeerIdDisabled)
dataPath?.setLZOFraming(LZOFraming)
canHandlePackets = true
}

View File

@ -861,7 +861,10 @@ public class SessionProxy {
}
authenticator = nil
negotiationKey.startHandlingPackets(withPeerId: peerId)
negotiationKey.startHandlingPackets(
withPeerId: peerId,
LZOFraming: configuration.LZOFraming
)
negotiationKey.controlState = .connected
connectedDate = Date()
transitionKeys()

View File

@ -52,27 +52,26 @@ class DataPathEncryptionTests: XCTestCase {
XCTAssertEqual(enc.peerId(), peerId & 0xffffff)
XCTAssertEqual(dec.peerId(), peerId & 0xffffff)
}
// enc.setLZOFraming(true)
// dec.setLZOFraming(true)
let payload = Data(hex: "00112233445566778899")
let packetId: UInt32 = 0x56341200
let key: UInt8 = 4
let compression: UInt8 = DataPacketCompressNone
var encryptedPayload: [UInt8] = [UInt8](repeating: 0, count: 1000)
var encryptedPayloadLength: Int = 0
enc.assembleDataPacket(withPacketId: packetId, compression: compression, payload: payload, into: &encryptedPayload, length: &encryptedPayloadLength)
enc.assembleDataPacket(withPacketId: packetId, payload: payload, into: &encryptedPayload, length: &encryptedPayloadLength)
let encrypted = try! enc.encryptedDataPacket(withKey: key, packetId: packetId, payload: encryptedPayload, payloadLength: encryptedPayloadLength)
var decrypted: [UInt8] = [UInt8](repeating: 0, count: 1000)
var decryptedLength: Int = 0
var decryptedPacketId: UInt32 = 0
var decryptedPayloadLength: Int = 0
var decryptedCompression: UInt8 = 0
try! dec.decryptDataPacket(encrypted, into: &decrypted, length: &decryptedLength, packetId: &decryptedPacketId)
let decryptedPtr = dec.parsePayload(withDataPacket: &decrypted, packetLength: decryptedLength, length: &decryptedPayloadLength, compression: &decryptedCompression)
let decryptedPtr = dec.parsePayload(withDataPacket: &decrypted, packetLength: decryptedLength, length: &decryptedPayloadLength)
let decryptedPayload = Data(bytes: decryptedPtr, count: decryptedPayloadLength)
XCTAssertEqual(payload, decryptedPayload)
XCTAssertEqual(packetId, decryptedPacketId)
XCTAssertEqual(compression, decryptedCompression)
}
}