Skip to content

Commit

Permalink
Revert all MySQL auth related changes
Browse files Browse the repository at this point in the history 
Per bug #76651 these changes do not appear to work correctly in
some cases. As no immediate fix seems to be forthcoming, I'm
reverting these changes.

Revert "Fixed invalid free introduced by d6e81f0 (avoid keeping "invalid" pointer)"

This reverts commit 11507c0.

Revert "Fix mysqlnd build without openssl"

This reverts commit 6c9db02.

Revert "Fix VC compilation as variable size array is not supported"

This reverts commit f96df64.

Revert "Fix MySQL 8 auth"

This reverts commit d6e81f0.
  • Loading branch information
@nikic
nikic committed Sep 4, 2018
1 parent f1f39d7 commit 03740ef
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 425 deletions.
293 changes: 7 additions & 286 deletions ext/mysqlnd/mysqlnd_auth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ mysqlnd_run_authentication(
}
}


{
zend_uchar * switch_to_auth_protocol_data = NULL;
size_t switch_to_auth_protocol_data_len = 0;
Expand All @@ -114,11 +113,10 @@ mysqlnd_run_authentication(
DBG_INF_FMT("salt(%d)=[%.*s]", plugin_data_len, plugin_data_len, plugin_data);
/* The data should be allocated with malloc() */
if (auth_plugin) {
scrambled_data = auth_plugin->methods.get_auth_data(
NULL, &scrambled_data_len, conn, user, passwd,
passwd_len, plugin_data, plugin_data_len,
session_options, conn->protocol_frame_codec->data,
mysql_flags);
scrambled_data =
auth_plugin->methods.get_auth_data(NULL, &scrambled_data_len, conn, user, passwd, passwd_len,
plugin_data, plugin_data_len, session_options,
conn->protocol_frame_codec->data, mysql_flags);
}

if (conn->error_info->error_no) {
Expand All @@ -129,7 +127,6 @@ mysqlnd_run_authentication(
charset_no,
first_call,
requested_protocol,
auth_plugin, plugin_data, plugin_data_len,
scrambled_data, scrambled_data_len,
&switch_to_auth_protocol, &switch_to_auth_protocol_len,
&switch_to_auth_protocol_data, &switch_to_auth_protocol_data_len
Expand Down Expand Up @@ -251,9 +248,6 @@ mysqlnd_auth_handshake(MYSQLND_CONN_DATA * conn,
unsigned int server_charset_no,
zend_bool use_full_blown_auth_packet,
const char * const auth_protocol,
struct st_mysqlnd_authentication_plugin * auth_plugin,
const zend_uchar * const orig_auth_plugin_data,
const size_t orig_auth_plugin_data_len,
const zend_uchar * const auth_plugin_data,
const size_t auth_plugin_data_len,
char ** switch_to_auth_protocol,
Expand Down Expand Up @@ -324,11 +318,6 @@ mysqlnd_auth_handshake(MYSQLND_CONN_DATA * conn,
conn->charset = mysqlnd_find_charset_nr(auth_packet->charset_no);
}

if (auth_plugin && auth_plugin->methods.handle_server_response) {
auth_plugin->methods.handle_server_response(auth_plugin, conn,
orig_auth_plugin_data, orig_auth_plugin_data_len, passwd, passwd_len);
}

if (FAIL == PACKET_READ(auth_resp_packet) || auth_resp_packet->response_code >= 0xFE) {
if (auth_resp_packet->response_code == 0xFE) {
/* old authentication with new server !*/
Expand Down Expand Up @@ -624,8 +613,7 @@ static struct st_mysqlnd_authentication_plugin mysqlnd_native_auth_plugin =
}
},
{/* methods */
mysqlnd_native_auth_get_auth_data,
NULL
mysqlnd_native_auth_get_auth_data
}
};

Expand Down Expand Up @@ -674,8 +662,7 @@ static struct st_mysqlnd_authentication_plugin mysqlnd_pam_authentication_plugin
}
},
{/* methods */
mysqlnd_pam_auth_get_auth_data,
NULL
mysqlnd_pam_auth_get_auth_data
}
};

Expand Down Expand Up @@ -859,283 +846,17 @@ static struct st_mysqlnd_authentication_plugin mysqlnd_sha256_authentication_plu
}
},
{/* methods */
mysqlnd_sha256_auth_get_auth_data,
NULL
mysqlnd_sha256_auth_get_auth_data
}
};
#endif

