[squid-dev] [PATCH] server_name ACL

[Amos Jeffries]

On 10/04/2015 1:06 a.m., Tsantilas Christos wrote:
> Hi all,
>  I am reposting this patch. It is updated to the latest squid-trunk.
> 
> In a discussion with Amos (the period the squid-dev was down):
>   1) The server_name should be renamed to tls_server_name or
> ssl::server_name
>   2) There is a bug in Ssl::matchX509CommonNames function. The
> subjectAltName if exists should be used instead of the subject name.
> 
> The (2) should be fixed as a separate issue/bug, and also applied to
> squid-3.5.
> 
> What about the (1) ?
> The "ssl:" prefix looks better because the new feature can be used for
> ssl v3 too, it is not depends on tls. (However I believe that we should
> agree and use one prefix for all of these features to not confuse users)

While being usable for SSLv3 is fine, SSL as a whole is already
deprecated (RFC 6101 is "Historic") and a "die die die" / "MUST NOT use
SSLv3" RFC is already on the fast track for publication within the year
mandating that SSLv3 be rejected on sight.


I'm agreeing with ssl::server_name not because its SSL-compatible test,
but because the existing ACLs for cert related details already use that
prefix. We are already in the unfortunate position of having to rename
at some future point, may as well at least be consistent until then.




As for the audit:

in src/acl/ServerName.h:

* please drop the "\ingroup" on new code
 That feature of doxygen is no longer being used.


in src/cf.data.pre:

* s/SslBmp/SslBump/ or s/SslBmp/Ssl-Bump/


in src/ssl/PeerConnector.cc:

* Ssl::PeerConnector::handleServerCertificate() - please dont add HERE
macro in new code.


+1, conditional on the name agreement and above cosmetic changes.

Amos