Skip to content

Commit

Permalink
Fixing errors with OpenSSL 1.0/1.1 (#105)
Browse files Browse the repository at this point in the history
* Fixing errors with OpenSSL 1.0/1.1

Issue: there's a slight behavior difference between OpenSSL 1.1 and 3.0
in regards to padding, and how the EVP encryption/decryption functions
exactly behave.

Our code also incorrectly used these functions until now, which by chance
worked correctly with 3.0, but caused decryption issues with 1.0/1.1.

This commit:

* Fixes the issue
* Adds a few related assertions
* Adds an Ubuntu 20.04 (OpenSSL 1.1) github runner to verify that everything
  works correctly with 1.1.

* Move padding initialziation to the correct location

* enable assertions in ssl 1.1 test

* also fix padding call at the other encryption function

* enabling cassert for normal 22.04 build

* fixing additional comments
  • Loading branch information
dutow authored Feb 5, 2024
1 parent dbc21ac commit 7109387
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 11 deletions.
92 changes: 92 additions & 0 deletions .github/workflows/postgresql-16-src-make-ssl11.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
name: postgresql-16-src-make-ssl11
on: [pull_request, workflow_dispatch]

jobs:
build:
name: pg-16-src-make-test-ssl11
runs-on: ubuntu-20.04
steps:


- name: Remove old postgres
run: |
sudo apt purge postgresql-client-common postgresql-common \
postgresql postgresql*
sudo rm -rf /var/lib/postgresql /var/log/postgresql /etc/postgresql \
/usr/lib/postgresql /usr/include/postgresql /usr/share/postgresql \
/etc/postgresql
sudo rm -f /usr/bin/pg_config
- name: Install dependencies
run: |
sudo apt-get update
sudo apt-get install -y libreadline6-dev systemtap-sdt-dev \
zlib1g-dev libssl-dev libpam0g-dev bison flex \
libipc-run-perl -y docbook-xsl docbook-xsl libxml2 libxml2-utils \
libxml2-dev libxslt-dev xsltproc libkrb5-dev libldap2-dev \
libsystemd-dev gettext tcl-dev libperl-dev pkg-config \
libselinux1-dev python3-dev \
uuid-dev liblz4-dev libjson-c-dev libcurl4-openssl-dev
sudo /usr/bin/perl -MCPAN -e 'install IPC::RUN'
sudo /usr/bin/perl -MCPAN -e 'install Text::Trim'
- name: Clone postgres repository
uses: actions/checkout@v2
with:
repository: 'postgres/postgres'
ref: 'a81e5516fa4bc53e332cb35eefe231147c0e1749'
path: 'src'

- name: Clone postgres-tde-ext repository
uses: actions/checkout@v2
with:
path: 'src/contrib/postgres-tde-ext'

- name: Create pgsql dir
run: mkdir -p /opt/pgsql

- name: Build postgres
run: |
./configure --with-openssl --enable-tap-tests=no --enable-cassert
make -j
sudo make install
working-directory: src

- name: Build postgres-tde-ext
run: |
./configure
make -j
sudo make install
working-directory: src/contrib/postgres-tde-ext

- name: Start postgresql cluster with pg_tde
run: |
export PATH="/usr/local/pgsql/bin:$PATH"
sudo cp /usr/local/pgsql/bin/pg_config /usr/bin
initdb -D /opt/pgsql/data
echo "shared_preload_libraries = 'pg_tde'" >> \
/opt/pgsql/data/postgresql.conf
echo "pg_tde.keyringConfigFile = '/tmp/keyring.json'" >> \
/opt/pgsql/data/postgresql.conf
cp src/contrib/postgres-tde-ext/keyring.json /tmp/keyring.json
pg_ctl -D /opt/pgsql/data -l logfile start
- name: Test postgres-tde-ext
run: |
make installcheck
working-directory: src/contrib/postgres-tde-ext

- name: Report on test fail
uses: actions/upload-artifact@v2
if: ${{ failure() }}
with:
name: Regressions diff and postgresql log
path: |
src/contrib/postgres-tde-ext/regression.diffs
logfile
retention-days: 3

- name: Report on test fail 2
if: ${{ failure() }}
run: |
cat src/contrib/postgres-tde-ext/regression.diffs
2 changes: 1 addition & 1 deletion .github/workflows/postgresql-16-src-make.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:

- name: Build postgres
run: |
./configure --with-openssl --enable-tap-tests=no
./configure --with-openssl --enable-tap-tests=no --enable-cassert
make -j
sudo make install
working-directory: src
Expand Down
28 changes: 18 additions & 10 deletions src/encryption/enc_aes.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,13 @@ void AesInit(void)

// TODO: a few things could be optimized in this. It's good enough for a prototype.
static void
AesRun2(EVP_CIPHER_CTX** ctxPtr, int enc, const unsigned char* key, const unsigned char* iv, const unsigned char* in, int in_len, unsigned char* out, int* out_len)
AesRunCtr(EVP_CIPHER_CTX** ctxPtr, int enc, const unsigned char* key, const unsigned char* iv, const unsigned char* in, int in_len, unsigned char* out, int* out_len)
{
if (*ctxPtr == NULL)
{
*ctxPtr = EVP_CIPHER_CTX_new();
EVP_CIPHER_CTX_init(*ctxPtr);

EVP_CIPHER_CTX_set_padding(*ctxPtr, 0);

if(EVP_CipherInit_ex(*ctxPtr, cipher2, NULL, key, iv, enc) == 0)
{
#ifdef FRONTEND
Expand All @@ -86,6 +84,8 @@ AesRun2(EVP_CIPHER_CTX** ctxPtr, int enc, const unsigned char* key, const unsign

return;
}

EVP_CIPHER_CTX_set_padding(*ctxPtr, 0);
}

if(EVP_CipherUpdate(*ctxPtr, out, out_len, in, in_len) == 0)
Expand All @@ -100,14 +100,13 @@ AesRun2(EVP_CIPHER_CTX** ctxPtr, int enc, const unsigned char* key, const unsign
}
}

static void AesRun(int enc, const unsigned char* key, const unsigned char* iv, const unsigned char* in, int in_len, unsigned char* out, int* out_len)
static void AesRunCbc(int enc, const unsigned char* key, const unsigned char* iv, const unsigned char* in, int in_len, unsigned char* out, int* out_len)
{
int out_len_final = 0;
EVP_CIPHER_CTX* ctx = NULL;
ctx = EVP_CIPHER_CTX_new();
EVP_CIPHER_CTX_init(ctx);

EVP_CIPHER_CTX_set_padding(ctx, 0);

if(EVP_CipherInit_ex(ctx, cipher, NULL, key, iv, enc) == 0)
{
#ifdef FRONTEND
Expand All @@ -119,6 +118,9 @@ static void AesRun(int enc, const unsigned char* key, const unsigned char* iv, c
goto cleanup;
}

EVP_CIPHER_CTX_set_padding(ctx, 0);
Assert(in_len % cipher_block_size == 0);

if(EVP_CipherUpdate(ctx, out, out_len, in, in_len) == 0)
{
#ifdef FRONTEND
Expand All @@ -130,7 +132,7 @@ static void AesRun(int enc, const unsigned char* key, const unsigned char* iv, c
goto cleanup;
}

if(EVP_CipherFinal_ex(ctx, out, out_len) == 0)
if(EVP_CipherFinal_ex(ctx, out + *out_len, &out_len_final) == 0)
{
#ifdef FRONTEND
fprintf(stderr, "ERROR: EVP_CipherFinal_ex failed. OpenSSL error: %s\n", ERR_error_string(ERR_get_error(), NULL));
Expand All @@ -141,19 +143,25 @@ static void AesRun(int enc, const unsigned char* key, const unsigned char* iv, c
goto cleanup;
}

/* We encrypt one block (16 bytes)
* Our expectation is that the result should also be 16 bytes, without any additional padding
*/
*out_len += out_len_final;
Assert(in_len == *out_len);

cleanup:
EVP_CIPHER_CTX_cleanup(ctx);
EVP_CIPHER_CTX_free(ctx);
}

void AesEncrypt(const unsigned char* key, const unsigned char* iv, const unsigned char* in, int in_len, unsigned char* out, int* out_len)
{
AesRun(1, key, iv, in, in_len, out, out_len);
AesRunCbc(1, key, iv, in, in_len, out, out_len);
}

void AesDecrypt(const unsigned char* key, const unsigned char* iv, const unsigned char* in, int in_len, unsigned char* out, int* out_len)
{
AesRun(0, key, iv, in, in_len, out, out_len);
AesRunCbc(0, key, iv, in, in_len, out, out_len);
}

/*
Expand Down Expand Up @@ -181,6 +189,6 @@ void Aes128EncryptedZeroBlocks(void* ctxPtr, const unsigned char* key, uint64_t
}
}

AesRun2(ctxPtr, 1, key, iv, data, dataLen, out, &outLen);
AesRunCtr(ctxPtr, 1, key, iv, data, dataLen, out, &outLen);
Assert(outLen == dataLen);
}

0 comments on commit 7109387

Please sign in to comment.