/*************************************** CACHING SHA2 Password *******************************/

#undef L64

#include "ext/hash/php_hash.h"
#include "ext/hash/php_hash_sha.h"

#define SHA256_LENGTH 32

/* {{{ php_mysqlnd_scramble_sha2 */
void php_mysqlnd_scramble_sha2(zend_uchar * const buffer, const zend_uchar * const scramble, const zend_uchar * const password, const size_t password_len)
{
PHP_SHA256_CTX context;
zend_uchar sha1[SHA256_LENGTH];
zend_uchar sha2[SHA256_LENGTH];

/* Phase 1: hash password */
PHP_SHA256Init(&context);
PHP_SHA256Update(&context, password, password_len);
PHP_SHA256Final(sha1, &context);

/* Phase 2: hash sha1 */
PHP_SHA256Init(&context);
PHP_SHA256Update(&context, (zend_uchar*)sha1, SHA256_LENGTH);
PHP_SHA256Final(sha2, &context);

/* Phase 3: hash scramble + sha2 */
PHP_SHA256Init(&context);
PHP_SHA256Update(&context, (zend_uchar*)sha2, SHA256_LENGTH);
PHP_SHA256Update(&context, scramble, SCRAMBLE_LENGTH);
PHP_SHA256Final(buffer, &context);

/* let's crypt buffer now */
php_mysqlnd_crypt(buffer, (const zend_uchar *)sha1, (const zend_uchar *)buffer, SHA256_LENGTH);
}
/* }}} */


/* {{{ mysqlnd_native_auth_get_auth_data */
static zend_uchar *
mysqlnd_caching_sha2_get_auth_data(struct st_mysqlnd_authentication_plugin * self,
size_t * auth_data_len,
MYSQLND_CONN_DATA * conn, const char * const user, const char * const passwd,
const size_t passwd_len, zend_uchar * auth_plugin_data, size_t auth_plugin_data_len,
const MYSQLND_SESSION_OPTIONS * const session_options,
const MYSQLND_PFC_DATA * const pfc_data,
zend_ulong mysql_flags
)
{
zend_uchar * ret = NULL;
DBG_ENTER("mysqlnd_caching_sha2_get_auth_data");
DBG_INF_FMT("salt(%d)=[%.*s]", auth_plugin_data_len, auth_plugin_data_len, auth_plugin_data);
*auth_data_len = 0;

DBG_INF("First auth step: send hashed password");
/* copy scrambled pass*/
if (passwd && passwd_len) {
ret = malloc(SHA256_LENGTH + 1);
*auth_data_len = SHA256_LENGTH;
php_mysqlnd_scramble_sha2((zend_uchar*)ret, auth_plugin_data, (zend_uchar*)passwd, passwd_len);
ret[SHA256_LENGTH] = '\0';
DBG_INF_FMT("hash(%d)=[%.*s]", *auth_data_len, *auth_data_len, ret);
}

DBG_RETURN(ret);
}
/* }}} */

