On Sat, Jan 27, 2018 at 05:00:17PM -0500, Peter Eisentraut wrote:
> On 1/25/18 09:07, Peter Eisentraut wrote:
>> On 1/19/18 13:43, Peter Eisentraut wrote:
>>> Comparing the existing {be,fe}-secure-openssl.c with the proposed
>>> {be,fe}-secure-gnutls.c, and with half an eye on the previously proposed
>>> Apple Secure Transport implementation, I have identified a few more
>>> areas of refactoring that should be done in order to avoid excessive
>>> copy-and-pasting in the new implementations:
>>
>> And here is another place that needs cleaning up, where the OpenSSL API
>> was used directly.
>
> And here is another one. The last one for now I think.
The comment at the top of PQinitSSL mentions OpenSSL, you may want to
get rid of it as well.
In short, this patch:
- moves wildcard_certificate_match from fe-secure-openssl.c to
fe-secure.c.
- splits verify_peer_name_matches_certificate into two, with one common
part as pq_verify_peer_name_matches_certificate and one part which is
SSL-specific as pgtls_verify_peer_name_matches_certificate_guts.
- splits verify_peer_name_matches_certificate_name into two, with one
common part as verify_peer_name_matches_certificate_name and one part
which is SSL-specific as openssl_verify_peer_name_matches_certificate_name().
To be honest, I find this refactoring confusing. As things stand on
HEAD, fe-secure.c depends on the contents of fe-secure-openssl.c, and
the dependency goes only in one direction. With your patch, you also
make fe-secure-openssl.c call things within fe-secure.c. It seems to me
that a cleaner split would be to introduce a common file for all the
low-level routines like the two ones you are introducing here, say
fe-secure-common.c. aND pq_verify_peer_name_matches_certificate_name and
pq_verify_peer_name_matches_certificate should be moved to that.
--
Michael