Skip to content

fix: check priv_key_bits only for relevant private key types #19103

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 5 additions & 6 deletions ext/openssl/openssl_backend_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1437,17 +1437,16 @@ static const char *php_openssl_get_evp_pkey_name(int key_type) {

EVP_PKEY *php_openssl_generate_private_key(struct php_x509_request * req)
{
if (req->priv_key_bits < MIN_KEY_LENGTH) {
php_error_docref(NULL, E_WARNING, "Private key length must be at least %d bits, configured to %d",
MIN_KEY_LENGTH, req->priv_key_bits);
return NULL;
}

int type = php_openssl_get_evp_pkey_type(req->priv_key_type);
if (type < 0) {
php_error_docref(NULL, E_WARNING, "Unsupported private key type");
return NULL;
}
if ((type == EVP_PKEY_RSA || type == EVP_PKEY_DSA || type == EVP_PKEY_DH) && req->priv_key_bits < MIN_KEY_LENGTH) {
php_error_docref(NULL, E_WARNING, "Private key length must be at least %d bits, configured to %d",
MIN_KEY_LENGTH, req->priv_key_bits);
return NULL;
}
const char *name = php_openssl_get_evp_pkey_name(req->priv_key_type);

int egdsocket, seeded;
Expand Down
59 changes: 59 additions & 0 deletions ext/openssl/tests/openssl_key_type_bits_enforcement.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
--TEST--
openssl: test key type and bit length enforcement in php_openssl_generate_private_key
--EXTENSIONS--
openssl
--SKIPIF--
<?php
if (!defined("OPENSSL_KEYTYPE_RSA")) die("skip RSA disabled");
if (!defined("OPENSSL_KEYTYPE_DSA")) die("skip DSA disabled");
if (!defined("OPENSSL_KEYTYPE_DH")) die("skip DH disabled");
if (!defined("OPENSSL_KEYTYPE_EC")) die("skip EC disabled");
?>
--FILE--
<?php
function test_key($type, $bits = null) {
$args = [];
switch ($type) {
case OPENSSL_KEYTYPE_RSA:
$args['private_key_type'] = OPENSSL_KEYTYPE_RSA;
if ($bits !== null) $args['private_key_bits'] = $bits;
break;
case OPENSSL_KEYTYPE_DSA:
$args['private_key_type'] = OPENSSL_KEYTYPE_DSA;
if ($bits !== null) $args['private_key_bits'] = $bits;
break;
case OPENSSL_KEYTYPE_DH:
$args['private_key_type'] = OPENSSL_KEYTYPE_DH;
if ($bits !== null) $args['private_key_bits'] = $bits;
break;
case OPENSSL_KEYTYPE_EC:
$args['curve_name'] = 'prime256v1';
$args['private_key_type'] = OPENSSL_KEYTYPE_EC;
break;
}
$key = @openssl_pkey_new($args);
var_dump($key !== false);
}

// Should fail: RSA, DSA, DH with bits < MIN_KEY_LENGTH
foreach ([OPENSSL_KEYTYPE_RSA, OPENSSL_KEYTYPE_DSA, OPENSSL_KEYTYPE_DH] as $type) {
test_key($type, 256); // too short
}
// Should succeed: RSA, DSA, DH with bits >= MIN_KEY_LENGTH
foreach ([OPENSSL_KEYTYPE_RSA, OPENSSL_KEYTYPE_DSA, OPENSSL_KEYTYPE_DH] as $type) {
test_key($type, 1024); // valid, but small to keep test fast
}
// Should succeed: EC with curve only
test_key(OPENSSL_KEYTYPE_EC);
// Should succeed: EC with bits too low
test_key(OPENSSL_KEYTYPE_EC, 256);
?>
--EXPECT--
bool(false)
bool(false)
bool(false)
bool(true)
bool(true)
bool(true)
bool(true)
bool(true)