#ifdef MYSQLND_HAVE_SSL
static RSA *
mysqlnd_caching_sha2_get_key(MYSQLND_CONN_DATA *conn)
{
RSA * ret = NULL;
const MYSQLND_PFC_DATA * const pfc_data = conn->protocol_frame_codec->data;
const char * fname = (pfc_data->sha256_server_public_key && pfc_data->sha256_server_public_key[0] != '\0')?
pfc_data->sha256_server_public_key:
MYSQLND_G(sha256_server_public_key);
php_stream * stream;
DBG_ENTER("mysqlnd_cached_sha2_get_key");
DBG_INF_FMT("options_s256_pk=[%s] MYSQLND_G(sha256_server_public_key)=[%s]",
pfc_data->sha256_server_public_key? pfc_data->sha256_server_public_key:"n/a",
MYSQLND_G(sha256_server_public_key)? MYSQLND_G(sha256_server_public_key):"n/a");
if (!fname || fname[0] == '\0') {
MYSQLND_PACKET_CACHED_SHA2_RESULT *req_packet = NULL;
MYSQLND_PACKET_SHA256_PK_REQUEST_RESPONSE *pk_resp_packet = NULL;

do {
DBG_INF("requesting the public key from the server");
req_packet = conn->payload_decoder_factory->m.get_cached_sha2_result_packet(conn->payload_decoder_factory, FALSE);
pk_resp_packet = conn->payload_decoder_factory->m.get_sha256_pk_request_response_packet(conn->payload_decoder_factory, FALSE);
req_packet->request = 1;

if (! PACKET_WRITE(req_packet)) {
DBG_ERR_FMT("Error while sending public key request packet");
php_error(E_WARNING, "Error while sending public key request packet. PID=%d", getpid());
SET_CONNECTION_STATE(&conn->state, CONN_QUIT_SENT);
break;
}
if (FAIL == PACKET_READ(pk_resp_packet) || NULL == pk_resp_packet->public_key) {
DBG_ERR_FMT("Error while receiving public key");
php_error(E_WARNING, "Error while receiving public key. PID=%d", getpid());
SET_CONNECTION_STATE(&conn->state, CONN_QUIT_SENT);
break;
}
DBG_INF_FMT("Public key(%d):\n%s", pk_resp_packet->public_key_len, pk_resp_packet->public_key);
/* now extract the public key */
{
BIO * bio = BIO_new_mem_buf(pk_resp_packet->public_key, pk_resp_packet->public_key_len);
ret = PEM_read_bio_RSA_PUBKEY(bio, NULL, NULL, NULL);
BIO_free(bio);
}
} while (0);
PACKET_FREE(req_packet);
PACKET_FREE(pk_resp_packet);

DBG_INF_FMT("ret=%p", ret);
DBG_RETURN(ret);

SET_CLIENT_ERROR(conn->error_info, CR_UNKNOWN_ERROR, UNKNOWN_SQLSTATE,
"caching_sha2_server_public_key is not set for the connection or as mysqlnd.sha256_server_public_key");
DBG_ERR("server_public_key is not set");
DBG_RETURN(NULL);
} else {
zend_string * key_str;
DBG_INF_FMT("Key in a file. [%s]", fname);
stream = php_stream_open_wrapper((char *) fname, "rb", REPORT_ERRORS, NULL);

if (stream) {
if ((key_str = php_stream_copy_to_mem(stream, PHP_STREAM_COPY_ALL, 0)) != NULL) {
BIO * bio = BIO_new_mem_buf(ZSTR_VAL(key_str), ZSTR_LEN(key_str));
ret = PEM_read_bio_RSA_PUBKEY(bio, NULL, NULL, NULL);
BIO_free(bio);
DBG_INF("Successfully loaded");
DBG_INF_FMT("Public key:%*.s", ZSTR_LEN(key_str), ZSTR_VAL(key_str));
zend_string_release(key_str);
}
php_stream_close(stream);
}
}
DBG_RETURN(ret);

}
#endif


/* {{{ mysqlnd_caching_sha2_get_key */
static size_t
mysqlnd_caching_sha2_get_and_use_key(MYSQLND_CONN_DATA *conn,
const zend_uchar * auth_plugin_data, size_t auth_plugin_data_len,
unsigned char **crypted,
const char * const passwd,
const size_t passwd_len)
{
#ifdef MYSQLND_HAVE_SSL
static RSA *server_public_key;
server_public_key = mysqlnd_caching_sha2_get_key(conn);

DBG_ENTER("mysqlnd_caching_sha2_get_and_use_key(");

if (server_public_key) {
int server_public_key_len;
char xor_str[passwd_len + 1];
memcpy(xor_str, passwd, passwd_len);
xor_str[passwd_len] = '\0';
mysqlnd_xor_string(xor_str, passwd_len, (char *) auth_plugin_data, auth_plugin_data_len);

server_public_key_len = RSA_size(server_public_key);
/*
Because RSA_PKCS1_OAEP_PADDING is used there is a restriction on the passwd_len.
RSA_PKCS1_OAEP_PADDING is recommended for new applications. See more here:
http://www.openssl.org/docs/crypto/RSA_public_encrypt.html
*/
if ((size_t) server_public_key_len - 41 <= passwd_len) {
/* password message is to long */
SET_CLIENT_ERROR(conn->error_info, CR_UNKNOWN_ERROR, UNKNOWN_SQLSTATE, "password is too long");
DBG_ERR("password is too long");
DBG_RETURN(0);
}

*crypted = emalloc(server_public_key_len);
RSA_public_encrypt(passwd_len + 1, (zend_uchar *) xor_str, *crypted, server_public_key, RSA_PKCS1_OAEP_PADDING);
DBG_RETURN(server_public_key_len);
}
DBG_RETURN(0);
#else
DBG_ENTER("mysqlnd_caching_sha2_get_and_use_key(");
php_error_docref(NULL, E_WARNING, "PHP was built without openssl extension, can't send password encrypted");
DBG_RETURN(0);
#endif
}
/* }}} */

