SSL_CTX_set_default_read_buffer_len seems unreliable

[bagder]

Problem:

When we use:

  SSL_CTX_set_default_read_buffer_len(ssl_ctx, 0x401e * 4);

... it seems to make curl transfers unreliable - in particular in one specific test in the curl test suite. (I realize this is a weak and problematic bug report and I won't hold it against you if you decide to close it.)

But: we build curl and run the exact same tests using "vanilla" OpenSSL without getting these problems so it seems to indicate that only aws-lc has these specific problems.

In curl, we will disable the use of this function when built with aws-lc as a work-around (curl/curl#18434) but that comes with a small performance penalty.

[samuel40791765]

Thank you for reaching out, we really appreciate the bug report! I'm not sure if this is the exact reason, but curl is setting a buffer size larger than the maximum buffer length that AWS-LC allows:

aws-lc/include/openssl/ssl.h

Lines 5396 to 5400 in 5a834ec

	// SSL_CTX_set_default_read_buffer_len sets the size of the buffer reads will use on
	// |ctx| if read ahead has been enabled. 0 is the minimum and 65535 is the maximum.
	// A |len| of 0 is the same behavior as read ahead turned off: each call to
	// |SSL_read| reads the amount specified in the TLS Record Header.
	OPENSSL_EXPORT int SSL_CTX_set_default_read_buffer_len(SSL_CTX *ctx, size_t len);

0x401e * 4 is inherently 65656, which is larger than the allowed 65535 and AWS-LC directly uses the largest size available (65535) if the consuming application sets a value larger than that. This limitation's due to a buffer size we inherited from upstream:

aws-lc/ssl/ssl_buffer.cc

Lines 32 to 34 in 5a834ec

	// BIO uses int instead of size_t. No lengths will exceed SSLBUFFER_MAX_CAPACITY
	// (uint16_t), so this will not overflow.
	static_assert(SSLBUFFER_MAX_CAPACITY <= INT_MAX,

I'll try debugging and upgrading the buffer size to see if it fixes things, but seeing that the failures are sporadic I can't be 100% confident as there could be other subtle nuances involved. We'll get back to you as soon as we've made more progress on this.

[smittals2]

Hi, this issue should be resolved as of #2673. This fix was also included in our v1.61.0 release.

This issue will now be close. Please let us know if you still continue to have issues after upgrading. Feel free to comment here or open a new issue if needed. Thanks for your help in improving our library! 😀