From 2fde43b1fc85485ab50386837c3270a76d894e5b Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 6 Nov 2018 10:25:35 +0100 Subject: [PATCH 1/4] Keep tag length constants private Also AD length in AEAD was an unresolved relic. --- TunnelKit/Sources/Core/CryptoAEAD.m | 2 +- TunnelKit/Sources/Core/CryptoCTR.h | 2 -- TunnelKit/Sources/Core/CryptoCTR.m | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/TunnelKit/Sources/Core/CryptoAEAD.m b/TunnelKit/Sources/Core/CryptoAEAD.m index d5f4e06..7fc9c2a 100644 --- a/TunnelKit/Sources/Core/CryptoAEAD.m +++ b/TunnelKit/Sources/Core/CryptoAEAD.m @@ -43,7 +43,7 @@ #import "Allocation.h" #import "Errors.h" -const NSInteger CryptoAEADTagLength = 16; +static const NSInteger CryptoAEADTagLength = 16; @interface CryptoAEAD () diff --git a/TunnelKit/Sources/Core/CryptoCTR.h b/TunnelKit/Sources/Core/CryptoCTR.h index a627078..451124f 100644 --- a/TunnelKit/Sources/Core/CryptoCTR.h +++ b/TunnelKit/Sources/Core/CryptoCTR.h @@ -30,8 +30,6 @@ NS_ASSUME_NONNULL_BEGIN -extern const NSInteger CryptoCTRADLength; - @interface CryptoCTR : NSObject - (instancetype)initWithCipherName:(nullable NSString *)cipherName digestName:(NSString *)digestName; diff --git a/TunnelKit/Sources/Core/CryptoCTR.m b/TunnelKit/Sources/Core/CryptoCTR.m index d91ef42..5832fce 100644 --- a/TunnelKit/Sources/Core/CryptoCTR.m +++ b/TunnelKit/Sources/Core/CryptoCTR.m @@ -33,7 +33,7 @@ #import "Allocation.h" #import "Errors.h" -const NSInteger CryptoCTRTagLength = 32; +static const NSInteger CryptoCTRTagLength = 32; @interface CryptoCTR () From 7ffbf41b3095a3f7725326ebde77664ca39d9776 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 6 Nov 2018 10:30:43 +0100 Subject: [PATCH 2/4] Expose internal tag length, 0 if none --- TunnelKit/Sources/Core/Crypto.h | 2 ++ TunnelKit/Sources/Core/CryptoAEAD.m | 5 +++++ TunnelKit/Sources/Core/CryptoBox.h | 1 + TunnelKit/Sources/Core/CryptoBox.m | 2 ++ TunnelKit/Sources/Core/CryptoCBC.m | 5 +++++ TunnelKit/Sources/Core/CryptoCTR.m | 5 +++++ 6 files changed, 20 insertions(+) diff --git a/TunnelKit/Sources/Core/Crypto.h b/TunnelKit/Sources/Core/Crypto.h index 109758c..4fb29a2 100644 --- a/TunnelKit/Sources/Core/Crypto.h +++ b/TunnelKit/Sources/Core/Crypto.h @@ -55,6 +55,7 @@ typedef struct { - (void)configureEncryptionWithCipherKey:(nullable ZeroingData *)cipherKey hmacKey:(nullable ZeroingData *)hmacKey; - (int)digestLength; +- (int)tagLength; - (NSInteger)encryptionCapacityWithLength:(NSInteger)length; - (BOOL)encryptBytes:(const uint8_t *)bytes length:(NSInteger)length dest:(uint8_t *)dest destLength:(NSInteger *)destLength flags:(const CryptoFlags *_Nullable)flags error:(NSError **)error; @@ -68,6 +69,7 @@ typedef struct { - (void)configureDecryptionWithCipherKey:(nullable ZeroingData *)cipherKey hmacKey:(nullable ZeroingData *)hmacKey; - (int)digestLength; +- (int)tagLength; - (NSInteger)encryptionCapacityWithLength:(NSInteger)length; - (BOOL)decryptBytes:(const uint8_t *)bytes length:(NSInteger)length dest:(uint8_t *)dest destLength:(NSInteger *)destLength flags:(const CryptoFlags *_Nullable)flags error:(NSError **)error; diff --git a/TunnelKit/Sources/Core/CryptoAEAD.m b/TunnelKit/Sources/Core/CryptoAEAD.m index 7fc9c2a..a2b77f5 100644 --- a/TunnelKit/Sources/Core/CryptoAEAD.m +++ b/TunnelKit/Sources/Core/CryptoAEAD.m @@ -97,6 +97,11 @@ static const NSInteger CryptoAEADTagLength = 16; return 0; } +- (int)tagLength +{ + return CryptoAEADTagLength; +} + - (NSInteger)encryptionCapacityWithLength:(NSInteger)length { return safe_crypto_capacity(length, CryptoAEADTagLength); diff --git a/TunnelKit/Sources/Core/CryptoBox.h b/TunnelKit/Sources/Core/CryptoBox.h index 037aa65..89dbf1f 100644 --- a/TunnelKit/Sources/Core/CryptoBox.h +++ b/TunnelKit/Sources/Core/CryptoBox.h @@ -73,6 +73,7 @@ NS_ASSUME_NONNULL_BEGIN - (id)decrypter; - (NSInteger)digestLength; +- (NSInteger)tagLength; @end diff --git a/TunnelKit/Sources/Core/CryptoBox.m b/TunnelKit/Sources/Core/CryptoBox.m index bbdd825..5a17557 100644 --- a/TunnelKit/Sources/Core/CryptoBox.m +++ b/TunnelKit/Sources/Core/CryptoBox.m @@ -52,6 +52,7 @@ @property (nonatomic, strong) NSString *cipherAlgorithm; @property (nonatomic, strong) NSString *digestAlgorithm; @property (nonatomic, assign) NSInteger digestLength; +@property (nonatomic, assign) NSInteger tagLength; @property (nonatomic, strong) id encrypter; @property (nonatomic, strong) id decrypter; @@ -147,6 +148,7 @@ NSAssert(self.encrypter.digestLength == self.decrypter.digestLength, @"Digest length mismatch in encrypter/decrypter"); self.digestLength = self.encrypter.digestLength; + self.tagLength = self.encrypter.tagLength; return YES; } diff --git a/TunnelKit/Sources/Core/CryptoCBC.m b/TunnelKit/Sources/Core/CryptoCBC.m index b9fc3b9..d6e5acb 100644 --- a/TunnelKit/Sources/Core/CryptoCBC.m +++ b/TunnelKit/Sources/Core/CryptoCBC.m @@ -113,6 +113,11 @@ const NSInteger CryptoCBCMaxHMACLength = 100; self.digest = NULL; } +- (int)tagLength +{ + return 0; +} + - (NSInteger)encryptionCapacityWithLength:(NSInteger)length { return safe_crypto_capacity(length, self.digestLength + self.cipherIVLength); diff --git a/TunnelKit/Sources/Core/CryptoCTR.m b/TunnelKit/Sources/Core/CryptoCTR.m index 5832fce..5181f63 100644 --- a/TunnelKit/Sources/Core/CryptoCTR.m +++ b/TunnelKit/Sources/Core/CryptoCTR.m @@ -95,6 +95,11 @@ static const NSInteger CryptoCTRTagLength = 32; self.digest = NULL; } +- (int)tagLength +{ + return CryptoCTRTagLength; +} + - (NSInteger)encryptionCapacityWithLength:(NSInteger)length { return safe_crypto_capacity(length, PacketOpcodeLength + PacketSessionIdLength + PacketReplayIdLength + PacketReplayTimestampLength + CryptoCTRTagLength); From b3669251259cf3f2ad8147b4c67db725a206d84f Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 6 Nov 2018 10:33:55 +0100 Subject: [PATCH 3/4] Hardcode digestLength to tagLength in CTR Code is not using digestLength in any way. --- TunnelKit/Sources/Core/CryptoCTR.m | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/TunnelKit/Sources/Core/CryptoCTR.m b/TunnelKit/Sources/Core/CryptoCTR.m index 5181f63..d2728fe 100644 --- a/TunnelKit/Sources/Core/CryptoCTR.m +++ b/TunnelKit/Sources/Core/CryptoCTR.m @@ -42,7 +42,6 @@ static const NSInteger CryptoCTRTagLength = 32; @property (nonatomic, assign) int cipherKeyLength; @property (nonatomic, assign) int cipherIVLength; @property (nonatomic, assign) int hmacKeyLength; -@property (nonatomic, assign) int digestLength; @property (nonatomic, unsafe_unretained) EVP_CIPHER_CTX *cipherCtxEnc; @property (nonatomic, unsafe_unretained) EVP_CIPHER_CTX *cipherCtxDec; @@ -70,14 +69,13 @@ static const NSInteger CryptoCTRTagLength = 32; self.cipherIVLength = EVP_CIPHER_iv_length(self.cipher); // as seen in OpenVPN's crypto_openssl.c:md_kt_size() self.hmacKeyLength = EVP_MD_size(self.digest); - self.digestLength = EVP_MD_size(self.digest); - NSAssert(self.digestLength == CryptoCTRTagLength, @"Expected digest size to be tag length (%ld)", CryptoCTRTagLength); + NSAssert(EVP_MD_size(self.digest) == CryptoCTRTagLength, @"Expected digest size to be tag length (%ld)", CryptoCTRTagLength); self.cipherCtxEnc = EVP_CIPHER_CTX_new(); self.cipherCtxDec = EVP_CIPHER_CTX_new(); self.hmacCtxEnc = HMAC_CTX_new(); self.hmacCtxDec = HMAC_CTX_new(); - self.bufferDecHMAC = allocate_safely(self.digestLength); + self.bufferDecHMAC = allocate_safely(CryptoCTRTagLength); } return self; } @@ -88,13 +86,18 @@ static const NSInteger CryptoCTRTagLength = 32; EVP_CIPHER_CTX_free(self.cipherCtxDec); HMAC_CTX_free(self.hmacCtxEnc); HMAC_CTX_free(self.hmacCtxDec); - bzero(self.bufferDecHMAC, self.digestLength); + bzero(self.bufferDecHMAC, CryptoCTRTagLength); free(self.bufferDecHMAC); self.cipher = NULL; self.digest = NULL; } +- (int)digestLength +{ + return CryptoCTRTagLength; +} + - (int)tagLength { return CryptoCTRTagLength; From 36e93651bae56a4abd617d7bf69e4148e81c212c Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 6 Nov 2018 10:31:03 +0100 Subject: [PATCH 4/4] Replace hardcoded 32 tag length in tls-crypt --- TunnelKit/Sources/Core/ControlChannelSerializer.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TunnelKit/Sources/Core/ControlChannelSerializer.swift b/TunnelKit/Sources/Core/ControlChannelSerializer.swift index 4714820..8e9f541 100644 --- a/TunnelKit/Sources/Core/ControlChannelSerializer.swift +++ b/TunnelKit/Sources/Core/ControlChannelSerializer.swift @@ -233,7 +233,7 @@ extension ControlChannel { headerLength = PacketOpcodeLength + PacketSessionIdLength adLength = headerLength + PacketReplayIdLength + PacketReplayTimestampLength - tagLength = 32 + tagLength = crypto.tagLength() currentReplayId = BidirectionalState(withResetValue: 1) plain = PlainSerializer()