/* {{{ mysqlnd_native_auth_get_auth_data */
static void
mysqlnd_caching_sha2_handle_server_response(struct st_mysqlnd_authentication_plugin *self,
MYSQLND_CONN_DATA * conn,
const zend_uchar * auth_plugin_data, size_t auth_plugin_data_len,
const char * const passwd,
const size_t passwd_len)
{
DBG_ENTER("mysqlnd_caching_sha2_handle_server_response");
MYSQLND_PACKET_CACHED_SHA2_RESULT *result_packet;
result_packet = conn->payload_decoder_factory->m.get_cached_sha2_result_packet(conn->payload_decoder_factory, FALSE);

if (FAIL == PACKET_READ(result_packet)) {
DBG_VOID_RETURN;
}

switch (result_packet->response_code) {
case 3:
DBG_INF("fast path suceeded");
PACKET_FREE(result_packet);
DBG_VOID_RETURN;
case 4:
if (conn->vio->data->ssl || conn->unix_socket.s) {
DBG_INF("fast path failed, doing full auth via SSL");
result_packet->password = (zend_uchar *)passwd;
result_packet->password_len = passwd_len + 1;
PACKET_WRITE(result_packet);
} else {
DBG_INF("fast path failed, doing full auth without SSL");
result_packet->password_len = mysqlnd_caching_sha2_get_and_use_key(conn, auth_plugin_data, auth_plugin_data_len, &result_packet->password, passwd, passwd_len);
PACKET_WRITE(result_packet);
efree(result_packet->password);
}
PACKET_FREE(result_packet);
DBG_VOID_RETURN;
case 2:
// The server tried to send a key, which we didn't expect
// fall-through
default:
php_error_docref(NULL, E_WARNING, "Unexpected server respose while doing caching_sha2 auth: %i", result_packet->response_code);
}

PACKET_FREE(result_packet);

DBG_VOID_RETURN;
}
/* }}} */

static struct st_mysqlnd_authentication_plugin mysqlnd_caching_sha2_auth_plugin =
{
{
MYSQLND_PLUGIN_API_VERSION,
"auth_plugin_caching_sha2_password",
MYSQLND_VERSION_ID,
PHP_MYSQLND_VERSION,
"PHP License 3.01",
"Johannes Schlüter <johannes.schlueter@php.net>",
{
NULL, /* no statistics , will be filled later if there are some */
NULL, /* no statistics */
},
{
NULL /* plugin shutdown */
}
},
{/* methods */
mysqlnd_caching_sha2_get_auth_data,
mysqlnd_caching_sha2_handle_server_response
}
};


