On 09/08/17 15:51, Mark Thomas wrote:
> I'll take a look at this. I think the hint about the statistics hack
> will help me get further than I have so far. Whether it is far enough,
> we'll see.
As it happened, the statistics weren't the fix I needed but they got me
looking in the right direction where I found SSL_renegotiate_pending() -
thanks for the hint.
I have attached my proposed patch for review.
It seems to be moving back towards an a previous approach. The commit
history suggests this approach should not be necessary. I have tested
both 7.0.x and 9.0.x and both versions need the additional reads. I'm
not 100% sure what is going on.
If there aren't any objections, I plan to apply this patch roll 1.2.13
in time for it to be included in the September round of Tomcat releases.
Mark
Index: native/src/sslnetwork.c
===================================================================
--- native/src/sslnetwork.c (revision 1804444)
+++ native/src/sslnetwork.c (working copy)
@@ -365,7 +365,7 @@
* Check for failed client authentication
*/
if (con->ctx->verify_mode != SSL_VERIFY_NONE &&
- (vr = SSL_get_verify_result(con->ssl)) != X509_V_OK) {
+ (vr = SSL_get_verify_result(con->ssl)) != X509_V_OK) {
if (SSL_VERIFY_ERROR_IS_OPTIONAL(vr) &&
con->ctx->verify_mode == SSL_CVERIFY_OPTIONAL_NO_CA) {
@@ -622,8 +622,9 @@
{
tcn_socket_t *s = J2P(sock, tcn_socket_t *);
tcn_ssl_conn_t *con;
- int retVal;
+ int retVal, error;
char peekbuf[1];
+ apr_interval_time_t timeout;
UNREFERENCED_STDARGS;
TCN_ASSERT(sock != 0);
@@ -633,28 +634,59 @@
* handshake to proceed.
*/
con->reneg_state = RENEG_ALLOW;
+
+ // Schedule a renegotiation request
retVal = SSL_renegotiate(con->ssl);
if (retVal <= 0)
return APR_EGENERAL;
- retVal = SSL_do_handshake(con->ssl);
- if (retVal <= 0)
- return APR_EGENERAL;
- if (!SSL_is_init_finished(con->ssl)) {
- return APR_EGENERAL;
- }
-
- /* Need to trigger renegotiation handshake by reading.
+ /* Need to trigger the renegotiation handshake by reading.
* Peeking 0 bytes actually works.
* See: http://marc.info/?t=145493359200002&r=1&w=2
+ *
+ * This will normally return SSL_ERROR_WANT_READ whether the renegotiation
+ * has been completed or not. Afterwards, need to determine if I/O needs to
+ * be triggered or not.
*/
- SSL_peek(con->ssl, peekbuf, 0);
+ retVal = SSL_peek(con->ssl, peekbuf, 0);
+ if (retVal < 1) {
+ error = SSL_get_error(con->ssl, retVal);
+ }
- con->reneg_state = RENEG_REJECT;
+ apr_socket_timeout_get(con->sock, &timeout);
+ // If the renegotiation is still pending, then I/O needs to be triggered
+ while (SSL_renegotiate_pending(con->ssl)) {
+ if (error == SSL_ERROR_WANT_READ) {
+ retVal = wait_for_io_or_timeout(con, error, timeout);
+ /*
+ * Since this is blocking I/O, anything other than APR_SUCCESS is an
+ * error.
+ */
+ if (retVal != APR_SUCCESS) {
+ printf("ERROR\n");
+ con->shutdown_type = SSL_SHUTDOWN_TYPE_UNCLEAN;
+ return retVal;
+ }
+ } else {
+ // SSL_ERROR_WANT_READ is expected. Anything else is an error.
+ return APR_EGENERAL;
+ }
- if (!SSL_is_init_finished(con->ssl)) {
- return APR_EGENERAL;
+ // Re-try SSL_peek after I/O
+ retVal = SSL_peek(con->ssl, peekbuf, 0);
+ if (retVal < 1) {
+ error = SSL_get_error(con->ssl, retVal);
+ } else {
+ /*
+ * Reset error to handle case where SSL_Peek returns 0 but
+ * SSL_renegotiate_pending returns true. This will trigger an error
+ * to be returned.
+ */
+ error = 0;
+ }
}
+
+ con->reneg_state = RENEG_REJECT;
return APR_SUCCESS;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
