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

Jump to bottom

BoringSSLRSAPublicKey use EVP_PKEY API #205

Merged
merged 3 commits into from
Oct 24, 2023
Merged

BoringSSLRSAPublicKey use EVP_PKEY API #205

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d08922f
BoringSSLRSAPublicKey use EVP_PKEY API
rnro Oct 19, 2023
aa63083
review comments
rnro Oct 23, 2023
a4454dd
BoringSSL Public/Private keys store EVP key
rnro Oct 24, 2023
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
8 changes: 8 additions & 0 deletions Sources/CCryptoBoringSSLShims/include/CCryptoBoringSSLShims.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ int CCryptoBoringSSLShims_RSA_public_encrypt(int flen, const void *from, void *t
int CCryptoBoringSSLShims_RSA_private_decrypt(int flen, const void *from, void *to,
RSA *rsa, int padding);

int CCryptoBoringSSLShims_EVP_PKEY_encrypt(EVP_PKEY_CTX *ctx, void *out,
size_t *out_len, const void *in,
size_t in_len);

int CCryptoBoringSSLShims_EVP_PKEY_decrypt(EVP_PKEY_CTX *ctx, void *out,
size_t *out_len, const void *in,
size_t in_len);

#if defined(__cplusplus)
}
#endif // defined(__cplusplus)
Expand Down
12 changes: 12 additions & 0 deletions Sources/CCryptoBoringSSLShims/shims.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,15 @@ int CCryptoBoringSSLShims_RSA_private_decrypt(int flen, const void *from, void *
RSA *rsa, int padding) {
return CCryptoBoringSSL_RSA_private_decrypt(flen, from, to, rsa, padding);
}

int CCryptoBoringSSLShims_EVP_PKEY_encrypt(EVP_PKEY_CTX *ctx, void *out,
size_t *out_len, const void *in,
size_t in_len) {
return CCryptoBoringSSL_EVP_PKEY_encrypt(ctx, out, out_len, in, in_len);
}

int CCryptoBoringSSLShims_EVP_PKEY_decrypt(EVP_PKEY_CTX *ctx, void *out,
size_t *out_len, const void *in,
size_t in_len) {
return CCryptoBoringSSL_EVP_PKEY_decrypt(ctx, out, out_len, in, in_len);
}
67 changes: 53 additions & 14 deletions Sources/_CryptoExtras/RSA/RSA_boring.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,32 @@ extension BoringSSLRSAPublicKey {
switch padding.backing {
case .pkcs1_oaep: rawPadding = RSA_PKCS1_OAEP_PADDING
}
let rc = CCryptoBoringSSLShims_RSA_public_encrypt(
CInt(dataPtr.count),
dataPtr.baseAddress,

let pkey = CCryptoBoringSSL_EVP_PKEY_new()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should amend the type to only hold this as an EVP_PKEY, instead of holding an RSA pointer at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously you have more context here but I'm not sure that's the best thing to do.

The RSA pointer is currently used in a few different locations outside of the initializers e.g. pkcs1DERRepresentation, derRepresentation, keySizeInBits, isValidSignature. Each of these would then need a call to get the RSA pointer unless we also stored that (which is an option). I looked for alternative functions but there are a few including BIO operations which now have annotations in the docs which look like deprecations (here) which I think make this complex enough to warrant a separate piece of work if we want to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the get0 functions offer a cheap way to access the underlying RSA object we have opted to just keep hold of the EVP_KEY

defer {
CCryptoBoringSSL_EVP_PKEY_free(pkey)
}

CCryptoBoringSSL_EVP_PKEY_set1_RSA(pkey, self.pointer)

// nil engine defaults to the standard implementation with no hooks
let ctx = CCryptoBoringSSL_EVP_PKEY_CTX_new(pkey, nil)
defer {
CCryptoBoringSSL_EVP_PKEY_CTX_free(ctx)
}

CCryptoBoringSSL_EVP_PKEY_encrypt_init(ctx)
CCryptoBoringSSL_EVP_PKEY_CTX_set_rsa_padding(ctx, rawPadding)

var writtenLength = bufferPtr.count
Lukasa marked this conversation as resolved.
Show resolved Hide resolved
let rc = CCryptoBoringSSLShims_EVP_PKEY_encrypt(
ctx,
bufferPtr.baseAddress,
self.pointer,
rawPadding
&writtenLength,
dataPtr.baseAddress,
dataPtr.count
)

return rc
}
}
Expand Down Expand Up @@ -465,26 +484,46 @@ extension BoringSSLRSAPrivateKey {
var output = Data(count: outputSize)

let contiguousData: ContiguousBytes = data.regions.count == 1 ? data.regions.first! : Array(data)
let rc: CInt = output.withUnsafeMutableBytes { bufferPtr in
let writtenLength: CInt = output.withUnsafeMutableBytes { bufferPtr in
contiguousData.withUnsafeBytes { dataPtr in
let rawPadding: CInt
switch padding.backing {
case .pkcs1_oaep: rawPadding = RSA_PKCS1_OAEP_PADDING
}
let rc = CCryptoBoringSSLShims_RSA_private_decrypt(
CInt(dataPtr.count),
dataPtr.baseAddress,

let pkey = CCryptoBoringSSL_EVP_PKEY_new()
defer {
CCryptoBoringSSL_EVP_PKEY_free(pkey)
}

CCryptoBoringSSL_EVP_PKEY_set1_RSA(pkey, self.pointer)

let ctx = CCryptoBoringSSL_EVP_PKEY_CTX_new(pkey, nil)
defer {
CCryptoBoringSSL_EVP_PKEY_CTX_free(ctx)
}

CCryptoBoringSSL_EVP_PKEY_decrypt_init(ctx)
CCryptoBoringSSL_EVP_PKEY_CTX_set_rsa_padding(ctx, rawPadding)

var writtenLength = bufferPtr.count

// returns 1 on success and 0 on failure
let rc = CCryptoBoringSSLShims_EVP_PKEY_decrypt(
ctx,
bufferPtr.baseAddress,
self.pointer,
rawPadding
&writtenLength,
dataPtr.baseAddress,
dataPtr.count
)
return rc

return rc == 0 ? CInt(-1) : CInt(writtenLength)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not use -1 as a sentinel, let's just throw.

}
}
if rc == -1 {
if writtenLength == -1 {
throw CryptoKitError.internalBoringSSLError()
}
output.removeSubrange(output.index(output.startIndex, offsetBy: Int(rc)) ..< output.endIndex)
output.removeSubrange(output.index(output.startIndex, offsetBy: Int(writtenLength)) ..< output.endIndex)
return output
}

Expand Down