On 1/25/2023 3:16 PM, Eric Covener wrote:
I can't state this enough: this is a serious security threat. MySQL connections need TLS support
from APR. This isn't a "feature"; it is a "security" issue. We should all
care very deeply about this.
I think it is/can be both a feature and a security issue. I don't
think the project can handle it as a vulnerability.
The title of "vulnerable" is shifted to any organization which uses APR
in its present state for MySQL connections where the client and server
are on different machines/interfaces. That's not a vulnerability in APR
itself but it is a serious security issue that can be fixed only within APR.
I'm asking that the next release of APR be held until this important fix is
merged in.
DBD is part of apr-util, so this is more about the proposed
apr-util-1.6.2 release. I am not sure it meets the (admittedly
unusual) project versioning rules for to be included in a micro
update. It is both new function and changed interpretation of
arguments. Others may know/feel differently here.
This is confusion that has stymied every attempt I've so far made to get
this patch merged in. I'll do whatever is necessary to see this through
if anyone can offer a clear path to do so. I've been at this same issue
for five years.
Regardless, I don't think it meets the bar to hold up a release. It is
not a regression.
APR releases are so rare. I fear that this serious security issue will
remain open for years to come.
Some high level feedback on the patch:
- is the example argument to cipher really correct? It is a single
protocol (tlsversion?) and not a "list of ciphers".
- The license should not be displaced by the comments added to the top
of the file
- I think a small percent of the top-of-file comment should be moved
in-line to wherever it's useful and the rest left for the bugzilla
entry.
- I think the parameters should be mentioned in doxygen in apr_dbd.h
- Should it fail if TLS parms are provided but the mysql version macro
will ignore it?
- Is there any impact to mariadb?
- c99 comments (//) should not be used
Wow; thanks! I'd have addressed any and all such feedback so many years
ago had it been given! Is there a PR process I can initiate to handle
feedback like this at the file-line level? Many years ago, I didn't have
access to such a mechanism. Since then, I believe I've seen some
traffic indicating a "new" GitHub avenue. I have a handful of projects
on GitHub and would be perfectly comfortable using it for this.
