Page MenuHomeFreeBSD

[PATCH] www/nginx-devel: choose modern TLS library for HTTPv3/QUIC
ClosedPublic

Authored by osa on Jan 6 2022, 10:26 PM.

Details

Summary

Hi,

it's possible to build NGINX with HTTPv3 support, the patch adds an ability to choose different implementations of modern protocol.

Please handle with care.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

osa requested review of this revision.Jan 6 2022, 10:26 PM
osa created this revision.
osa added reviewers: jhb, ashish, brnrd.

My gripe is not with www/nginx-devel port, but rather the security/boringssl and security/openssl-quictls ports, as some of us are using DEFAULT_VERSIONS+= ssl=openssl (i.e. security/openssl from ports) which means we can't use www/nginx-devel with HTTPv3 support unless security/boringssl and security/openssl-quictls get installed to different prefixes so as to not conflict with security/openssl, as not all OpenSSL dependent software currently builds with boringssl, or openssl-quictls.

Thanks @osa for making available HTTP/3 support in nginx for testing.

Apart from DEFAULT_VERSIONS unfortunateness, overall it looks good and was tested to work.

www/nginx-devel/Makefile
90

I'd limit these radio options to DEPENDS business,
and leave configure/patch part for HTTPV3, to where it is now.

91

N.B. (and for the sake of archives)
this line is no longer required after recent fixes

92

This looks redundant (and would miss the stream part):
it's implied by corresponding quic modules, similar to rewrite and pcre.
So, I'd just strip it.

93

It could be more specific against what nginx-quic revision it is being built on
(i.e. include revision number here).

104

This way the file doesn't differentiate from security/openssl(-devel)
and would allow to build (and fail) with such an inappropriate library.
I'd replace it with something more specific, like include/openssl/quic.h, to get more strength.

180

With this differential revision, HTTPV3 does nothing being listed in options.
From the user perspective, IMHO it's still useful to have it, together with the HTTPV2 option as it's now.
With the above other suggestions, it could be further improved similar to GSSAPI:

V3_BORING_IMPLIES=	HTTPV3
V3_QUICTLS_IMPLIES=	HTTPV3

and

.if ${PORT_OPTIONS:MHTTPV3} && \
    (empty(PORT_OPTIONS:MV3_BORING) && empty(PORT_OPTIONS:MV3_QUICTLS))
IGNORE=		required V3_BORING or V3_QUICTLS \
		to be defined. Please do the needful
 .endif

Notably, enabling HTTPV3 won't try to build against the default (incompatible) SSL library.

osa marked 5 inline comments as done.
pluknet requested changes to this revision.Jan 10 2022, 8:54 PM
pluknet added inline comments.
www/nginx-devel/Makefile
75

It's useful to mark these options as conflicting, something like that:

HTTPV3_BORING_PREVENTS= HTTPV3_QTLS
181

Needs to be ${LOCALBASE}/include/openssl/quic.h (missing "openssl" subdirectory)

This revision now requires changes to proceed.Jan 10 2022, 8:54 PM
osa marked 2 inline comments as done.
This revision is now accepted and ready to land.Jan 11 2022, 9:21 AM