Page MenuHomeFreeBSD

Improve support for stream ciphers in the software encryption interface.
ClosedPublic

Authored by jhb on Mon, May 18, 10:29 PM.

Details

Summary

Add a 'native_blocksize' member to 'struct enc_xform' that stream ciphers
use to report their internal block size. cryptosoft will only pass in
native blocks to encrypt and decrypt hooks. For the final partial block,
'struct enc_xform' now has new encrypt_last/decrypt_last hooks which
accept the length of the final block. The multi_block methods are also
retired.

Mark AES-ICM (AES-CTR) as a stream cipher.

Test Plan
  • cryptosoft -d soft -a all 17 (and various other odd sizes)

AES-XTS still uses a block size of 16 due to lack of ciphertext stealing.

Diff Detail

Repository
rS FreeBSD src 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 requested review of this revision.Mon, May 18, 10:29 PM
jhb created this revision.
jhb updated this revision to Diff 71965.Mon, May 18, 10:31 PM
  • Add crypto.7 change.
jhb added a reviewer: cem.Mon, May 18, 10:32 PM
cem accepted this revision.Tue, May 19, 3:09 AM
cem added inline comments.
share/man/man7/crypto.7
84–85 ↗(On Diff #71965)

Might be better to document stream ciphers as stream ciphers, rather than the sort of confusing 1-byte block size? This is a section 7 manual page, not the implementation details of OCF.

sys/opencrypto/cryptosoft.c
122 ↗(On Diff #71965)

I would do this the other way around; if (native_blocksize == 0) { ...

315 ↗(On Diff #71965)

Ditto here. I would use native_blocksize as the sentinel for "is this a stream cipher that needs final subblock processing?"

For example, you could imagine a cipher with a block size of 1 and no larger native block size (rc4); we would probably leave native_blocksize zero for that, and it definitely does not need special final processing.

Either way is fine and I don't have a compelling argument for my preference.

This revision is now accepted and ready to land.Tue, May 19, 3:09 AM
jhb marked 2 inline comments as done.Wed, May 20, 9:27 PM
jhb added inline comments.
share/man/man7/crypto.7
84–85 ↗(On Diff #71965)

Hmmm, perhaps that should be done as a followup in some way since it would apply to GCM as well.

sys/opencrypto/cryptosoft.c
315 ↗(On Diff #71965)

I will do this. I think it also fits a bit better with something like XTS which isn't a stream cipher, per se, but does permit a special case for the last block via the context stealing.

jhb updated this revision to Diff 72056.Wed, May 20, 10:20 PM
jhb marked an inline comment as done.
  • Depend on native_blocksize != 0 instead of blocksize == 1.
This revision now requires review to proceed.Wed, May 20, 10:20 PM

Just wanted to let you know that this patch will have some ramifications when it comes to using AES-CTR in ipsec.
Before sending a frame its payload is aligned to the block size of the cipher used, in case of AES-CTR it was 16 bytes before this patch.
Since old versions of FreeBSD can't decrypt partial blocks of AES-CTR, ipsec tunnels that use this cipher, established between FreeBSD with this patch applied and without won't work.
I think that we will have the same problem with OpenBSD too.
With that being said RFC3686 specifies that payload should not be aligned to block size when using AES-CTR.
In particular Linux doesn't do that, so right now we have the same problem with ipsec between Linux host and FreeBSD.

I've tested this patch and confirmed that it fixes AES-CTR tunnels with a Linux host.

I suppose that the compatibility issue could be fixed by enforcing a 16byte alignment when AES-CTR is used in esp_output routine.
Any thoughts?

jhb added a comment.Fri, May 22, 1:54 PM

Arguably this means we are now compliant with the RFC when we weren't before I think? But I think I want to better understand what you are saying:

  1. Can Linux still decrypt a message sent with the extra padding bytes ok, it's just that we couldn't decrypt packets sent by Linux using AES-CTR previously?
  1. I think if the answer to that is yes, then it means adding the padding won't break the now-fixed Linux interop?

I think that perhaps the right way to handle this if both 1) and 2) are true would be to add a sysctl knob (it could even default to on for now) to add the padding in esp_output() for AES-CTR explicitly. If the answers to 1) or 2) are no, then we might want to make the default setting of the sysctl be off instead of on.

cem added a comment.Fri, May 22, 2:05 PM

If I understand correctly, we are compliant now and previously relied on non-compliant behavior as a performance optimization; either for receive (where we only accepted aligned packets) or in both send and receive (if recipients had to be non-standard to decode our AES-aligned packets). If so, I agree that an ipsec-specific sysctl to control AES-CTR alignment for backwards compatibility may be the way to go. I don't understand the situation well enough to recommend a default direction.

In general, to the extent it is incompatible with the conforming Linux implementation, I would not recommend the old design choice here — speaking non-conformant protocol for the sake of performance. That said, it exists, and backwards compatibility is nice if we can do it.

Arguably this means we are now compliant with the RFC when we weren't before I think?

Correct, to be more specific RFC states that:

To encrypt a payload with AES-CTR, the encryptor partitions the
plaintext, PT, into 128-bit blocks. The final block need not be 128
bits; it can be less.

  1. Can Linux still decrypt a message sent with the extra padding bytes ok, it's just that we couldn't decrypt packets sent by Linux using AES-CTR previously?

Yes, exactly. Old version of swcr_encdec simply couldn't handle partial blocks.

  1. I think if the answer to that is yes, then it means adding the padding won't break the now-fixed Linux interop?

I've hardcoded blocksize used for padding in sys/netipsec/xform_esp.c:655 to 16 and a tested it by pinging a linux host over ipsec tunnel.
By looking at tcpdump frames sent by us are aligned to 16 bytes as expected and everything works just fine.

If I understand correctly, we are compliant now and previously relied on non-compliant behavior as a performance optimization; either for receive (where we only accepted aligned packets) or in both send and receive (if recipients had to be non-standard to decode our AES-aligned packets). If so, I agree that an ipsec-specific sysctl to control AES-CTR alignment for backwards compatibility may be the way to go. I don't understand the situation well enough to recommend a default direction.

I don't think that it was caused by performance reasons, but rather by lack of support for processing partial blocks in swcr_encdec.
Because of that we had to align data before encrypting and sending it and we could only decrypt aligned data for the same reason.

By the way the issue is still present when using aesni even though it can parse partial blocks.
ipsec code (sys/netipsec/xform_esp.c) relies on blocksize read from enc_xform structure - it will drop a frame if its payload is not divisable by it.

jhb added a comment.Fri, May 22, 3:42 PM

Yes, it is the block size in enc_xform_aes_icm that matters. Do you want to work on the sysctl as a followup change to this or would you like me to do so? Since I don't plan to MFC any of this I think it is fine for there to be a narrow gap in HEAD before the compat sysctl shows up.

jhb added a comment.Fri, May 22, 3:44 PM

Also, on a separate question, do you have any of your clients at semi half that are interested in using chacha20-poly1035 in AEAD mode for IPsec? (RFC 7634) Note that both IPsec and TLS use a specific AEAD mode that combines these two that isn't the "simple" ETA mode used for AES-CBC, AES-CTR, Cameilla-CBC + HMAC.

Yes, it is the block size in enc_xform_aes_icm that matters. Do you want to work on the sysctl as a followup change to this or would you like me to do so? Since I don't plan to MFC any of this I think it is fine for there to be a narrow gap in HEAD before the compat sysctl shows up.

I can prepare a patch early next week.

Also, on a separate question, do you have any of your clients at semi half that are interested in using chacha20-poly1035 in AEAD mode for IPsec? (RFC 7634) Note that both IPsec and TLS use a specific AEAD mode that combines these two that isn't the "simple" ETA mode used for AES-CBC, AES-CTR, Cameilla-CBC + HMAC.

I'm not sure. I'll ask and get you the answer next week.

This revision was not accepted when it landed; it landed in state Needs Review.Fri, May 22, 4:29 PM
This revision was automatically updated to reflect the committed changes.