Nothing Special   »   [go: up one dir, main page]

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

openssl_get_publickey(): With public key PEM, results in error:0480006C:PEM routines::no start line #11054

Closed
famoser opened this issue Apr 10, 2023 · 0 comments · Fixed by ThePHPF/thephp.foundation#90

Comments

@famoser
Copy link
Contributor
famoser commented Apr 10, 2023

Description

The following code:

<?php
$publicKeyPem = <<EOD
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvYH14fT4DPgyffkDOrHt
x0q+rxclB48h2ykgbR3QyDG2d7hMSXjtqEseO/iR1FdAv7UevIKyHFbHpJilOIwo
mEqQNxUQCWdZsWhv7ZVfG8UUgki7LKMGPruJM97vteBS101hSCaCQz+zTyVyP8Uy
nqx5zlPmcBUA92gAFfSCa+tm/lR2BY5g/20mZX/lMY0xXV1iLhfdK6RgJYXX2SdH
YR/01IgmjgTfIp7gX+xixDgGZuZY++jo8C52udFkCf5vxyG4Ed57vRfCLFOPfeY4
r3i0Jiply65zSo8y/6KxudRtmGOfV2qb2EsMTW9PaLs3+rnhhiYBM/nR4V5ux6u6
DwIDAQAB
-----END PUBLIC KEY-----
EOD;
openssl_get_publickey($publicKeyPem);
var_dump(openssl_error_string());

Resulted in this output:

string(54) "error:0906D06C:PEM routines:PEM_read_bio:no start line"

But I expected this output instead:

bool(false)

Note that openssl_get_publickey succeeded; the error is there nonetheless. More elaborate test cases here: https://github.com/famoser/polyas-verification/blob/a28f16437ab695548b3cb301258f87badf7aee69/tests/Utils/OpenSSLBugs.php

Analysis

The error happens when the PEM contains a public key, as it will be first tried to be parsed as a certificate (see here). The parsing as a certificate fails, which then leads to a corresponding error tracked by PHP with the next call to php_openssl_store_errors().

As PHP handles the case where the certificate cannot be parsed, consequentially this error should never reach the user. Else the user-code would need to ignore this error somehow, which requires knowledge of implementation details when this is safe to do, and when not.

Solution

I see two ways how to fix the issue.

Prevent error being stored

The error should never reach PHP's OPENSSL_G(errors) buffer. Prevent php_openssl_x509_from_str to store the errors, and discard them in php_openssl_pkey_from_zval.

Pseudo-code:

/* {{{ php_openssl_discard_errors */
void php_openssl_discard_errors(void)
{
	int error_code;
	while ((error_code = ERR_get_error()));
}
/* }}} */

static EVP_PKEY *php_openssl_pkey_from_zval(
		zval *val, int public_key, char *passphrase, size_t passphrase_len, uint32_t arg_num)
{
  // ...
  php_openssl_store_errors();
  cert = php_openssl_x509_from_str(Z_STR_P(val), arg_num, false, NULL, true);
  if (cert) {
    free_cert = 1;
  } else {
    php_openssl_discard_errors();
    // ...
  }
}

static X509 *php_openssl_x509_from_str(
		zend_string *cert_str, uint32_t arg_num, bool is_from_array, const char *option_name, const int store_errors) {
  // ... (4 times)
  if (store_errors) {
    php_openssl_store_errors();
  }
  // ...
}

Mark & Revert error

Similar to how openSSL error buffer works, introduce an option to mark the buffer at a specific state, and allow to revert to this state.

Pseudo-code:

/* {{{ php_openssl_errors_set_mark */
void php_openssl_errors_set_mark(void) {
	if (!OPENSSL_G(errors)) {
		return;
	}

	OPENSSL_G(errors_mark) = pecalloc(1, sizeof(struct php_openssl_errors), 1);
	memcpy(OPENSSL_G(errors), OPENSSL_G(errors_mark), sizeof(struct php_openssl_errors));
}
/* }}} */

/* {{{ php_openssl_errors_restore_mark */
void php_openssl_errors_restore_mark(void) {
	struct php_openssl_errors *errors;

	if (!OPENSSL_G(errors)) {
		return;
	}

	if (!OPENSSL_G(errors_mark)) {
		errors = OPENSSL_G(errors);
		errors->top = 0;
		errors->bottom = 0;
	} else {
		memcpy(OPENSSL_G(errors_mark), OPENSSL_G(errors), sizeof(struct php_openssl_errors));
	}
}
/* }}} */

static EVP_PKEY *php_openssl_pkey_from_zval(
		zval *val, int public_key, char *passphrase, size_t passphrase_len, uint32_t arg_num)
{
  // ...
  php_openssl_errors_set_mark();
  cert = php_openssl_x509_from_str(Z_STR_P(val), arg_num, false, NULL);

  if (cert) {
    free_cert = 1;
  } else {
    php_openssl_errors_restore_mark();
    // ...
  }
}

I'll create a PR shortly for this second version. I feel it is less intrusive and simpler to understand. Let me know if you have a different option on this, and I'll update the PR.

PHP Version

PHP 8.0.28, PHP 8.1.17, PHP 8.2.4

Operating System

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants