Page MenuHomeFreeBSD

ossl: Add Poly1305 digest support.
ClosedPublic

Authored by jhb on Feb 17 2021, 10:32 PM.

Details

Diff Detail

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

sys/crypto/openssl/ossl.c
180–183

It seems like we have an extra nest level here. csp_auth_key != NULL should be true IFF csp_auth_klen != 0, and vice versa? (During newsession.)

Nevermind, I guess we could be creating a keyed-hash session without any per-session key.

185–194

The implication is that hash functions with a Setkey operation cannot be HMACs? That's probably fine.

230–231

Why move opad initialization below instead of just doing it here? Avoid a copy?

sys/crypto/openssl/ossl.h
41

this feels brittle, although it's not a regression in this diff

sys/crypto/openssl/ossl_poly1305.c
6

What portions of this file are direct from openssl and what portions are novel?

jhb marked 2 inline comments as done.Mar 2 2021, 10:47 PM
jhb added inline comments.
sys/crypto/openssl/ossl.c
185–194

The implication is that hash functions with a Setkey operation cannot be HMACs? That's probably fine.

Correct.

230–231

Why move opad initialization below instead of just doing it here? Avoid a copy?

It overwrites the single ctx. Basically, we have a single auth ctx on the stack. We either copy it from a saved context when using session keys, or we generate the context on the fly when using per-op keys. Moving opad here would mean having to store two copies on the stack.

sys/crypto/openssl/ossl.h
41

this feels brittle, although it's not a regression in this diff

There are static assertions in each of the auth hash files that the context is big enough. The alternative of trying to include the relevant headers is a bit messy.

sys/crypto/openssl/ossl_poly1305.c
143

From here down is new, the #includes are new, and the explicit #define is new, the rest is from OpenSSL.

cem added inline comments.
sys/crypto/openssl/ossl.c
230–231

Ok

sys/crypto/openssl/ossl.h
41

Yeah, that's reasonable. Is uint32 aligned enough for all algorithms and architectures?

sys/crypto/openssl/ossl_poly1305.c
143

Thanks!

149–150

Would it make sense to assert klen matches openssl's expectations?

This revision is now accepted and ready to land.Mar 3 2021, 5:11 AM
jhb marked 4 inline comments as done.Mar 3 2021, 10:51 PM
jhb added inline comments.
sys/crypto/openssl/ossl.h
41

Yeah, that's reasonable. Is uint32 aligned enough for all algorithms and architectures?

It's actually 32 byte alignment for AVX and is sufficient for everything currently.

sys/crypto/openssl/ossl_poly1305.c
149–150

Would it make sense to assert klen matches openssl's expectations?

I can add that.

This revision was automatically updated to reflect the committed changes.