Code review comment for ~ahasenack/ubuntu/+source/apache2:bionic-client-cert-auth-fix

Revision history for this message
Bryce Harrington (bryce) wrote :

LGTM +1

* I ran through the full test case in the bug (thanks for preparing it!)
  - Red/Green testing worked as expected.
  - http://paste.ubuntu.com/p/tzVmRKK7Dh/
* Debdiff: looks good, one minor spelling error in comment (see inline)
* Review of code: no obvious errors spotted

SSL_CTX_clear_mode() removes the SSL_MODE_AUTO_RETRY mode via the ctx bitmask. From the description of this flag, I gather that during the initialization process there is unexpected handshake data, and the code is confused and keeps retrying looking for application data. With this patch, if it encounters handshake data it skips the retry and continues with processing. For regression potential I take it there is no chance that skipping the retry could cause issues if transmission errors were to occur, but perhaps that would be something to watch for.

review: Approve (code/debdiff)

« Back to merge proposal