cryptosoft: Allocate cipher contexts on the stack during operations.

[jhb]

Summary

As is done with authentication contexts, allocate cipher contexts on
the stack while completing requests. This permits safely dispatching
concurrent requests on a single session. The cipher context in the
session is now only allocated when a session key is provided during
session setup to serve as a template to initialize the on-stack
context similar to auth operations.

Sponsored by: The FreeBSD Foundation

[mjg]

what, if anything, is needed on top of this to remove the lock from swcr_process?

[markj]

Doesn't swcr_authcompute() still modify per-session state when the request specifies a key? In particular, it calls swcr_authprepare() using the session's context rather than one allocated on the stack.

Did you consider making a union encctx rather than using alloca()? I guess the latter is fine, but it's a little weird that we use different mechanisms for pure authentication vs. encryption.

[jhb]

In D33198#750456, @mjg wrote:

what, if anything, is needed on top of this to remove the lock from swcr_process?

I think Mark's comment is about auth state. Fixing that isn't too hard (and maybe I will do that as well). Fixing that would also permit per-op auth keys which we don't currently allow.

[jhb]

In D33198#750566, @markj wrote:

Did you consider making a union encctx rather than using alloca()? I guess the latter is fine, but it's a little weird that we use different mechanisms for pure authentication vs. encryption.

It means exporting various structures that are currently private. I'd honestly rather avoid doing that and letting them be opaque instead. We already have some newer auth ones that don't export their structure and just assert the union is big enough so I'd actually like to kill the union rather than add more of them.

[markj]

In D33198#750875, @jhb wrote:

In D33198#750456, @mjg wrote:

what, if anything, is needed on top of this to remove the lock from swcr_process?

I think Mark's comment is about auth state. Fixing that isn't too hard (and maybe I will do that as well). Fixing that would also permit per-op auth keys which we don't currently allow.

I don't quite follow. The problem with auth state has exactly to do with the handling of per-op auth keys.

The change looks ok to me since it's explicitly about cipher contexts, I just meant to point out that swcr_authcompute() doesn't seem to be thread safe.

sys/opencrypto/cryptosoft.c

110–111

Make sw a pointer to const?

473–474

Can swe and swa be declared const now?

[jhb]

In D33198#750899, @markj wrote:

In D33198#750875, @jhb wrote:

In D33198#750456, @mjg wrote:

what, if anything, is needed on top of this to remove the lock from swcr_process?

I think Mark's comment is about auth state. Fixing that isn't too hard (and maybe I will do that as well). Fixing that would also permit per-op auth keys which we don't currently allow.

I don't quite follow. The problem with auth state has exactly to do with the handling of per-op auth keys.

Oh, sorry. per-op auth keys don't actually work correctly currently as we require a key for new sessions and OCF's model is that you either have a session-wide key, or per-op keys, but not both. cryptosoft breaks that by always requiring a per-session auth key. That is what I'd like to fix, and doing that probably also entails fixing the remaining issues requiring a lock is what I was trying to say.

The change looks ok to me since it's explicitly about cipher contexts, I just meant to point out that swcr_authcompute() doesn't seem to be thread safe.

Yes.

sys/opencrypto/cryptosoft.c

110–111

Oh, that's a good idea, yes.

[markj]

In D33198#751152, @jhb wrote:

In D33198#750899, @markj wrote:

In D33198#750875, @jhb wrote:

In D33198#750456, @mjg wrote:

what, if anything, is needed on top of this to remove the lock from swcr_process?

I think Mark's comment is about auth state. Fixing that isn't too hard (and maybe I will do that as well). Fixing that would also permit per-op auth keys which we don't currently allow.

I don't quite follow. The problem with auth state has exactly to do with the handling of per-op auth keys.

Oh, sorry. per-op auth keys don't actually work correctly currently as we require a key for new sessions and OCF's model is that you either have a session-wide key, or per-op keys, but not both. cryptosoft breaks that by always requiring a per-session auth key. That is what I'd like to fix, and doing that probably also entails fixing the remaining issues requiring a lock is what I was trying to say.

I see now, thanks.