Bug #80344: LDAP SASL authentication for mysqlnd #6447
Conversation
…thentication This will introduce support for LDAP SASL authentication using SCRAM-SHA-1 and SCRAM-SHA-256, at the moment those methods are supported only by the enterprise version of MySQL, but are widely used.
|
Does anybody can have a look at this and give me a review? Thanks |
|
@johannes, @andreyhristov, could you please have a look at this? |
|
Hi @johannes , @andreyhristov, how's going this review? Thanks |
| AC_DEFINE([MYSQLND_SSL_SUPPORTED], 1, [Enable core mysqlnd SSL code]) | ||
| AC_DEFINE([MYSQLND_SASL_SUPPORTED], 1, [Enable sasl code]) |
| @@ -630,6 +630,8 @@ enum mysqlnd_packet_type | |||
| PROT_SHA256_PK_REQUEST_PACKET, | |||
| PROT_SHA256_PK_REQUEST_RESPONSE_PACKET, | |||
| PROT_CACHED_SHA2_RESULT_PACKET, | |||
| PROT_SASL_PK_REQUEST_PACKET, | |||
| @@ -936,6 +937,7 @@ struct st_mysqlnd_connection_data | |||
| unsigned int protocol_version; | |||
| unsigned int port; | |||
| zend_ulong server_capabilities; | |||
| void* sasl_connection; | |||
There was a problem hiding this comment.
Adding a structure member in the middle of a structure changes the ABI. This means that the change should go into x.y.0 version of PHP or has the potential to break installations which use mysqlnd plugins. struct st_mysqlnd_connection_data is a structure which plugins often use.
| MYSQLND_VIO * vio = conn->vio; | ||
| MYSQLND_STATS * stats = conn->stats; | ||
| MYSQLND_PACKET_SASL_PK_REQUEST* pkt_req = (MYSQLND_PACKET_SASL_PK_REQUEST*)_packet; | ||
| size_t sent = 0; |
| MYSQLND_PACKET_SASL_PK_REQUEST* pkt_req = (MYSQLND_PACKET_SASL_PK_REQUEST*)_packet; | ||
| size_t sent = 0; | ||
| zend_uchar* buffer = (zend_uchar*)malloc(pkt_req->data_len + MYSQLND_HEADER_SIZE); | ||
| if( !buffer ) { |
There was a problem hiding this comment.
There should be whitespace after if and no whitespace around the condition
| stats, error_info); | ||
|
|
||
| if(buffer) { | ||
| free( buffer ); |
| MYSQLND_STATS * stats = conn->stats; | ||
| MYSQLND_CONNECTION_STATE * connection_state = &conn->state; | ||
| zend_uchar buffer[SASL_PK_REQUEST_RESP_BUFFER_SIZE]; | ||
| if( packet->data_len > SASL_PK_REQUEST_RESP_BUFFER_SIZE ) { |
| zend_uchar buffer[SASL_PK_REQUEST_RESP_BUFFER_SIZE]; | ||
| if( packet->data_len > SASL_PK_REQUEST_RESP_BUFFER_SIZE ) { | ||
| DBG_ERR_FMT("Not able to handle response, max packet size is %d", | ||
| SASL_PK_REQUEST_RESP_BUFFER_SIZE); |
There was a problem hiding this comment.
You can put the whole on one line
| @@ -2734,6 +2845,8 @@ MYSQLND_CLASS_METHODS_START(mysqlnd_protocol_payload_decoder_factory) | |||
| MYSQLND_METHOD(mysqlnd_protocol, init_sha256_pk_request_packet), | |||
| MYSQLND_METHOD(mysqlnd_protocol, init_sha256_pk_request_response_packet), | |||
| MYSQLND_METHOD(mysqlnd_protocol, init_cached_sha2_result_packet), | |||
| MYSQLND_METHOD(mysqlnd_protocol, init_sasl_pk_request_packet), | |||
| @@ -297,7 +297,17 @@ typedef struct st_mysqlnd_packet_cached_sha2_result { | |||
| unsigned int error_no; | |||
| } MYSQLND_PACKET_CACHED_SHA2_RESULT; | |||
|
|
|||
|
|
|||
| typedef struct st_mysqlnd_packet_sasl_pk_request { | |||
| MYSQLND_PACKET_HEADER header; | |||
There was a problem hiding this comment.
whitespace/aligning in both structures
|
@fjanisze, are you still interested in having this merged? As I understand it, a big problem is that probably none of the php-src maintainers might have a way to test this functionality, let alone to check it in CI. |
|
@fjanisze: Why have you closed this PR? It is an important missing feature! Linked to: |
|
I wish you a Happy New Year 2024! It is possible to have an answer? Thanks in advance. |
This was originally a pull request for 7.4 (#6417), but new code like this won't make it to 7.4 anymore.
This will introduce support for LDAP SASL authentication using
SCRAM-SHA-1 and SCRAM-SHA-256, at the moment those methods
are supported only by the enterprise version of MySQL, but are
widely used.