Page MenuHomeFreeBSD

Refactor driver and consumer interfaces for OCF (in-kernel crypto).
ClosedPublic

Authored by jhb on Feb 13 2020, 11:51 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 11, 1:49 AM
Unknown Object (File)
Thu, Dec 5, 9:26 AM
Unknown Object (File)
Mon, Dec 2, 9:48 AM
Unknown Object (File)
Wed, Nov 20, 9:03 AM
Unknown Object (File)
Tue, Nov 19, 5:36 PM
Unknown Object (File)
Tue, Nov 19, 5:35 PM
Unknown Object (File)
Tue, Nov 19, 5:31 PM
Unknown Object (File)
Tue, Nov 19, 5:27 PM

Details

Summary
  • The linked list of cryptoini structures used in session initialization is replaced with a new flat structure: struct crypto_session_params. This session includes a new mode to define how the other fields should be interpreted. Available modes include:
    • COMPRESS (for compression/decompression)
    • CIPHER (for simply encryption/decryption)
    • DIGEST (computing and verifying digests)
    • AEAD (combined auth and encryption such as AES-GCM and AES-CCM)
    • ETA (combined auth and encryption using encrypt-then-authenticate)

      Additional modes could be added in the future (e.g. if we wanted to support TLS MtE for AES-CBC in the kernel we could add a new mode for that. TLS modes might also affect how AAD is interpreted, etc.)

      The flat structure also includes the key lengths and algorithms as before. However, code doesn't have to walk the linked list and switch on the algorithm to determine which key is the auth key vs encryption key. The 'csp_auth_*' fields are always used for auth keys and settings and 'csp_cipher_*' for cipher. (Compression algorithms are stored in csp_cipher_alg.)
  • Drivers no longer register a list of supported algorithms. This doesn't quite work when you factor in modes (e.g. a driver might support both AES-CBC and SHA2-256-HMAC separately but not combined for ETA). Instead, a new 'crypto_probesession' method has been added to the kobj interface for symmteric crypto drivers. This method returns a negative value on success (similar to how device_probe works) and the crypto framework uses this value to pick the "best" driver. There are three constants for hardware (e.g. ccr), accelerated software (e.g. aesni), and plain software (cryptosoft) that give preference in that order. One effect of this is that if you request only hardware when creating a new session, you will no longer get a session using accelerated software. Another effect is that the default setting to disallow software crypto via /dev/crypto now disables accelerated software.

    Once a driver is chosen, 'crypto_newsession' is invoked as before.
  • Crypto operations are now solely described by the flat 'cryptop' structure. The linked list of descriptors has been removed.

    A separate enum has been added to describe the type of data buffer in use instead of using CRYPTO_F_* flags to make it easier to add more types in the future if needed (e.g. wired userspace buffers for zero-copy). It will also make it easier to re-introduce separate input and output buffers (in-kernel TLS would benefit from this).

    Try to make the flags related to IV handling less insane:
    • CRYPTO_F_IV_SEPARATE means that the IV is stored in the 'crp_iv' member of the operation structure. If this flag is not set, the IV is stored in the data buffer at the 'crp_iv_start' offset.
    • CRYPTO_F_IV_GENERATE means that a random IV should be generated and stored into the data buffer. This cannot be used with CRYPTO_F_IV_SEPARATE.

      If a consumer wants to deal with explicit vs implicit IVs, etc. it can always generate the IV however it needs and store partial IVs in the buffer and the full IV/nonce in crp_iv and set CRYPTO_F_IV_SEPARATE.

      The layout of the buffer is now described via fields in cryptop. crp_aad_start and crp_aad_length define the boundaries of any AAD. Previously with GCM and CCM you defined an auth crd with this range, but for ETA your auth crd had to span both the AAD and plaintext (and they had to be adjacent).

      crp_payload_start and crp_payload_length define the boundaries of the plaintext/ciphertext. Modes that only do a single operation (COMPRESS, CIPHER, DIGEST) should only use this region and leave the AAD region empty.

      If a digest is present (or should be generated), it's starting location is marked by crp_digest_start.

      Instead of using the CRD_F_ENCRYPT flag to determine the direction of the operation, cryptop now includes an 'op' field defining the operation to perform. For digests I've added a new VERIFY digest mode which assumes a digest is present in the input and fails the request with EBADMSG if it doesn't match the internally-computed digest. GCM and CCM already assumed this, and the new AEAD mode requires this for decryption. However, ETA mode now supports doing a verify as well (so in this tree IPsec always requests that the crypto layer verify the digest instead of doing it in IPsec). Simple DIGEST operations can also do this, though there are no in-tree consumers.

      To eventually support some refcounting to close races, the session cookie is now passed to crypto_getop() and clients should no longer set crp_sesssion directly.
  • Assymteric crypto operation structures should be allocated via crypto_getkreq() and freed via crypto_freekreq(). This permits the crypto layer to track open asym requests and close races with a driver trying to unregister while asym requests are in flight.
  • crypto_copyback, crypto_copydata, crypto_apply, and crypto_contiguous_subsegment now accept the 'crp' object as the first parameter instead of individual members. This makes it easier to deal with different buffer types in the future as well as separate input and output buffers. It's also just simpler for driver writers to use.
  • bus_dmamap_load_crp() loads a DMA mapping for a crypto buffer. This understands the various types of buffers so that drivers that use dma do not have to be aware of different buffer types.
  • Helper routines now exist to build an auth context for HMAC IPAD and OPAD. This reduces some duplicated work among drivers.
  • Key buffers are now treated as const throughout the framework and in device drivers. However, session key buffers provided when a session is created are expected to remain alive for the duration of the session.
  • GCM and CCM sessions now only specify a cipher algorithm and a cipher key. The redundant auth information is not needed or used.
  • For cryptosoft, split up the code a bit such that the 'process' callback now invokes a function pointer in the session. This function pointer is set based on the mode (in effect) though it simplifies a few edge cases that would otherwise be in the switch in 'process'.

    It does split up GCM vs CCM which I think is more readable even if there is some duplication.
  • I changed /dev/crypto to support GMAC requests using CRYPTO_AES_NIST_GMAC as an auth algorithm and updated cryptocheck to work with it.
  • Combined cipher and auth sessions via /dev/crypto now always use ETA mode. The COP_F_CIPHER_FIRST flag is now a no-op that is ignored. This was actually documented as being true in crypto(4) before, but the code had not implemented this before I added the CIPHER_FIRST flag.
  • I have not yet updated /dev/crypto to be aware of explicit modes for sessions. I will probably do that at some point in the future as well as teach it about IV/nonce and tag lengths for AEAD so we can support all of the NIST KAT tests for GCM and CCM.
  • I've split up the exising crypto.9 manpage into several pages of which many are written from scratch.
  • I have converted all drivers and consumers in the tree and verified that they compile, but I have not tested all of them. I have tested the following drivers:
    • cryptosoft
    • aesni (AES only)
    • blake2
    • ccr

      and the following consumers:
    • cryptodev
    • IPsec
    • ktls_ocf
    • GELI (lightly)

      I have not tested the following:
    • ccp
    • aesni with sha
    • hifn
    • kgssapi_krb5
    • ubsec
    • padlock
    • safe
    • armv8_crypto (aarch64)
    • glxsb (i386)
    • sec (ppc)
    • cesa (armv7)
    • cryptocteon (mips64)
    • nlmsec (mips64)
Test Plan
  • cryptocheck of tested drivers
  • have tested ktls_ocf using normal KTLS testing (nginx)
  • tested IPsec with pinging while over a tunnel for most of the supported algorithms in the tree (not able to test IPcomp since I couldn't get it to work in stock FreeBSD)
  • tested GELI using a memory disk with aesni0, software, and ccr0 as backing drivers
  • make tinderbox

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30113
Build 27920: arc lint + arc unit

Event Timeline

notes
2

I won't commit this file, but it lives in the branch and ended up in the review as a result.

share/man/man4/crypto.4
151

Will bump Dd on all affected manpages at time of commit.

sys/crypto/armv8/armv8_crypto.c
292–298

This applies r342024 to this driver which was copied from aesni,

sys/crypto/ccp/ccp_hardware.c
1703

Hmm, in ccr I ended up renaming 'authenc' to 'eta'. Might be prudent to do the same in this driver post-commit.

sys/dev/cxgbe/crypto/t4_crypto.c
1085–1086

ETA always requires checking the tag, so I can G/C this workaround now.

sys/dev/glxsb/glxsb.c
494

Need to fix the accellerate typo here and in padlock.

sys/dev/hifn/hifn7751.c
2709

The fact that hifn(4) wants to replace the mbuf pointer in the crp is pretty crappy. I think it might happen to work by accident with IPsec, but it doesn't seem like the right thing to me to be changing the pointer. I added an XXX for ubsec(4) which has copied and pasted all the craziness from hifn(4) here but then fails to actually update the mbuf pointer in crp. All of these shenanigans appear to be trying to avoid the second copy of the data back into the original mbuf when using normal bus_dma bounce buffers. I feel like the driver should just use bounce buffers if it has to.

sys/dev/ubsec/ubsec.c
1442

I guess the original version did actually update crp_mbuf. Sigh. I guess I can at least replicate what I did for hifn(4) here then.

sys/geom/eli/g_eli_privacy.c
251

I removed this "optimization" because we need to use crypto_getreq to get proper reference counting on sessions in the future. OTOH, we are at least only allocating a single crp and not the descriptors anymore.

sys/kgssapi/krb5/kcrypto_des3.c
51–52

This crypto consumer was "creative". It created a single session that had both DES and MAC entries when the session was created, but then would only send down requests for either DES or MAC, but not both. To make this work in the new (saner) world order, it now uses separate sessions for DES and MAC.

sys/dev/cxgbe/crypto/t4_crypto.c
1085–1086

Actually, GELI does send these requests still, but it probably shouldn't.

  • Remove OBE workaround from ccr_eta_done.
  • Don't permit DECRYPT | COMPUTE for ETA.
  • accellerate -> accelerate.
  • Let OCF verify HMACs for ETA.
  • Update crp_mbuf instead of leaking q_dst_m.
  • Fix a couple of bugs in my geli ETA changes.
  • Tidy GELI auth failure logging.

I've retested with GELI + AUTH after my changes to mandate verify for ETA and fixed a couple of bugs I had as a result. I also discovered a rather interesting behavior change. Currently, if data fails the authentication check, geli warns about it on the console, but returns the decrypted data (which may be garbage) to the reader instead of failing the I/O request. The version in this branch will instead fail I/O requests with EBADMSG if the authentication fails. For GELI I found that performance was slightly worse, I suspect because I've gone back to doing separate uma_zalloc calls for struct cryptop objects instead of pre-allocating them in the bio_driver2 block. However, I do hope that adding support for separate in vs out buffers (which can avoid the bcopy currently done) will more than offset this in the future.

So GELI is not quite as bad as I said above. If the authentication doesn't match, it doesn't return the output of the decryption operation to the caller. Instead, it leaves whatever happens to be in the caller-provided buffer unchanged. However, this seems possibly bogus as it's not clear if the caller-provided buffer is always sanitized (e.g. it might be better if the current GELI code were to explicitly zero the data that it isn't able to verify to avoid leaking random kernel heap memory). I think I'd prefer to stick with the new semantics (albeit substituting EINTEGRITY for EBADMSG as suggested by @cem). If someone really needs the old semantics it can be re-added.

I've not reviewed the entire patch, but I have converted a (not in-tree) crypto driver to this. It works, and greatly improves the open crypto API from the driver perspective.

Actually, I misread the code. On any corruption mismatch it sets bio_error to -1, and then converts that -1 to EINVAL. The -1 just a hack to avoid double-logging it seems.

  • Some perf numbers from geli.
  • Map EBADMSG to EINTEGRITY for GELI.
  • Allow other errors to supersede EINTEGRITY.
  • Coalesce auth errors.
  • Various fixes to previous.

So GELI with auth now does the same exact logging as before. The only remaining difference is that the error returned is still EINTEGRITY rather than EINVAL.

This revision is now accepted and ready to land.Mar 28 2020, 1:06 AM