Page MenuHomeFreeBSD

New port: security/ktls_isa-l_crypto.
ClosedPublic

Authored by gallatin on Aug 27 2019, 10:54 PM.

Details

Summary

This port provides a kernel module containing a KTLS software
backend for AES-GCM connections using Intel's ISA-L crypto
library.

Test Plan
  • tested with KTLS
  • I have left gallatin@ as the MAINTAINER as he is likely to be more active with this going forward I think

Diff Detail

Repository
rP 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

jhb created this revision.Aug 27 2019, 10:54 PM
gallatin accepted this revision.Aug 30 2019, 1:46 PM
gallatin added inline comments.
security/ktls_isa-l_crypto/files/Makefile
35 ↗(On Diff #61380)

I'm not a ports expert, but does specifying the full path to yasm here come with any drawbacks?

This revision is now accepted and ready to land.Aug 30 2019, 1:46 PM
jhb added inline comments.Aug 30 2019, 9:41 PM
security/ktls_isa-l_crypto/files/Makefile
35 ↗(On Diff #61380)

Probably. It should maybe use LOCALBASE. I'll try to find a ports person who can review this.

imp accepted this revision.Aug 30 2019, 10:02 PM
imp added a subscriber: imp.

It looks good to my eye, modulo my suggestion.

security/ktls_isa-l_crypto/files/Makefile
35 ↗(On Diff #61380)

YASM=${PREFIX}/bin/yasm
I think is the right thing

jhb updated this revision to Diff 61495.Aug 30 2019, 10:22 PM
  • Use LOCALBASE for yasm.
  • Make portlint mostly happy.
  • Use IGNORE instead of BROKEN.
  • Use PLIST_FILES instead of 1-line pkg-plist.
This revision now requires review to proceed.Aug 30 2019, 10:22 PM
jhb marked 2 inline comments as done.Aug 30 2019, 10:24 PM
jhb updated this revision to Diff 61498.Aug 31 2019, 12:12 AM
  • Use bsd.post.port.mk at bottom.
  • Add missing svn props on intel_isakern.c.

By convention we use the suffix -kmod for such ports.

Let me take a look at a few other things that those ports do by default (I know USES=uidfix is in most of them). I am not that familiar with them.

Please see PR 240395. After a half hour of trying to argue with phabricator-- and failing -- I need to move on to other tasks.

gallatin commandeered this revision.Thu, Sep 26, 7:04 PM
gallatin edited reviewers, added: jhb; removed: gallatin.
gallatin updated this revision to Diff 62612.Thu, Sep 26, 7:07 PM
  • I've taken linimon's changes from bugzilla
  • I've hooked the port to the Makefile in the parent

Mark: I was not able to reproduce your build failure. If you could give me more details about where you're building the port, I can try to fix what you are seeing.

gallatin updated this revision to Diff 62662.Fri, Sep 27, 7:20 PM

Updated the port to handle TLS 1.3

gallatin updated this revision to Diff 62663.Fri, Sep 27, 7:22 PM

Re-submit diff with -U999999 for full context.

Mark: I was not able to reproduce your build failure. If you could give me more details about where you're building the port, I can try to fix what you are seeing.

I am trying to bring up a later 13 snapshot so that I can try again.

linimon added inline comments.Sun, Sep 29, 12:56 AM
security/ktls_isa-l_crypto-kmod/Makefile
1 ↗(On Diff #62663)

So, the Created by: line is only "about" the ports Makefile itself. Even so, we've been moving away from including it, in favor of letting people use SVN to look at the committer of the initial import. FYI.

This revision was not accepted when it landed; it landed in state Needs Review.Sun, Sep 29, 1:10 AM
This revision was automatically updated to reflect the committed changes.
jhb added inline comments.Mon, Sep 30, 11:33 PM
head/security/ktls_isa-l_crypto-kmod/files/intelisa_kern.c
166

This actually undoes one of the changes I had made. For TLS < 1.3 you need to read the 8 byte nonce out of the TLS header supplied by the kernel instead of assuming it is the sequence number. Especially if we do the change to use a random starting value, then this will break since the nonce on the wire will be the random value but this module would have encrypted it as if it was the sequence number, so the client will get garbage when it tries to decrypt.

jhb added inline comments.Tue, Oct 1, 4:38 PM
head/security/ktls_isa-l_crypto-kmod/files/intelisa_kern.c
142

Why is this 32 bytes I wonder? The AES IV is always 16 bytes regardless of key size as it is the length of one block. Furthermore, for GCM it is really always 12 bytes with the last 4 bytes as a counter that always starts as 0x0002 and increments by 1 for each 16-byte block.