Hi Nathan,
On Thu, Apr 08, 2021 at 06:15:10PM -0700, Nathan Konopinski wrote:
> One other behavior I've observed, nginx has an ssl_buffer_size (
> http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_buffer_size)
> option that defaults to 16kb. As I decrease its value, I start seeing the
> same body size != content length errors from clients.
Interesting, really, as it definitely confirms an issue with the way the
client handles TLS.
> I tried experimenting with tune.idletimer to force the use of large ssl
> buffers (tune.ssl.maxrecord's default of no limit seems > than 16kb so I
> didn't bother adjusting it) but I'm still seeing errors.
I've just rechecked in the code and indeed, by default it will not
limit.
> From my reading of
> the docs, a stream must be idle for 1s by default before the larger ssl
> buffers are used. It also seems a stream must be idle for at least 1ms
> before the large buffers are available. Is that correct?
I don't remember these details but the maxrecord is only an upper bound
so if you didn't set it it won't have any effect.
> Is there a way to
> always use the larger ssl buffers without this detection?
Based on your observation, I suspect that for whatever reason, the client
is bogus and expects certain data to be located within a single SSL record,
which is a violation of the standard, but bugs are present :-/
What might very likely happen here is that some records are shorter
than others just because the buffer wraps, and the openssl API doesn't
provide a vector-based send function. So you can't say "send 1kB till
the end of the buffer and loop back to the beginning for 3 extra kB".
Instead you have to perform one 1kB write and a second 3kB write,
resulting in two records.
One workaround could be the following patch which always unwraps the
buffer before sending, always resulting in the largest possible record:
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f1b0668ab..2b47e6b10 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6071,6 +6071,7 @@ static size_t ssl_sock_to_buf(struct connection *conn,
void *xprt_ctx, struct bu
static size_t ssl_sock_from_buf(struct connection *conn, void *xprt_ctx, const
struct buffer *buf, size_t count, int flags)
{
struct ssl_sock_ctx *ctx = xprt_ctx;
+ struct buffer aligned_buf = *buf;
ssize_t ret;
size_t try, done;
@@ -6093,6 +6094,13 @@ static size_t ssl_sock_from_buf(struct connection *conn,
void *xprt_ctx, const s
#endif
try = b_contig_data(buf, done);
+
+ if (try < (b_data(buf) - done) && try < count) {
+ b_slow_realign(&aligned_buf, trash.area, done);
+ try = b_contig_data(&aligned_buf, done);
+ buf = &aligned_buf;
+ }
+
if (try > count)
try = count;
It's not pretty but it should do the job by making certain that every
time the record size is limited by wrapping, the buffer is first
unwrapped and the record is maximized. I'm also attaching it to help
you apply it. It was made for 2.4-dev but I verified that it still
applies (with an offset) on 2.2. I'd be interested in knowing if that
addresses your problem.
> I'm not sure what a stream represents in this context.
It's a bidirectional communication between a client and a server in
the context of a single HTTP transaction. You can see this as the
application level in haproxy, which deals with request/response
forwarding between the two sides and which executes in turn all the
analysers and filters corresponding to the rules in your config.
You can have a look at doc/internals/muxes.png for a quick overview
(the stream is the entity at the top with carries the request buffer).
Willy
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f1b0668ab..2b47e6b10 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6071,6 +6071,7 @@ static size_t ssl_sock_to_buf(struct connection *conn,
void *xprt_ctx, struct bu
static size_t ssl_sock_from_buf(struct connection *conn, void *xprt_ctx, const
struct buffer *buf, size_t count, int flags)
{
struct ssl_sock_ctx *ctx = xprt_ctx;
+ struct buffer aligned_buf = *buf;
ssize_t ret;
size_t try, done;
@@ -6093,6 +6094,13 @@ static size_t ssl_sock_from_buf(struct connection *conn,
void *xprt_ctx, const s
#endif
try = b_contig_data(buf, done);
+
+ if (try < (b_data(buf) - done) && try < count) {
+ b_slow_realign(&aligned_buf, trash.area, done);
+ try = b_contig_data(&aligned_buf, done);
+ buf = &aligned_buf;
+ }
+
if (try > count)
try = count;