/* {{{ mysqlnd_register_builtin_authentication_plugins */
void
mysqlnd_register_builtin_authentication_plugins(void)
{
mysqlnd_plugin_register_ex((struct st_mysqlnd_plugin_header *) &mysqlnd_native_auth_plugin);
mysqlnd_plugin_register_ex((struct st_mysqlnd_plugin_header *) &mysqlnd_pam_authentication_plugin);
mysqlnd_plugin_register_ex((struct st_mysqlnd_plugin_header *) &mysqlnd_caching_sha2_auth_plugin);

This comment has been minimized.

Sign in to view

Copy link
@nikic

nikic Nov 20, 2018

Author Member

I think the main bug here was that the auth plugin was always registered and the implementation then checked for SSL support and errored out in that case. Instead this line should be part of the MYSQLND_HAVE_SSL block, so the plugin is not visible when openssl is not supported.

This still leaves the issue of the hard-dependency on ext/hash. I think with this fixed we could at least reapply this on master, as ext/hash is required there anyway.
#ifdef MYSQLND_HAVE_SSL
mysqlnd_plugin_register_ex((struct st_mysqlnd_plugin_header *) &mysqlnd_sha256_authentication_plugin);
#endif
Expand Down
23 changes: 20 additions & 3 deletions ext/mysqlnd/mysqlnd_auth.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,26 @@ mysqlnd_auth_handshake(MYSQLND_CONN_DATA * conn,
unsigned int server_charset_no,
zend_bool use_full_blown_auth_packet,
const char * const auth_protocol,
struct st_mysqlnd_authentication_plugin * auth_plugin,
const zend_uchar * const orig_auth_plugin_data,
const size_t orig_auth_plugin_data_len,
const zend_uchar * const auth_plugin_data,
const size_t auth_plugin_data_len,
char ** switch_to_auth_protocol,
size_t * switch_to_auth_protocol_len,
zend_uchar ** switch_to_auth_protocol_data,
size_t * switch_to_auth_protocol_data_len
);

enum_func_status
mysqlnd_auth_handshake(MYSQLND_CONN_DATA * conn,
const char * const user,
const char * const passwd,
const size_t passwd_len,
const char * const db,
const size_t db_len,
const MYSQLND_SESSION_OPTIONS * const session_options,
zend_ulong mysql_flags,
unsigned int server_charset_no,
zend_bool use_full_blown_auth_packet,
const char * const auth_protocol,
const zend_uchar * const auth_plugin_data,
const size_t auth_plugin_data_len,
char ** switch_to_auth_protocol,
Expand Down

9 comments on commit 03740ef

@Destroy666x
Copy link

Choose a reason for hiding this comment

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

Why would you revert and break all installations that relied on this with no mention in changelog? Makes no sense to me

@Majkl578
Copy link
Contributor

Choose a reason for hiding this comment

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

As this has already been part of stable releases, this is quite a serious BC break.

@nikic
Copy link
Member Author

@nikic nikic commented on 03740ef Nov 20, 2018

Choose a reason for hiding this comment

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

Sorry that this change never made it into the changelog. Unfortunately the implementation of this functionality was not correct and broke existing installations. I would have preferred it if the issues were fixed instead, but this did not happen and I had to revert this entirely instead.

@Destroy666x
Copy link

Choose a reason for hiding this comment

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

@nikic the authentication method is there since 7.2.4 and from what I see the mentioned bug was for 7.2.8, were all the reverts needed?

@nikic
Copy link
Member Author

@nikic nikic commented on 03740ef Nov 27, 2018

Choose a reason for hiding this comment

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

@Destroy666x This authentication method was added in PHP 7.1.19 and PHP 7.2.8. It was certainly not present in PHP 7.2.4 mysqlnd.

@Destroy666x
Copy link

@Destroy666x Destroy666x commented on 03740ef Nov 27, 2018 •
edited

Choose a reason for hiding this comment

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

I'm fairly sure I didn't change default authentication method in 7.2.5 and 7.2.7 and it worked. Documentation also suggests that starting version for PDO at least: http://php.net/manual/en/ref.pdo-mysql.php It's also misleading for 7.2.11+ now.

Anyways, regardless of that, it's a BC break caused by revert to fix a BC break, which is not nearly any better.

@nikic
Copy link
Member Author

@nikic nikic commented on 03740ef Nov 27, 2018 •
edited

Choose a reason for hiding this comment

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

PHP 7.2.4 fixed auth plugin negotiation to retry authentication using mysql_native_password. This did not get reverted.

The only thing that was reverted is explicit support for caching sha2 auth. As long as your server still supports mysql_native_password things should continue to work, as between PHP 7.2.4 and 7.2.8.

If PHP 7.2.5 worked for you and 7.2.11 does not, without a change to the MySQL configuration (either explicitly, or implicitly through an update), then I'm stumped as to what is going on here. Can you please check which authentication methods are supported on your MySQL server?

@Destroy666x
Copy link

Choose a reason for hiding this comment

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

PHP 7.2.4 fixed auth plugin negotiation to retry authentication using mysql_native_password. This did not get reverted.

Hmmm, if that's the case it doesn't work as you mentioned. Both methods are supported on my local server, MySQL 8.0.13 on Docker. And I'm not alone: https://bugs.php.net/bug.php?id=77113

@SugarD-x
Copy link

@SugarD-x SugarD-x commented on 03740ef Dec 13, 2018 •
edited

Choose a reason for hiding this comment

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

I'm fairly sure I didn't change default authentication method in 7.2.5 and 7.2.7 and it worked.

I upgraded from an earlier version of PHP 7.1.x to PHP 7.2.8, and it automatically switched my MySQL 8.x database over to caching_sha2_password in the process. From what I've read elsewhere, this method existed as far back as PHP 7.2.4 or 7.2.5. Are you sure you upgrading PHP didn't automatically change the method for you too?

Edit: Probably worth mentioning that I upgraded MySQL at the same time, so now that I think about it, that may have been what triggered the method update. Not sure if MySQL 8.x automatically detects whether this is supported on install or not and adjusts the database accordingly.

Please sign in to comment.