Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Added deallocate functions #36

Merged
merged 11 commits into from
Feb 27, 2019
Merged

fix: Added deallocate functions #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
85b0483
rebased to master
Andrew-Lees11 Feb 25, 2019
750c033
Add key frees
Andrew-Lees11 Feb 25, 2019
547d178
Added RSAKey free
Andrew-Lees11 Feb 25, 2019
4416061
Added rsa Free
Andrew-Lees11 Feb 25, 2019
e5d5fd8
Use evp for native key
Andrew-Lees11 Feb 25, 2019
0c73af2
use make optional for evp_key
Andrew-Lees11 Feb 26, 2019
5d4b0b7
Copy EVP key for encryption
Andrew-Lees11 Feb 26, 2019
1d3f918
Added ctx cleanup
Andrew-Lees11 Feb 26, 2019
ac329da
remove padding ctx
Andrew-Lees11 Feb 26, 2019
7248cd9
fix whitespace changes
Andrew-Lees11 Feb 26, 2019
fd6adcd
Remove remaining whitespace changes
Andrew-Lees11 Feb 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 74 additions & 69 deletions Sources/CryptorRSA/CryptorRSA.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,18 @@ public class CryptorRSA {
let rsaEncryptCtx = EVP_CIPHER_CTX_new_wrapper()
EVP_CIPHER_CTX_init_wrapper(rsaEncryptCtx)

// get rsaKey
guard let rsaKey = EVP_PKEY_get1_RSA(.make(optional: key.reference)) else {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {
throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
}
defer {
RSA_free(rsaKey)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This additional block exists now because we are storing the envelope key rather than the RSA key.

Changing the key to store the envelope key rather than the RSA key it contains is beneficial because the sign, verify and CBC decrypt functions no longer need to construct a new one (and copy the RSA key in), which should shorten their pathlength. (caveat: we did not benchmark this)

// Set the additional authenticated data (aad) as the RSA key modulus and publicExponent in an ASN1 sequence.
guard let aad = key.publicKeyBytes else {
let source = "Encryption failed"
Expand Down Expand Up @@ -368,7 +380,7 @@ public class CryptorRSA {
// Set the aeskey and iv for the symmetric encryption.
EVP_EncryptInit_ex(rsaEncryptCtx, nil, nil, aeskey, iv) == 1,
// Encrypt the aes key using the rsa public key with SHA1, OAEP padding.
RSA_public_encrypt(Int32(keySize), aeskey, encryptedKey, .make(optional: key.reference), RSA_PKCS1_OAEP_PADDING) == encryptedCapacity,
RSA_public_encrypt(Int32(keySize), aeskey, encryptedKey, .make(optional: rsaKey), RSA_PKCS1_OAEP_PADDING) == encryptedCapacity,
// Add the aad to the encryption context.
// This is used in generating the GCM tag. We don't use this processedLength.
EVP_EncryptUpdate(rsaEncryptCtx, nil, &processedLength, [UInt8](aad), Int32(aad.count)) == 1
Expand Down Expand Up @@ -417,16 +429,13 @@ public class CryptorRSA {
}

func encryptedCBC(with key: PublicKey) throws -> EncryptedData? {
// Convert RSA key to EVP
// Copy the EVP Key
var evp_key = EVP_PKEY_new()
var rc = EVP_PKEY_set1_RSA(evp_key, .make(optional: key.reference))
guard rc == 1 else {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {

throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
let rsa = EVP_PKEY_get1_RSA(.make(optional: key.reference))
EVP_PKEY_set1_RSA(evp_key, rsa)
RSA_free(rsa)
defer {
EVP_PKEY_free(evp_key)
}

// TODO: hash type option is not being used right now.
Expand All @@ -438,23 +447,34 @@ public class CryptorRSA {
defer {
EVP_CIPHER_CTX_reset_wrapper(rsaEncryptCtx)
EVP_CIPHER_CTX_free_wrapper(rsaEncryptCtx)
EVP_PKEY_free(evp_key)
}

EVP_CIPHER_CTX_set_padding(rsaEncryptCtx, padding)

// Initialize the AES encryption key array (of size 1)
typealias UInt8Ptr = UnsafeMutablePointer<UInt8>?
var ek: UInt8Ptr
ek = UnsafeMutablePointer<UInt8>.allocate(capacity: Int(EVP_PKEY_size(evp_key)))
ek = UnsafeMutablePointer<UInt8>.allocate(capacity: Int(EVP_PKEY_size(.make(optional: key.reference))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this change is correct / necessary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I get it - key is (now) a pointer to an envelope key, so this is equivalent to what we had before

let ekPtr = UnsafeMutablePointer<UInt8Ptr>.allocate(capacity: MemoryLayout<UInt8Ptr>.size)
ekPtr.pointee = ek

// Assign size of the corresponding cipher's IV
let IVLength = EVP_CIPHER_iv_length(.make(optional: enc))
let iv = UnsafeMutablePointer<UInt8>.allocate(capacity: Int(IVLength))

let encrypted = UnsafeMutablePointer<UInt8>.allocate(capacity: self.data.count + Int(IVLength))
defer {
#if swift(>=4.1)
ek?.deallocate()
ekPtr.deallocate()
iv.deallocate()
encrypted.deallocate()
#else
ek?.deallocate(capacity: Int(EVP_PKEY_size(.make(optional: key.reference))))
ekPtr.deallocate(capacity: MemoryLayout<UInt8Ptr>.size)
iv.deallocate(capacity: Int(IVLength))
encrypted.deallocate(capacity: self.data.count + Int(IVLength))
#endif
}
var encKeyLength: Int32 = 0
var processedLength: Int32 = 0
var encLength: Int32 = 0
Expand Down Expand Up @@ -505,6 +525,22 @@ public class CryptorRSA {
// Initialize the decryption context.
let rsaDecryptCtx = EVP_CIPHER_CTX_new()
EVP_CIPHER_CTX_init_wrapper(rsaDecryptCtx)
defer {
// On completion deallocate the memory
EVP_CIPHER_CTX_reset_wrapper(rsaDecryptCtx)
EVP_CIPHER_CTX_free_wrapper(rsaDecryptCtx)
}
// get rsaKey
guard let rsaKey = EVP_PKEY_get1_RSA(.make(optional: key.reference)) else {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {
throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
}
defer {
RSA_free(rsaKey)
}

// Set the additional authenticated data (aad) as the RSA key modulus and publicExponent in an ASN1 sequence.
guard let aad = key.publicKeyBytes else {
Expand Down Expand Up @@ -542,7 +578,6 @@ public class CryptorRSA {
let decrypted = UnsafeMutablePointer<UInt8>.allocate(capacity: Int(encryptedData.count + 16))
defer {
// On completion deallocate the memory
EVP_CIPHER_CTX_free_wrapper(rsaDecryptCtx)
#if swift(>=4.1)
aeskey.deallocate()
decrypted.deallocate()
Expand All @@ -559,7 +594,7 @@ public class CryptorRSA {
let iv = [UInt8](repeating: 0, count: 16)

// Decrypt the encryptedKey into the aeskey using the RSA private key
guard RSA_private_decrypt(Int32(encryptedKey.count), [UInt8](encryptedKey), aeskey, .make(optional: key.reference), RSA_PKCS1_OAEP_PADDING) != 0,
guard RSA_private_decrypt(Int32(encryptedKey.count), [UInt8](encryptedKey), aeskey, rsaKey, RSA_PKCS1_OAEP_PADDING) != 0,
// Set the IV length to be 16 bytes.
EVP_CIPHER_CTX_ctrl(rsaDecryptCtx, EVP_CTRL_GCM_SET_IVLEN, 16, nil) == 1,
// Set the AES key to be 16 bytes.
Expand Down Expand Up @@ -607,23 +642,11 @@ public class CryptorRSA {
/// Decrypt the data using aes GCM for cross platform support.
func decryptedCBC(with key: PrivateKey) throws -> PlaintextData? {
// Convert RSA key to EVP
var evp_key = EVP_PKEY_new()
var status = EVP_PKEY_set1_RSA(evp_key, .make(optional: key.reference))
guard status == 1 else {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {

throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
}

// TODO: hash type option is not being used right now.
let encType = EVP_aes_256_cbc()
let padding = RSA_PKCS1_OAEP_PADDING

// Size of symmetric encryption
let encKeyLength = Int(EVP_PKEY_size(evp_key))
let encKeyLength = Int(EVP_PKEY_size(.make(optional: key.reference)))
// Size of the corresponding cipher's IV
let encIVLength = Int(EVP_CIPHER_iv_length(.make(optional: encType)))
// Size of encryptedKey
Expand All @@ -640,7 +663,6 @@ public class CryptorRSA {
defer {
EVP_CIPHER_CTX_reset_wrapper(rsaDecryptCtx)
EVP_CIPHER_CTX_free_wrapper(rsaDecryptCtx)
EVP_PKEY_free(evp_key)
}

EVP_CIPHER_CTX_set_padding(rsaDecryptCtx, padding)
Expand All @@ -651,11 +673,17 @@ public class CryptorRSA {
var decMsgLen: Int32 = 0

let decrypted = UnsafeMutablePointer<UInt8>.allocate(capacity: Int(encryptedData.count + encryptedIV.count))

defer {
#if swift(>=4.1)
decrypted.deallocate()
#else
decrypted.deallocate(capacity: Int(encryptedData.count + encryptedIV.count))
#endif
}
// EVP_OpenInit returns 0 on error or the recovered secret key size if successful
status = encryptedKey.withUnsafeBytes({ (ek: UnsafePointer<UInt8>) -> Int32 in
var status = encryptedKey.withUnsafeBytes({ (ek: UnsafePointer<UInt8>) -> Int32 in
return encryptedIV.withUnsafeBytes({ (iv: UnsafePointer<UInt8>) -> Int32 in
return EVP_OpenInit(rsaDecryptCtx, .make(optional: encType), ek, Int32(encryptedKey.count), iv, evp_key)
return EVP_OpenInit(rsaDecryptCtx, .make(optional: encType), ek, Int32(encryptedKey.count), iv, .make(optional: key.reference))
})
})
guard status != 0 else {
Expand Down Expand Up @@ -723,28 +751,12 @@ public class CryptorRSA {
EVP_MD_CTX_free_wrapper(md_ctx)
}

// convert RSA key to EVP
let evp_key = EVP_PKEY_new()
var rc = EVP_PKEY_set1_RSA(evp_key, .make(optional: key.reference))
guard rc == 1 else {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {

throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
}

let (md, padding) = algorithm.algorithmForSignature
let (md, _) = algorithm.algorithmForSignature

// Provide a pkey_ctx to EVP_DigestSignInit so that the EVP_PKEY_CTX of the signing operation
// is written to it, to allow alternative signing options to be set
var pkey_ctx = EVP_PKEY_CTX_new(evp_key, nil)

EVP_DigestSignInit(md_ctx, &pkey_ctx, .make(optional: md), nil, evp_key)

// Now that Init has initialized pkey_ctx, set the padding option
EVP_PKEY_CTX_ctrl(pkey_ctx, EVP_PKEY_RSA, -1, EVP_PKEY_CTRL_RSA_PADDING, padding, nil)
EVP_DigestSignInit(md_ctx, nil, .make(optional: md), nil, .make(optional: key.reference))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, as a note to other readers, we discovered through experimentation that the attempt to set the padding algorithm (that is being removed here) 1. fails to actually alter the padding used, and 2. causes the memory allocated by other openssl routines to be leaked.

The default padding algorithm is already RSA_PKCS1_PADDING, so there's no need to set it. Ideally we would be explicit about it (in case the default changes in the future) but for now, this achieves the same as the existing code and does not leak.


// Convert Data to UnsafeRawPointer!
_ = self.data.withUnsafeBytes({ (message: UnsafePointer<UInt8>) -> Int32 in
Expand All @@ -756,7 +768,15 @@ public class CryptorRSA {
EVP_DigestSignFinal(md_ctx, nil, &sig_len)
let sig = UnsafeMutablePointer<UInt8>.allocate(capacity: sig_len)

rc = EVP_DigestSignFinal(md_ctx, sig, &sig_len)
defer {
#if swift(>=4.1)
sig.deallocate()
#else
sig.deallocate(capacity: sig_len)
#endif
}

let rc = EVP_DigestSignFinal(md_ctx, sig, &sig_len)
guard rc == 1, sig_len > 0 else {
let source = "Signing failed."
if let reason = CryptorRSA.getLastError(source: source) {
Expand Down Expand Up @@ -824,30 +844,15 @@ public class CryptorRSA {
EVP_MD_CTX_free_wrapper(md_ctx)
}

// convert RSA key to EVP
let evp_key = EVP_PKEY_new()
var rc = EVP_PKEY_set1_RSA(evp_key, .make(optional: key.reference))
guard rc == 1 else {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {

throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
}

let (md, padding) = algorithm.algorithmForSignature
let (md, _) = algorithm.algorithmForSignature

// Provide a pkey_ctx to EVP_DigestSignInit so that the EVP_PKEY_CTX of the signing operation
// is written to it, to allow alternative signing options to be set
var pkey_ctx = EVP_PKEY_CTX_new(evp_key, nil)

EVP_DigestVerifyInit(md_ctx, &pkey_ctx, .make(optional: md), nil, evp_key)
EVP_DigestVerifyInit(md_ctx, nil, .make(optional: md), nil, .make(optional: key.reference))

// Now that EVP_DigestVerifyInit has initialized pkey_ctx, set the padding option
EVP_PKEY_CTX_ctrl(pkey_ctx, EVP_PKEY_RSA, -1, EVP_PKEY_CTRL_RSA_PADDING, padding, nil)

rc = self.data.withUnsafeBytes({ (message: UnsafePointer<UInt8>) -> Int32 in
var rc = self.data.withUnsafeBytes({ (message: UnsafePointer<UInt8>) -> Int32 in
return EVP_DigestUpdate(md_ctx, message, self.data.count)
})
guard rc == 1 else {
Expand Down
24 changes: 7 additions & 17 deletions Sources/CryptorRSA/CryptorRSAKey.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,25 +362,12 @@ extension CryptorRSA {
}

// Extract the certificate's public key data.
let evp_key = X509_get_pubkey(cert)
let evp_key: OpaquePointer? = .init(X509_get_pubkey(cert))
if evp_key == nil {
throw Error(code: ERR_CREATE_CERT_FAILED, reason: "Error getting public key from certificate")
}

let key = EVP_PKEY_get1_RSA( evp_key)
if key == nil {
throw Error(code: ERR_CREATE_CERT_FAILED, reason: "Error getting public key from certificate")
}
defer {
// RSA_free(key)
EVP_PKEY_free(evp_key)
}

#if swift(>=4.1)
return PublicKey(with: .make(optional: key!)!)
#else
return PublicKey(with: key!)
#endif
return PublicKey(with: evp_key)

#else

Expand Down Expand Up @@ -610,6 +597,9 @@ extension CryptorRSA {

#if os(Linux)
var publicKeyBytes: Data?
deinit {
EVP_PKEY_free(.make(optional: reference))
}
#endif

/// Represents the type of key data contained.
Expand Down Expand Up @@ -683,7 +673,7 @@ extension CryptorRSA {
///
/// - Returns: New `RSAKey` instance.
///
internal init(with nativeKey: UnsafeMutablePointer<rsa_st>, type: KeyType) {
internal init(with nativeKey: UnsafeMutablePointer<EVP_PKEY>, type: KeyType) {

self.type = type
self.reference = .make(optional: nativeKey)
Expand Down Expand Up @@ -811,7 +801,7 @@ extension CryptorRSA {
///
/// - Returns: New `RSAKey` instance.
///
public init(with nativeKey: UnsafeMutablePointer<rsa_st>) {
public init(with nativeKey: UnsafeMutablePointer<EVP_PKEY>) {

super.init(with: nativeKey, type: .publicType)
}
Expand Down
17 changes: 1 addition & 16 deletions Sources/CryptorRSA/CryptorRSAUtilities.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ public extension CryptorRSA {
}

var evp_key: OpaquePointer?

defer {
EVP_PKEY_free(.make(optional: evp_key))
}

// Read in the key data and process depending on key type...
if type == .publicType {
Expand All @@ -88,18 +84,7 @@ public extension CryptorRSA {

evp_key = .init(PEM_read_bio_PrivateKey(bio, nil, nil, nil))
}

let key = EVP_PKEY_get1_RSA(.make(optional: evp_key))
if key == nil {
let source = "Couldn't create key reference from key data"
if let reason = CryptorRSA.getLastError(source: source) {

throw Error(code: ERR_ADD_KEY, reason: reason)
}
throw Error(code: ERR_ADD_KEY, reason: source + ": No OpenSSL error reported.")
}

return .init(key!)
return evp_key
}

///
Expand Down