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
signature.asc
Description: PGP signature
