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.

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

EVP_PKEY_new_raw_private_key(NID_ED25519) does not return fail on short key #17017

Closed
pemensik opened this issue Nov 12, 2021 · 6 comments
Closed
Labels
triaged: bug The issue/pr is/fixes a bug

Comments

@pemensik
Copy link

Detected by failures on strongswan test suite, reported on strongswan/strongswan#753.

// compile by: gcc -Wall -g test.c -o test $(pkg-config --libs --cflags openssl)
#include <openssl/evp.h>

int main(int argc, char *argv[])
{
        unsigned char data[] = { 234, };
        EVP_PKEY *p;
        p = EVP_PKEY_new_raw_private_key(NID_ED25519, NULL, data, 1 /*sizeof(data)*/);
        printf("PKEY: %p\n", p);
        return 0;
}

Above simple code raises assertion failure on openssl 3.0.0, but returned just NULL on openssl 1.1. It is deliberately passed there with too short data. I think it should not crash whole program but return error, just like previous versions.

test: malloc.c:2515: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.
```.
@pemensik pemensik added the issue: bug report The issue was opened to report a bug label Nov 12, 2021
@pemensik
Copy link
Author

Related to MR #15643.

@mattcaswell mattcaswell added triaged: bug The issue/pr is/fixes a bug and removed issue: bug report The issue was opened to report a bug labels Nov 15, 2021
@mattcaswell
Copy link
Member

Hmm. This is curious. In fact the code is attempting to fail at this point. It has detected the short key and is trying to add an error to the error stack indicating that the setup of the key has failed. The assertion failure is not coming out of OpenSSL at all, but seems to come from libc!! My first thought was that some kind of memory corruption was triggering this - but recompiling with address sanitizer switched on reveals no problems - and in fact the assertion failure goes away completely and the reproducer comes back with the NULL response as expected.

Is this a possible libc bug!!??

@mattcaswell
Copy link
Member

Is this a possible libc bug!!??

Nope. This really is an OpenSSL problem.

@mattcaswell
Copy link
Member

A memory corruption is occurring. I'm not sure why address sanitizer didn't pick it up...

mattcaswell added a commit to mattcaswell/openssl that referenced this issue Nov 15, 2021
If an ECX key is created and the private key is too short, a fromdata
call would create the key, and then later detect the error and report it
after freeing the key. However freeing the key was calling
OPENSSL_secure_clear_free() and assuming that the private key was of the
correct length. If it was actually too short this will write over memory
that it shouldn't.

Fixes openssl#17017
@mattcaswell
Copy link
Member

Fix for this in #17041

@pemensik
Copy link
Author

Thank you for a quick solution. Requested backport in RHBZ #2023671 for Fedora.

openssl-machine pushed a commit that referenced this issue Nov 16, 2021
If an ECX key is created and the private key is too short, a fromdata
call would create the key, and then later detect the error and report it
after freeing the key. However freeing the key was calling
OPENSSL_secure_clear_free() and assuming that the private key was of the
correct length. If it was actually too short this will write over memory
that it shouldn't.

Fixes #17017

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #17041)

(cherry picked from commit 50938ae)
ramikhaldi pushed a commit to ramikhaldi/openssl that referenced this issue Nov 21, 2021
If an ECX key is created and the private key is too short, a fromdata
call would create the key, and then later detect the error and report it
after freeing the key. However freeing the key was calling
OPENSSL_secure_clear_free() and assuming that the private key was of the
correct length. If it was actually too short this will write over memory
that it shouldn't.

Fixes openssl#17017

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#17041)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants