Re: HOLD apr-1.7.1-rc2

On Wed, Jan 25, 2023 at 1:53 PM William Kimball Jr.
<[email protected]> wrote:
>
> Eric Covener has instructed me to spin this discussion off to another thread, so here it is.
>
> Way back in 2018, I submitted https://bz.apache.org/bugzilla/show_bug.cgi?id=62342 (apr_dbd_mysql Lacks TLS Support).  Data exfiltration is a serious threat to businesses.  I found that MySQL connections using APR were exposed and there was no way to encrypt them via the library.  So, I volunteered my time to offer the necessary patch to close this serious security risk.
>
> New to the APR list and process, I asked for guidance as to how to submit my work.  I followed every instruction provided to me, even when I was instructed to submit a second patch for a future APR 2.x.  Now going on 5 years later, my contribution is still missing from APR.
>
> 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.

> 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.

Regardless, I don't think it meets the bar to hold up a release. It is
not a regression.

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