Page MenuHomeFreeBSD

[new port] security/openssl-chelsio: SSL and crypto library supporting Chelsio TLS offload
Needs ReviewPublic

Authored by jhb on Mar 9 2018, 7:42 PM.

Details

Reviewers
brnrd
Summary

This port includes additional patches from Chelsio intended to be used
in conjunction with Chelsio NICs supporting offload of TCP TLS connections.
This port installs OpenSSL libraries which should be binary compatible
with base system libraries.

Test Plan
  • need to do more testing, have only verified it builds and generates the correct package contents (libraries in right place) so far

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23044
Build 22114: arc lint + arc unit

Event Timeline

jhb created this revision.Mar 9 2018, 7:42 PM
jhb updated this revision to Diff 40123.Mar 10 2018, 12:53 AM
  • Update to newer Chelsio patch that only modifies libssl. As a result, stop installing libcrypto and engines. No need to create libssl.so symlink either as this is not intended as a compile target, just a runtime override.
  • Remove pkg-message.in since we aren't shipping a sample config file.
jhb updated this revision to Diff 40124.Mar 10 2018, 1:01 AM
  • Since this package is meant to override the base system libssl.so.8 (at least currently), read config files from /etc/ssl.
brnrd added a comment.Mar 10 2018, 9:38 AM

There's little difference against the regular port. Biggest diff is the pkg-plist.
I can see reasons for keeping it separate, e.g. to decouple version updates but this port could also be a slave port of security/openssl if that were modified to have dynamic library path.

EXTRA_PATCHES=${CURDIR}/files/extra-patch-chssl
# Force SHARED opt

Got to get a CURRENT system up to see if I can build this...

security/openssl-chelsio/Makefile
7

Appears before DIST_SUBDIR but after MASTER_SITES

19

Doesn't conflict with any of these ports at the moment...
Can coexist with the regular ports.

102

Will remove this from the regular port. Relics from the past ๐Ÿ˜„

106

Port, so belongs in ${PREFIX}/etc

110

Should also mention that this is in 12.x (-STABLE) at the moment?

137

Can't this just be a regular patch? That would require patching the extra-patch-chssl file...

151

Shouldn't this go into ${PREFIX}/openssl-chelsio/libdata ?

158

Does this use standard libcrypto from base or the port?
Sounds like a dangerous thing to do...

security/openssl-chelsio/pkg-plist
3

Doesn't need it's own set of include files?

#include "ssl_tom.h"

or are these private headers.

jhb marked 5 inline comments as done.Mar 12 2018, 7:01 PM

There's little difference against the regular port. Biggest diff is the pkg-plist.
I can see reasons for keeping it separate, e.g. to decouple version updates but this port could also be a slave port of security/openssl if that were modified to have dynamic library path.

EXTRA_PATCHES=${CURDIR}/files/extra-patch-chssl
# Force SHARED opt

Got to get a CURRENT system up to see if I can build this...

Note that that the src change that adds the needed t4_tls.h header is still in review (D14529), so this port won't build on stock head yet until that is committed.

(Also, this is just a first attempt at a port, it might be we want to do this differently)

security/openssl-chelsio/Makefile
106

I had done this on purpose to try to be a transparent plugin for the base OpenSSL, but I guess it's fine to do. I should probably install a sample config file as well in that case.

137

I did it this way so I could easily generate it from the patches I currently have (which are against the base OpenSSL in FreeBSD and a FreeBSD tree rather than against an OpenSSL tree). I could change it though.

151

Hmm, this matches what openssl-unsafe does, but I'm not sure if we want pkgconfig data for this package anyway since it's only intended to be used as an alternate runtime library?

158

From base, but I could change it back to shipping a libcrypto.so.

jhb marked an inline comment as done.Mar 12 2018, 8:59 PM
jhb added inline comments.
security/openssl-chelsio/pkg-plist
3

That is a private header.

jhb updated this revision to Diff 40221.Mar 12 2018, 9:02 PM
  • Fix various review feedback comments.
  • Re-add libcrypto.so and engine shared objects.
brnrd accepted this revision.Apr 8 2018, 12:57 PM

Looking OK apart from the last inline comment

security/openssl-chelsio/Makefile
98

Still overwrites base?
And I don't see it used in pkg-plist. Am I missing something?

This revision is now accepted and ready to land.Apr 8 2018, 12:57 PM
jhb updated this revision to Diff 46097.Jul 31 2018, 7:25 PM
  • Updated chssl patch to fix a seg fault when aesni is enabled.
  • Fix OPENSSLDIR to not include etc.
  • Install sample config file like the normal openssl port.
This revision now requires review to proceed.Jul 31 2018, 7:25 PM
jhb added a comment.Jul 31 2018, 7:27 PM

Sorry for the long delay in replying. Forgot to mention that I also fixed it to use SRC_BASE.

I haven't tried yet to move to 1.0.2o.

security/openssl-chelsio/Makefile
98

It does not overwrite base, and it no longer tries to use config files from base. Given that, I've installed a sample config file like the normal port in ${PREFIX}/openssl-chelsio/

linimon retitled this revision from Add a security/openssl-chelsio port. to [new port] security/openssl-chelsio: SSL and crypto library supporting Chelsio TLS offload.Aug 2 2018, 10:18 PM
jhb updated this revision to Diff 54990.Mar 12 2019, 4:32 PM
  • Port to OpenSSL 1.1.1.
jhb added a comment.Thu, Mar 28, 8:58 PM

@brnrd I've rebased this patchset to be relative to OpenSSL 1.1.1 rather than 1.0.2. It is now much closer to the 1.1.1 port. It installs into a dedicated directory so it does not conflict with other openssl ports. It matches the 12.x base shared library version so that it could be used with LD_LIBRARY_PATH, but in theory a user could also use libmap to use it with ports built against the openssl111 port. It generally borrows all of the patches other than the extra chelsio patch from the openssl111 port. I didn't make this a slave port b/c I didn't want it to be a drag on updating the main openssl port in case the patches didn't apply out-of-the-box to newer versions, etc.