Don't encrypting correctly wincrypt

Question

I'm trying to encrypt a string as size of 8 bytes (64 bits) using AES128 from wincrypt.h, note that the string is smaller than the block size of AES which is 128 bits.

HCRYPTKEY hKey;
if (!CryptDeriveKey(hProv, CALG_AES_128, hHash, 0, &hKey)) {
    dwStatus = GetLastError();
    printf("CryptDeriveKey failed: %x\n", dwStatus);
    CryptReleaseContext(hProv, 0);
    system("pause");
    return dwStatus;
}

printf("[+] CryptDeriveKey Success\n");

const size_t string_size = 8;
BYTE string[8] = { "Foooooo" };   // \0 counts as 1 byte
DWORD out_len = 8;


if (!CryptEncrypt(hKey, NULL, TRUE, 0, string, &out_len, string_size)) {
        printf("[-] CryptEncrypt failed\n");
}

printf("%s", string);
printf("\n");

if (!CryptDecrypt(hKey, NULL, TRUE, 0, string, &out_len)) {
    printf("[-] CryptDecrypt failed\n");

}

printf("%s", string);
printf("\n");

But it doesn't look to encrypt/decrypt well because I get this Output:

[+] CryptDeriveKey Success
Fooooooo
[-] CryptEncrypt failed
ÉÆ╠1╔P█ídhù;$§┴
[-] CryptDecrypt failed

What I'm doing wrong? &out_len or string_size should be 128 as AES block size?

Ok, Final is set, it might be helpful to retrieve the error code with `GetLastError()` — Ctx, Aug 18 '18 at 15:09
`As a rule, if a stream cipher is used, the ciphertext is the same size as the plaintext. If a block cipher is used, the ciphertext is up to a block length larger than the plaintext.`, so you should have at least 24 bytes of space — Ctx, Aug 18 '18 at 15:15
Ok you are correct I just put 128 bytes as buffer for the string and it worked flawessly. — , Aug 18 '18 at 15:22
Note [the docs state](https://learn.microsoft.com/en-us/windows/desktop/api/wincrypt/nf-wincrypt-cryptencrypt), _"If `pbData` is NULL, no error is returned, and the function stores the size of the encrypted data, in bytes, in the DWORD value pointed to by `pdwDataLen`. **This allows an application to determine the correct buffer size.**"_ You can also attempt to "one-shot" it and if the function fails, and `GetLastError() == ERROR_MORE_DATA`, the docs state, that the function _"stores the required buffer size, in bytes, in the DWORD value pointed to by `pdwDataLen`."_ — , Aug 18 '18 at 15:34
In short, encrypt something, and if it fails and `GetLastError() == ERROR_MORE_DATA`, copy your string to a new buffer with at least `out_len` bytes, then encrypt that buffer. For this reason, you'll likely use `malloc`/`calloc`/`realloc` and `free` with a pointer, not a static array. — , Aug 18 '18 at 15:34

lockcmpxchg8b · Answer 1 · 2018-08-18T15:25:36.720

1

The docs for CryptEncrypt indicate that the input/output lengths are in bytes, so this should be a multiple of 16 for AES*

This is also key to your issue, I think: CryptEncrypt operates in-place---that is, it overwrites the input plaintext with the output ciphertext. Your buffer is too small to receive a 16-byte output, so the last 8 bytes are most likely being corrupted between your Encrypt and Decrypt operations.

Try bumping

BYTE string[8] = { "Foooooo" }; const size_t string_size = 8;

to

BYTE string[16] = { "Foooooo" }; const size_t string_size = 16;

Also, read up on PKCS7 padding, which is probably how wincrypt is determining that your ciphertext is bad.

*except for ciphertext stealing modes, or stream modes. These modes can't "fail" on decrypt, so wincrypt is not using one of those.

Not related to your question, but printing ciphertext via printf is also not a good idea. You'll get a better understanding by printing the ciphertext in hex:

{DWORD i; for(DWORD i=0; i < out_len; i++) printf("%02x ", string[i]); printf("\n");}

edited Aug 18 '18 at 15:25

answered Aug 18 '18 at 15:18

lockcmpxchg8b

2,205
10
16

The manpage demands plainlen + blocksize bytes at least in the output buffer – Ctx Aug 18 '18 at 15:21
That's probably because PKCS7 padding will add an entire block of padding if the plaintext length exactly divides 16. It's easier to explain as "leave an extra block of space" rather than "round up to a multiple of 16, unless it's already a multiple, and in that case, add 16". – lockcmpxchg8b Aug 18 '18 at 15:27
thanks guys I will look for dynamic memory, but you are correct the buffer of string was very small to get the ciphertext – Aug 18 '18 at 15:45
@EduardoG: dynamic memory allocation gives you flexibility in terms of message size, but has a few costs: 1. the obvious heap-walk duration and fragmentation issues that affect long-running applications; and 2. security. Crypto is typically used in security applications, and that forces one to consider how you know the right size to allocate...particularly if you're decrypting things received from untrusted parties on a network. Just a few things to consider. – lockcmpxchg8b Aug 18 '18 at 15:54

Don't encrypting correctly wincrypt

1 Answers1