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

CVE-2016-6225: xtrabackup encryption is not setting the IV correctly #266

Merged
merged 1 commit into from
Nov 22, 2016
Merged

CVE-2016-6225: xtrabackup encryption is not setting the IV correctly #266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 25 additions & 10 deletions storage/innobase/xtrabackup/src/ds_encrypt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA

#include "xbcrypt.h"

#if !defined(GCRYPT_VERSION_NUMBER) || (GCRYPT_VERSION_NUMBER < 0x010600)
Copy link

Choose a reason for hiding this comment

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

Can we not rely on the GCRYPT_VERSION_NUMBER to be always available and shouldn't we error out if this is not available ? (I presume this is likely to be backwards compatibility)

Also why do we require the version to be less than 0x010600 (67072)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. I added this check for compatibility. Older libgrypt versions required to use

GCRY_THREAD_OPTION_PTHREAD_IMPL;

and

gcry_control(GCRYCTL_SET_THREAD_CBS, &gcry_threads_pthread);

to work correctly in multithreaded environment. This code no longer needed and deprecated since libgrypt 1.6. Compiler gives warnings for this code. Unfortunately I cannot just remove it, because we support CentOS 5 and 6. Version number is declared in the gcrypt.h as follows:

/* The version of this header should match the one of the library. It
   should not be used by a program because gcry_check_version() should
   return the same version.  The purpose of this macro is to let
   autoconf (using the AM_PATH_GCRYPT macro) check that this header
   matches the installed library.  */
#define GCRYPT_VERSION "1.7.3"

/* The version number of this header.  It may be used to handle minor
   API incompatibilities.  */
#define GCRYPT_VERSION_NUMBER 0x010703

0x010703 is 1.7.3. So, hexadecimal number in fact should be read as decimal. Ask libgrcypt maintainers why :)

GCRY_THREAD_OPTION_PTHREAD_IMPL;
#endif

#define XB_CRYPT_CHUNK_SIZE ((size_t) (xtrabackup_encrypt_chunk_size))

