On 2/3/22 06:27, Hans Leidekker wrote:
On Tue, 2022-03-01 at 19:34 -0300, Santino Mazza wrote: Signed-off-by: Santino Mazza mazzasantino1206@gmail.com
dlls/ncrypt/Makefile.in | 1 + dlls/ncrypt/main.c | 101 +++++++++++++--------------------- dlls/ncrypt/ncrypt_internal.h | 30 ++-------- dlls/ncrypt/tests/ncrypt.c | 2 + 4 files changed, 45 insertions(+), 89 deletions(-)
diff --git a/dlls/ncrypt/Makefile.in b/dlls/ncrypt/Makefile.in index ad3ed409961..58ec6b3456b 100644 --- a/dlls/ncrypt/Makefile.in +++ b/dlls/ncrypt/Makefile.in @@ -1,5 +1,6 @@ IMPORTLIB = ncrypt MODULE = ncrypt.dll +IMPORTS = bcrypt
EXTRADLLFLAGS = -Wb,--prefer-native
diff --git a/dlls/ncrypt/main.c b/dlls/ncrypt/main.c index 68d9d884618..2804708f10f 100644 --- a/dlls/ncrypt/main.c +++ b/dlls/ncrypt/main.c @@ -88,20 +88,14 @@ SECURITY_STATUS WINAPI NCryptFreeBuffer(PVOID buf)
static SECURITY_STATUS free_key_object(struct key *key) {
- switch (key->alg)
- {
- case RSA:
- {
free(key->rsa.modulus);free(key->rsa.public_exp);free(key->rsa.prime1);free(key->rsa.prime2);break;- }
- default:
WARN("invalid key %p\n", key);return NTE_INVALID_HANDLE;- }
- NTSTATUS bcrypt_ret;
- bcrypt_ret = BCryptDestroyKey(key->bcrypt_key);
- if(bcrypt_ret != ERROR_SUCCESS) return NTE_INVALID_HANDLE;
- bcrypt_ret = BCryptCloseAlgorithmProvider(key->alg_prov, 0);
- if(bcrypt_ret != ERROR_SUCCESS) return NTE_INVALID_HANDLE;
There's no need to check for failure here. If BCryptDestroyKey() would fail you would leak the algorithm handle.
return ERROR_SUCCESS; }
@@ -246,6 +240,7 @@ SECURITY_STATUS WINAPI NCryptImportKey(NCRYPT_PROV_HANDLE provider, NCRYPT_KEY_H BYTE *data, DWORD datasize, DWORD flags) { BCRYPT_KEY_BLOB *header = (BCRYPT_KEY_BLOB *)data;
- struct object *key_object;
TRACE("(%#Ix, %#Ix, %s, %p, %p, %p, %lu, %#lx): stub\n", provider, decrypt_key, wine_dbgstr_w(type), params, handle, data, datasize, flags); @@ -270,70 +265,48 @@ SECURITY_STATUS WINAPI NCryptImportKey(NCRYPT_PROV_HANDLE provider, NCRYPT_KEY_H return NTE_BAD_FLAGS; }
- switch (header->Magic)
- if(!(key_object = allocate_object(KEY)))
- {
ERR("Error allocating memory\n");return NTE_NO_MEMORY;- }
- key_object->key.storage_prov = provider;
- switch(header->Magic)
{
- case BCRYPT_RSAFULLPRIVATE_MAGIC:
- case BCRYPT_RSAPRIVATE_MAGIC:
case BCRYPT_RSAPUBLIC_MAGIC: {
DWORD expected_size;struct object *object;struct key *key;BYTE *public_exp, *modulus;BCRYPT_RSAKEY_BLOB *rsaheader = (BCRYPT_RSAKEY_BLOB *)data;if (datasize < sizeof(*rsaheader))
NTSTATUS ret;BCRYPT_RSAKEY_BLOB *rsablob = (BCRYPT_RSAKEY_BLOB *)data;ret = BCryptOpenAlgorithmProvider(&key_object->key.alg_prov, BCRYPT_RSA_ALGORITHM, NULL, 0);if(ret != ERROR_SUCCESS){
ERR("Invalid buffer size.\n");return NTE_BAD_DATA;
ERR("Error opening algorithm provider\n");return NTE_INTERNAL_ERROR;Should we return the error from BCryptOpenAlgorithmProvider() here?
}
expected_size = sizeof(*rsaheader) + rsaheader->cbPublicExp + rsaheader->cbModulus;if (datasize != expected_size)
ret = BCryptImportKeyPair(key_object->key.alg_prov, NULL, type, &key_object->key.bcrypt_key, data, datasize, 0);if(ret != ERROR_SUCCESS){
ERR("Invalid buffer size.\n");return NTE_BAD_DATA;
ERR("Error importing keypair with bcrypt %#lx\n", ret);return NTE_INTERNAL_ERROR;And return the error from BCryptImportKeyPair() here?
}
if (!(object = allocate_object(KEY))){ERR("Error allocating memory.\n");return NTE_NO_MEMORY;}key = &object->key;key->alg = RSA;key->rsa.bit_length = rsaheader->BitLength;key->rsa.public_exp_size = rsaheader->cbPublicExp;key->rsa.modulus_size = rsaheader->cbModulus;if (!(key->rsa.public_exp = malloc(rsaheader->cbPublicExp))){ERR("Error allocating memory.\n");free(object);return NTE_NO_MEMORY;}if (!(key->rsa.modulus = malloc(rsaheader->cbModulus))){ERR("Error allocating memory.\n");free(key->rsa.public_exp);free(object);return NTE_NO_MEMORY;}public_exp = &data[sizeof(*rsaheader)]; /* The public exp is after the header. */modulus = &public_exp[rsaheader->cbPublicExp]; /* The modulus is after the public exponent. */memcpy(key->rsa.public_exp, public_exp, rsaheader->cbPublicExp);memcpy(key->rsa.modulus, modulus, rsaheader->cbModulus);set_object_property(object, NCRYPT_ALGORITHM_GROUP_PROPERTY, (BYTE *)L"RSA", sizeof(L"RSA"));set_object_property(object, NCRYPT_LENGTH_PROPERTY, (BYTE *)&key->rsa.bit_length, sizeof(key->rsa.bit_length));set_object_property(object, NCRYPT_PROVIDER_HANDLE_PROPERTY, (BYTE *)&provider, sizeof(provider));*handle = (NCRYPT_KEY_HANDLE)object;
set_object_property(key_object, NCRYPT_PROVIDER_HANDLE_PROPERTY, (BYTE *)&provider, sizeof(NCRYPT_PROV_HANDLE));set_object_property(key_object, NCRYPT_ALGORITHM_GROUP_PROPERTY, (BYTE *)BCRYPT_RSA_ALGORITHM, sizeof(BCRYPT_RSA_ALGORITHM));set_object_property(key_object, NCRYPT_LENGTH_PROPERTY, (BYTE *)&rsablob->BitLength, sizeof(rsablob->BitLength));break; } default:
FIXME("Unhandled key magic %#lx.\n", header->Magic);
- {
FIXME("Unhandled key magic %#lx\n", header->Magic);return NTE_INVALID_PARAMETER;
break;- }
}
- *handle = (NCRYPT_KEY_HANDLE)key_object;
return ERROR_SUCCESS; }
diff --git a/dlls/ncrypt/ncrypt_internal.h b/dlls/ncrypt/ncrypt_internal.h index 3966dd73ed6..2d916d4fbd8 100644 --- a/dlls/ncrypt/ncrypt_internal.h +++ b/dlls/ncrypt/ncrypt_internal.h @@ -16,34 +16,14 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */
-enum key_algorithm -{
- DH,
- DSA,
- ECC,
- RSA,
-};
-struct rsa_key -{
- DWORD bit_length;
- DWORD public_exp_size;
- BYTE *public_exp;
- DWORD modulus_size;
- BYTE *modulus;
- DWORD prime1_size;
- BYTE *prime1;
- DWORD prime2_size;
- BYTE *prime2;
-}; +#include <ncrypt.h> +#include <bcrypt.h>
struct key {
- enum key_algorithm alg;
- union
- {
struct rsa_key rsa;- };
- NCRYPT_PROV_HANDLE storage_prov;
- BCRYPT_ALG_HANDLE alg_prov;
- BCRYPT_KEY_HANDLE bcrypt_key;
};
struct storage_provider diff --git a/dlls/ncrypt/tests/ncrypt.c b/dlls/ncrypt/tests/ncrypt.c index d1bcbbcab2a..8080e465527 100644 --- a/dlls/ncrypt/tests/ncrypt.c +++ b/dlls/ncrypt/tests/ncrypt.c @@ -123,6 +123,7 @@ static void test_key_import_rsa(void) ok(key, "got null handle\n"); NCryptFreeObject(key);
- todo_wine{
ret = NCryptImportKey(prov, 0, BCRYPT_PUBLIC_KEY_BLOB, NULL, &key, rsa_key_blob_with_invalid_publicexp_size, sizeof(rsa_key_blob_with_invalid_publicexp_size), 0); ok(ret == NTE_BAD_DATA, "got %#lx\n", ret); @@ -132,6 +133,7 @@ static void test_key_import_rsa(void)
ret = NCryptImportKey(prov, 0, BCRYPT_PUBLIC_KEY_BLOB, NULL, &key, rsa_key_blob, sizeof(rsa_key_blob) + 1, 0); ok(ret == NTE_BAD_DATA, "got %#lx\n", ret);
- }
NCryptFreeObject(prov); }
You should restructure your patches so that you don't have to reintroduce todo_wine statements.
Hello Hans, thanks for your review. About returning the errors of bcrypt from a ncrypt function (Like the calls to BCryptOpenAlgorithmProvider() from NCryptImportKey), I try to prevent doing that because both functions return two different types of errors, ncrypt returns SECURITY_STATUS and bcrypt returns NTSTATUS.