Expand Down Expand Up @@ -101,7 +103,6 @@ static uint encrypt_algos[] = { GCRY_CIPHER_NONE, GCRY_CIPHER_AES128,
static uint encrypt_algo;
static const uint encrypt_mode = GCRY_CIPHER_MODE_CTR;
static uint encrypt_key_len = 0;
static const unsigned char encrypt_iv[] = "Percona Xtrabackup is Awesome!!!";
static size_t encrypt_iv_len = 0;

static
Expand Down Expand Up @@ -132,6 +133,7 @@ encrypt_init(const char *root)

/* Acording to gcrypt docs (and my testing), setting up the threading
callbacks must be done first, so, lets give it a shot */
#if !defined(GCRYPT_VERSION_NUMBER) || (GCRYPT_VERSION_NUMBER < 0x010600)
Copy link

Choose a reason for hiding this comment

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

as per above comments

gcry_error = gcry_control(GCRYCTL_SET_THREAD_CBS, &gcry_threads_pthread);
if (gcry_error) {
msg("encrypt: unable to set libgcrypt thread cbs - "
Expand All @@ -140,6 +142,7 @@ encrypt_init(const char *root)
gcry_strerror(gcry_error));
return NULL;
}
#endif

/* Version check should be the very next call because it
makes sure that important subsystems are intialized. */
Expand Down Expand Up @@ -181,7 +184,6 @@ encrypt_init(const char *root)
/* Set up the iv length */
encrypt_iv_len = gcry_cipher_get_algo_blklen(encrypt_algo);
xb_a(encrypt_iv_len > 0);
xb_a(encrypt_iv_len <= sizeof(encrypt_iv));

/* Now set up the key */
if (xtrabackup_encrypt_key == NULL &&
Expand Down Expand Up @@ -345,7 +347,8 @@ encrypt_write(ds_file_t *file, const void *buf, size_t len)

if (xb_crypt_write_chunk(crypt_file->xbcrypt_file,
threads[i].to,
threads[i].from_len,
threads[i].from_len +
XB_CRYPT_HASH_LEN,
threads[i].to_len,
threads[i].iv,
encrypt_iv_len)) {
Expand Down Expand Up @@ -424,8 +427,8 @@ create_worker_threads(uint n)
thd->cancelled = FALSE;
thd->data_avail = FALSE;

thd->to = (char *) my_malloc(XB_CRYPT_CHUNK_SIZE,
MYF(MY_FAE));
thd->to = (char *) my_malloc(XB_CRYPT_CHUNK_SIZE +
XB_CRYPT_HASH_LEN, MYF(MY_FAE));

thd->iv = (char *) my_malloc(encrypt_iv_len,
MYF(MY_FAE));
Expand Down Expand Up @@ -552,6 +555,14 @@ encrypt_worker_thread_func(void *arg)
if (thd->cancelled)
break;

/* ensure that XB_CRYPT_HASH_LEN is the correct length
of XB_CRYPT_HASH hashing algorithm output */
assert(gcry_md_get_algo_dlen(XB_CRYPT_HASH) ==
Copy link

Choose a reason for hiding this comment

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

May want to add some comments in here that we want to force an error to write to stderr then abort() the process where the length of the hash XB_CRYPT_HASH does not match the expected length XB_CRYPT_HASH please correct me if I am wrong at all here, it's been a long time since I looked at C/C++

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I defined XB_CRYPT_HASH_LEN manually. It must match the length of the output given by chosen hash function (XB_CRYPT_HASH). I use it for buffer allocations and so on. This assert helps to ensure that if I change the hash function (XB_CRYPT_HASH), I also change XB_CRYPT_HASH_LEN and avoid buffer overflow errors. It helps to keep the code maintainable.

Copy link

Choose a reason for hiding this comment

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

Yes I understand that, I just meant that a source code comment would have been appreciated here e.g.

#Ensure the returned value of gcry_md_get_algo_dlen(XB_CRYPT_HASH) is equal to XB_CRYPT_HASH_LEN or throw an assertion.
assert(gcry_md_get_algo_dlen(XB_CRYPT_HASH) ==
               XB_CRYPT_HASH_LEN);

Entirely depends on your coding standard though, so feel free to not include something like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment added

XB_CRYPT_HASH_LEN);

memcpy(thd->to, thd->from, thd->from_len);
Copy link

Choose a reason for hiding this comment

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

thd->to char* pointer containing my_malloc(XB_CRYPT_CHUNK_SIZE + XB_CRYPT_HASH_LEN, MYF(MY_FAE))
thd->from contains (const char *) buf presumable this is the plaintext ?
thd->from_len contains chunk_len = (len > XB_CRYPT_CHUNK_SIZE) ? XB_CRYPT_CHUNK_SIZE : len;

So this confirms we are running a memcpy to memory of the plaintext right before it is encrypted ?
There's no routine I can see to run GC so we're not purging memory after completion of the backup process ? this could lead to artifacts which contain the unencrypted content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is an interesting part actually. I want to encrypt buffer pointed by thd->from and its hash and put the result into thd->to. I have two options:

  1. allocate hash_buffer, put hash there, then invoke gcry_cipher_encrypt(thd->from, thd->to); gcry_cipher_encrypt(hash_buffer, thd->to + thd->from_len). I.e. encrypt the buffer and the hash separately (it is streaming mode, so we are good) and put the result into the thd->to.
  2. copy thd->from into thd->to, append hash to thd->to, encrypt entire chunk with single gcry_cipher_encrypt(thd->to, thd->to). Libgrypt can work with the buffer inplace.

The caveat is that option 1 doesn't work for me in multithreaded environments in CentOS 5 and 6 (sigh), so I went with option 2.

As for plaintext chunks left over in the memory... Yes, we don't scrape the buffer, but it will be overwritten with the next chunk, and so on. At the moment when xtrabackup exists, all its memory will be given back to the system and it's kernels trouble to fill them with random garbage if needed. Isn't it?

Copy link

@Oneiroi Oneiroi Nov 16, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Disclaimer is I may be wrong here, but C/C++ does not have any Garbage Collection as I noted above, the Linux Kernel does not (be default) run any GC / Purge against free memory.

You are literally waiting for a process to come along and overwrite the same address in memory, or the system is shutdown (must be powered off) and allowed to cold boot after some time (even then this doesn't address physical memory artifacts).

Perhaps I am overthinking this, however I think that right after we are done with this buffer writing an equivalent length of random data would ensure the artifact is removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sometime the key copies in memory are overwritten once they are not used anymore (http://security.stackexchange.com/questions/74280/would-it-be-good-secure-programming-practice-to-overwrite-a-sensitive-variable etc). If you choose to do so, make sure not to use memset which can be optimized away (memset_s if available is OK)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably makes sense to wipe out buffers for long living processes when keys or data are no longer used may still reside in processes memory and even be accessible to remote attacker via buffer overflow or other potential bugs.

I think xtrabackup is not the case. Encryption thread working as follows:

  1. Copy plaintext from thd->from to thd->to (thd->from is not
    needed anymore it will be reused to read the next chunk of
    plaintext)
  2. Append hash to thd->to
  3. Encrypt thd->to in place

Yes, 16 kbytes of plaintext are left in the thd->from and we don't
erase it (actually the whole 1M of plaintext can be read into memory
at once and we process it in 16k chunks). We also keep secret key in
memory for the entire life of xtrabackup process. I don't think it
is an issue because:

  • we reuse the same buffer to read the next chunk of plaintext
    immediately after we wrote down encrypted data
  • if the chunk was the last one, xtrabackup is going to exit.
    xtrabackup is a simple tool, it exits once copying is done.
  • after xtrabackup exited, its memory goes back to Linux kernel.
    Linux zeroes the memory before giving it to another process.

Lets say attacker has an access to xtrabackup memory. xtrabackup
isn't available via network, so attacker likely already has an access
to the filesystem. Then he will just read the data from the
filesystem, it is much easier option for him.

Am I missing something here?

gcry_md_hash_buffer(XB_CRYPT_HASH, thd->to + thd->from_len,
thd->from, thd->from_len);
thd->to_len = thd->from_len;

if (encrypt_algo != GCRY_CIPHER_NONE) {
Copy link

Choose a reason for hiding this comment

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

we only run the encryption where it is not set to the algorithm GCRY_CIPHER_NONE can we think of any conditions where this may occur ? (not a priority to do this, just for my curiosity and education on PXB)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible values for encrypt-algo option are "NONE", "AES128", "AES192" and "AES256". It is user settable. One could run xtrabackup --backup --encrypt --encrypt-algo=NONE and get the unencrypted output. The only use case I can imagine for it is debugging.

Expand All @@ -568,7 +579,7 @@ encrypt_worker_thread_func(void *arg)
}

xb_crypt_create_iv(thd->iv, encrypt_iv_len);
gcry_error = gcry_cipher_setiv(thd->cipher_handle,
gcry_error = gcry_cipher_setctr(thd->cipher_handle,
thd->iv,
encrypt_iv_len);
if (gcry_error) {
Expand All @@ -582,18 +593,22 @@ encrypt_worker_thread_func(void *arg)

gcry_error = gcry_cipher_encrypt(thd->cipher_handle,
thd->to,
thd->to_len,
thd->from,
thd->from_len);
thd->to_len +
XB_CRYPT_HASH_LEN,
thd->to,
thd->from_len +
XB_CRYPT_HASH_LEN);
if (gcry_error) {
msg("encrypt: unable to encrypt buffer - "
"%s : %s\n", gcry_strsource(gcry_error),
gcry_strerror(gcry_error));
thd->to_len = 0;
}
} else {
memcpy(thd->to, thd->from, thd->from_len);
memcpy(thd->to, thd->from,
Copy link

Choose a reason for hiding this comment

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

as per comments for line 561

thd->from_len + XB_CRYPT_HASH_LEN);
}
thd->to_len += XB_CRYPT_HASH_LEN;
}

pthread_mutex_unlock(&thd->data_mutex);
Expand Down
103 changes: 61 additions & 42 deletions storage/innobase/xtrabackup/src/xbcrypt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
#include "xbcrypt.h"
#include <gcrypt.h>

#if !defined(GCRYPT_VERSION_NUMBER) || (GCRYPT_VERSION_NUMBER < 0x010600)
Copy link

Choose a reason for hiding this comment

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

as per comments above, why we requiring to be less than a certain version ?

GCRY_THREAD_OPTION_PTHREAD_IMPL;
#endif

#define XBCRYPT_VERSION "1.0"
#define XBCRYPT_VERSION "1.1"

typedef enum {
RUN_MODE_NONE,
Expand Down Expand Up @@ -56,8 +58,6 @@ static uint encrypt_algos[] = { GCRY_CIPHER_NONE,
static int encrypt_algo = 0;
static int encrypt_mode = GCRY_CIPHER_MODE_CTR;
static uint encrypt_key_len = 0;
static const unsigned char v1_encrypt_iv[] =
"Percona Xtrabackup is Awesome!!!";
static size_t encrypt_iv_len = 0;

static struct my_option my_long_options[] =
Expand Down Expand Up @@ -130,7 +130,9 @@ mode_encrypt(File filein, File fileout);
int
main(int argc, char **argv)
{
#if !defined(GCRYPT_VERSION_NUMBER) || (GCRYPT_VERSION_NUMBER < 0x010600)
Copy link

Choose a reason for hiding this comment

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

as per comments above, why we requiring to be less than a certain version ?

gcry_error_t gcry_error;
#endif
File filein = 0;
File fileout = 0;

Expand All @@ -142,6 +144,7 @@ main(int argc, char **argv)

/* Acording to gcrypt docs (and my testing), setting up the threading
callbacks must be done first, so, lets give it a shot */
#if !defined(GCRYPT_VERSION_NUMBER) || (GCRYPT_VERSION_NUMBER < 0x010600)
Copy link

Choose a reason for hiding this comment

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

as per comments above, why we requiring to be less than a certain version ?

gcry_error = gcry_control(GCRYCTL_SET_THREAD_CBS, &gcry_threads_pthread);
if (gcry_error) {
msg("%s: unable to set libgcrypt thread cbs - "
Expand All @@ -150,6 +153,7 @@ main(int argc, char **argv)
gcry_strerror(gcry_error));
return 1;
}
#endif

/* Version check should be the very first call because it
makes sure that important subsystems are intialized. */
Expand Down Expand Up @@ -303,6 +307,7 @@ mode_decrypt(File filein, File fileout)
xb_rcrypt_result_t result;
gcry_cipher_hd_t cipher_handle;
gcry_error_t gcry_error;
my_bool hash_appended;

if (encrypt_algo != GCRY_CIPHER_NONE) {
gcry_error = gcry_cipher_open(&cipher_handle,
Expand Down Expand Up @@ -338,7 +343,7 @@ mode_decrypt(File filein, File fileout)
/* Walk the encrypted chunks, decrypting them and writing out */
while ((result = xb_crypt_read_chunk(xbcrypt_file, &chunkbuf,
&originalsize, &chunksize,
&ivbuf, &ivsize))
&ivbuf, &ivsize, &hash_appended))
== XB_CRYPT_READ_CHUNK) {

if (encrypt_algo != GCRY_CIPHER_NONE) {
Expand All @@ -352,13 +357,9 @@ mode_decrypt(File filein, File fileout)
}

if (ivsize) {
gcry_error = gcry_cipher_setiv(cipher_handle,
ivbuf,
ivsize);
} else {
gcry_error = gcry_cipher_setiv(cipher_handle,
v1_encrypt_iv,
encrypt_iv_len);
gcry_error = gcry_cipher_setctr(cipher_handle,
ivbuf,
ivsize);
}
if (gcry_error) {
msg("%s:decrypt: unable to set cipher iv - "
Expand All @@ -369,16 +370,10 @@ mode_decrypt(File filein, File fileout)
}

if (decryptbufsize < originalsize) {
if (decryptbufsize) {
decryptbuf = my_realloc(decryptbuf,
originalsize,
MYF(MY_WME));
decryptbufsize = originalsize;
} else {
decryptbuf = my_malloc(originalsize,
MYF(MY_WME));
decryptbufsize = originalsize;
}
decryptbuf = my_realloc(decryptbuf,
originalsize,
MYF(MY_WME | MY_ALLOW_ZERO_PTR));
decryptbufsize = originalsize;
}

/* Try to decrypt it */
Expand All @@ -400,8 +395,29 @@ mode_decrypt(File filein, File fileout)
decryptbuf = chunkbuf;
}

if (hash_appended) {
uchar hash[XB_CRYPT_HASH_LEN];

originalsize -= XB_CRYPT_HASH_LEN;

/* ensure that XB_CRYPT_HASH_LEN is the correct length
of XB_CRYPT_HASH hashing algorithm output */
assert(gcry_md_get_algo_dlen(XB_CRYPT_HASH) ==
XB_CRYPT_HASH_LEN);
gcry_md_hash_buffer(XB_CRYPT_HASH, hash, decryptbuf,
originalsize);
if (memcmp(hash, (char *) decryptbuf + originalsize,
XB_CRYPT_HASH_LEN) != 0) {
msg("%s:%s invalid plaintext hash. "
"Wrong encrytion key specified?\n",
my_progname, __FUNCTION__);
result = XB_CRYPT_READ_ERROR;
goto err;
}
}

/* Write it out */
if (my_write(fileout, decryptbuf, originalsize,
if (my_write(fileout, (const uchar *) decryptbuf, originalsize,
Copy link

Choose a reason for hiding this comment

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

can I presume we're expecting multibyte characters and hence this is why uchar is used ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, this buffer is used in two places. We pass it to libgrypt and we pass it to my_write. format of this buffer is binary, no encodings involved. We output the buffer "as is" after it was processed by libgrypt. Issue is, libgcrypt declares buffers as void * while my_write takes uchar *. So, we need a type cast to make compiler happy.

MYF(MY_WME | MY_NABP))) {
msg("%s:decrypt: unable to write output chunk.\n",
my_progname);
Expand Down Expand Up @@ -456,7 +472,7 @@ mode_encrypt(File filein, File fileout)
{
size_t bytesread;
size_t chunkbuflen;
void *chunkbuf = NULL;
uchar *chunkbuf = NULL;
void *ivbuf = NULL;
size_t encryptbuflen = 0;
size_t encryptedlen = 0;
Expand Down Expand Up @@ -504,11 +520,20 @@ mode_encrypt(File filein, File fileout)
ivbuf = my_malloc(encrypt_iv_len, MYF(MY_FAE));

/* now read in data in chunk size, encrypt and write out */
chunkbuflen = opt_encrypt_chunk_size;
chunkbuf = my_malloc(chunkbuflen, MYF(MY_FAE));
while ((bytesread = my_read(filein, chunkbuf, chunkbuflen,
chunkbuflen = opt_encrypt_chunk_size + XB_CRYPT_HASH_LEN;
chunkbuf = (uchar *) my_malloc(chunkbuflen, MYF(MY_FAE));
Copy link

Choose a reason for hiding this comment

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

MYF(MY_FAE) apologies but I could not figure out what data this returns nor where MY_FAE is set, can you please explain this to me ? (not a priority, this is purely for my education on PXB).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my_malloc is a handy wrapper declared in MySQL source code. It takes flags. The MY_FAE flag signals treat allocation errors as fatal, i.e. crash if memory could not be allocated. It is the best we can do actually.

Copy link

Choose a reason for hiding this comment

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

do you mean MYF() is a declared in the MySQL source, and is used to throw errors if a malloc fails ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in my_sys.h:

#define MY_FAE      8   /* Fatal if any error */
#define MY_WME      16  /* Write message on error */

MY_FAE is to print error message and terminate in case of error, and MY_WME is to only print error message. MYF() is to pass flags.

while ((bytesread = my_read(filein, chunkbuf, opt_encrypt_chunk_size,
MYF(MY_WME))) > 0) {

size_t origbuflen = bytesread + XB_CRYPT_HASH_LEN;

/* ensure that XB_CRYPT_HASH_LEN is the correct length
of XB_CRYPT_HASH hashing algorithm output */
assert(XB_CRYPT_HASH_LEN ==
gcry_md_get_algo_dlen(XB_CRYPT_HASH));
gcry_md_hash_buffer(XB_CRYPT_HASH, chunkbuf + bytesread,
chunkbuf, bytesread);

if (encrypt_algo != GCRY_CIPHER_NONE) {
gcry_error = gcry_cipher_reset(cipher_handle);

Expand All @@ -521,7 +546,7 @@ mode_encrypt(File filein, File fileout)
}

xb_crypt_create_iv(ivbuf, encrypt_iv_len);
gcry_error = gcry_cipher_setiv(cipher_handle,
gcry_error = gcry_cipher_setctr(cipher_handle,
ivbuf,
encrypt_iv_len);

Expand All @@ -533,26 +558,19 @@ mode_encrypt(File filein, File fileout)
continue;
}

if (encryptbuflen < bytesread) {
if (encryptbuflen) {
encryptbuf = my_realloc(encryptbuf,
bytesread,
MYF(MY_WME));
encryptbuflen = bytesread;
} else {
encryptbuf = my_malloc(bytesread,
MYF(MY_WME));
encryptbuflen = bytesread;
}
if (encryptbuflen < origbuflen) {
encryptbuf = my_realloc(encryptbuf, origbuflen,
MYF(MY_WME | MY_ALLOW_ZERO_PTR));
Copy link

Choose a reason for hiding this comment

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

what is MY_WME and MY_ALLOW_ZERO_PTR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MY_WME another flag which tells to print error message and continue in case of memory allocation error. Actually, I cloud replace it with MY_FAE.
MY_ALLOW_ZERO_PTR is a flag telling my_realloc to handle NULL pointer correctly if it has been passed. I.e. I allows to avoid construction if (ptr == NULL) ptr=malloc() else ptr=realloc()

encryptbuflen = origbuflen;
}

gcry_error = gcry_cipher_encrypt(cipher_handle,
encryptbuf,
encryptbuflen,
chunkbuf,
bytesread);
origbuflen);

encryptedlen = bytesread;
encryptedlen = origbuflen;

if (gcry_error) {
msg("%s:encrypt: unable to encrypt chunk - "
Expand All @@ -563,11 +581,12 @@ mode_encrypt(File filein, File fileout)
goto err;
}
} else {
encryptedlen = bytesread;
encryptedlen = origbuflen;
encryptbuf = chunkbuf;
}

if (xb_crypt_write_chunk(xbcrypt_file, encryptbuf, bytesread,
if (xb_crypt_write_chunk(xbcrypt_file, encryptbuf,
bytesread + XB_CRYPT_HASH_LEN,
encryptedlen, ivbuf, encrypt_iv_len)) {
msg("%s:encrypt: abcrypt_write_chunk() failed.\n",
my_progname);
Expand Down
9 changes: 7 additions & 2 deletions storage/innobase/xtrabackup/src/xbcrypt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,14 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
#include "common.h"

#define XB_CRYPT_CHUNK_MAGIC1 "XBCRYP01"
#define XB_CRYPT_CHUNK_MAGIC2 "XBCRYP02" /* must be same size as ^^ */
#define XB_CRYPT_CHUNK_MAGIC2 "XBCRYP02"
#define XB_CRYPT_CHUNK_MAGIC3 "XBCRYP03" /* must be same size as ^^ */
#define XB_CRYPT_CHUNK_MAGIC_CURRENT XB_CRYPT_CHUNK_MAGIC3
#define XB_CRYPT_CHUNK_MAGIC_SIZE (sizeof(XB_CRYPT_CHUNK_MAGIC1)-1)

#define XB_CRYPT_HASH GCRY_MD_SHA256
#define XB_CRYPT_HASH_LEN 32
Copy link

Choose a reason for hiding this comment

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

XB_CRYPT_HASH_LEN note this is the digest length, and not the hexdigest which would be 64 (my notes, no review needed here)


/******************************************************************************
Write interface */
typedef struct xb_wcrypt_struct xb_wcrypt_t;
Expand Down Expand Up @@ -66,7 +71,7 @@ typedef enum {

xb_rcrypt_result_t xb_crypt_read_chunk(xb_rcrypt_t *crypt, void **buf,
size_t *olen, size_t *elen, void **iv,
size_t *ivlen);
size_t *ivlen, my_bool *hash_appended);

int xb_crypt_read_close(xb_rcrypt_t *crypt);

Expand Down
Loading