> On May 31, 2023, at 1:02 PM, David Howells <[email protected]> wrote:
>
> Hi Herbert, Chuck,
>
> I'm wondering how to make progress on getting my krb5 crypto lib upstream.
>
> Can I push it as it stands and then we try and build it into the crypto API
> for it later? That would allow me to push my rxgk implementation for AF_RXRPC
> and at least allow userspace to use that.
Sharing our private conversation during LSF here:
My feeling is crypto/krb5 needs at least one consumer, and
it makes sense for the first one to be AFS.
Once crypto/krb5 is in the tree, I'll be glad to look at
starting to replace the internals of the SunRPC GSS Kerberos
mechanism with what is provided in crypto/krb5.
However, if there is going to be significant API churn after
crypto/krb5 lands, I'd like to wait until that stabilizes
before adopting major pieces of crypto/krb5 in SunRPC.
> As far as building a crypto API around it goes, we need four interfaces:
>
> (1) Key generation. We may need to generate a {cipher,hash} key pair {Ke,Ki}
> or just a hash key Kc. We might conceivably want both.
>
> At the moment, I return a prepared cipher or a prepared hash. I don't
> deal with the key pairing here as it makes testing a bit more awkward.
>
> int crypto_krb5_get_Kc(const struct krb5_enctype *krb5,
> const struct krb5_buffer *TK,
> u32 usage,
> struct krb5_buffer *key,
> struct crypto_shash **_shash,
> gfp_t gfp);
> int crypto_krb5_get_Ke(const struct krb5_enctype *krb5,
> const struct krb5_buffer *TK,
> u32 usage,
> struct krb5_buffer *key,
> struct crypto_sync_skcipher **_ci,
> gfp_t gfp);
> int crypto_krb5_get_Ki(const struct krb5_enctype *krb5,
> const struct krb5_buffer *TK,
> u32 usage,
> struct krb5_buffer *key,
> struct crypto_shash **_shash,
> gfp_t gfp);
>
> (2) PRF+ generation. This takes some a key and a metadata blob and generates
> a new blob that then gets used as a key.
>
> int crypto_krb5_calc_PRFplus(const struct krb5_enctype *krb5,
> const struct krb5_buffer *K,
> unsigned int L,
> const struct krb5_buffer *S,
> struct krb5_buffer *result,
> gfp_t gfp);
>
> (3) Encrypt and Decrypt.
>
> Encrypt has to be parameterised to take a specific confounder for testing
> and generate a random one for normal operation. The IV is fixed all
> zeroes in the cases I've implemented. When testing, decrypt should
> perhaps be passed the confounder to check it.
>
> When encrypting, the output buffer will be larger than the input buffer
> (or, at least, room must be set aside) so that a confounder, padding and
> a checksum can be inserted.
>
> When decrypting, we either want to provide a separate output buffer so
> that the confounder and checksum can be stripped off, or we need to be
> able to find out where the decrypted payload plus the padding will be (we
> can't work out how much padding there is - that's left to the caller).
>
> At the moment, I pass a single buffer descriptor, providing encrypt with
> extra space front and back and providing decrypt with somewhere to save
> offset and length:
>
> ssize_t crypto_krb5_encrypt(const struct krb5_enctype *krb5,
> struct krb5_enc_keys *keys,
> struct scatterlist *sg, unsigned int nr_sg,
> size_t sg_len,
> size_t data_offset, size_t data_len,
> bool preconfounded);
> int crypto_krb5_decrypt(const struct krb5_enctype *krb5,
> struct krb5_enc_keys *keys,
> struct scatterlist *sg, unsigned int nr_sg,
So are we going to stick with struct scatterlist here,
or should it be rather an iterator of some kind?
> size_t *_offset, size_t *_len,
> int *_error_code);
>
> I also allow a krb5/gssapi error code to be returned so that it can be
> used in the generation of abort messages. This needs sorting out
> better. It may be that only one code is actually relevant to this and
> that the caller generates all the rest as it checks the metadata.
>
> The AEAD interface might suffice as it stands if we pass in the keys
> already generated and passed in as a single key blob. We don't want an
> IV generator as the protocol defines the IV to use.
>
> (4) GetMIC and VerifyMIC.
>
> Both of these need parameterising with extra metadata that will get
> inserted into the hash before the data is hashed. GetMIC will insert the
> checksum into the buffer and VerifyMIC will check it and strip it off.
>
> I'm not sure that the hash API is suitable for this. AEAD might suit for
> GetMIC at least, but using AEAD for VerifyMIC would lead to an extraneous
> copy I'd prefer to avoid.
>
>
> ssize_t crypto_krb5_get_mic(const struct krb5_enctype *krb5,
> struct crypto_shash *shash,
> const struct krb5_buffer *metadata,
> struct scatterlist *sg, unsigned int nr_sg,
> size_t sg_len,
> size_t data_offset, size_t data_len);
> int crypto_krb5_verify_mic(const struct krb5_enctype *krb5,
> struct crypto_shash *shash,
> const struct krb5_buffer *metadata,
> struct scatterlist *sg, unsigned int nr_sg,
> size_t *_offset, size_t *_len,
> int *_error_code);
>
> There's a lot to be said in using an asynchronous overrideable interface for
> encrypt and decrypt. The problem is that we want to do simultaneous hash and
> crypt if we can. I think the Intel AES/SHA instructions permit this to be
> done and there is sufficient register space to do it - and I *think* that it
> may be possible to offload this to the Intel QAT or the Intel IAA on the
> latest 4th Gen Xeons - and maybe NICs that can handle NFS/SMB offload.
I agree that a hardware-based AES/SHA implementation of
encrypt/decrypt seems like a good step forward for storage
consumers like NFS and AFS. That could be a significant
benefit for switching SunRPC GSS to crypto/krb5.
> I think we'll perhaps need a "krb5 encoding type" API that can provide key
> derivation, PRF+ and information - something along the following lines:
>
> struct krb5_enctype {
> int etype; // Encryption (key) type
> int ctype; // Checksum type
> const char *name; // "Friendly" name
> const char *encrypt_name; // Crypto encrypt name
> const char *cksum_name; // Crypto checksum name
> const char *hash_name; // Crypto hash name
> u16 block_len; // Length of encryption block
> u16 conf_len; // Length of confounder
> u16 cksum_len; // Length of checksum
> u16 key_bytes; // Length of raw key
> u16 key_len; // Length of final key
> u16 hash_len; // Length of hash
> u16 prf_len; // Length of PRF() result
> u16 Kc_len; // Length of Kc
> u16 Ke_len; // Length of Ke
> u16 Ki_len; // Length of Ki
> bool keyed_cksum; // T if a keyed cksum
> bool pad; // T if should pad
> };
>
> We need to be able to look the encoding up by ID, not by name.
It's not clear why something like this would need to be
exposed to crypto/krb5 consumers. There are a few items
in here that XDR needs to know about (lengths and such)
but that kind of thing can be provided by a function
call rather than by having direct access to a structure.
--
Chuck Lever
Chuck Lever III <[email protected]> wrote:
> > int crypto_krb5_decrypt(const struct krb5_enctype *krb5,
> > struct krb5_enc_keys *keys,
> > struct scatterlist *sg, unsigned int nr_sg,
>
> So are we going to stick with struct scatterlist here,
> or should it be rather an iterator of some kind?
For my purposes, a scatterlist is more useful as I have an skbuff to work
with - plus I have to pass a scatterlist into the crypto functions inside of
the krb5 lib.
> It's not clear why something like this would need to be
> exposed to crypto/krb5 consumers. There are a few items
> in here that XDR needs to know about (lengths and such)
> but that kind of thing can be provided by a function
> call rather than by having direct access to a structure.
Fair point. In rxgk, I use key_len, key_bytes, block_len, cksum_len plus the
name for procfs purposes. I also wonder if I need separate key_len and
key_bytes if I'm not supporting DES (DES keys gets expanded IIRC). Also, some
of the checks I'm doing could perhaps be moved into the krb5 lib.
The krb5 selftest code makes use of more of the fields, but I guess that's
internal to krb5lib.
David
On 5/31/2023 4:08 PM, David Howells wrote:
> Fair point. In rxgk, I use key_len, key_bytes, block_len, cksum_len plus the
> name for procfs purposes. I also wonder if I need separate key_len and
> key_bytes if I'm not supporting DES (DES keys gets expanded IIRC). Also, some
> of the checks I'm doing could perhaps be moved into the krb5 lib.
The "K" in RXGK is RFC3961 without support for weak ciphers. No DES, no
3DES
and no RC4-HMAC. DES keys are never expanded.
The supported ciphers are
* aes128-cts-hmac-sha1-96 (RFC3962)
* aes256-cts-hmac-sha1-96 (RFC3962)
* aes128-cts-hmac-sha256-128 (RFC8009)
* aes256-cts-hmac-sha384-192 (RFC8009)
There are other Kerberos ciphers that could be used with RXGK but there
are no RXGK server implementations that use them. None of the RFC3961
ciphers or the RFC3961 interfaces support AEAD modes.
Luke Howard proposed "AEAD Encryption Types for Kerberos 5"
https://datatracker.ietf.org/doc/draft-howard-krb-aead/ to IETF Kitten
which would add AES128 and AES256 GCM, CCM, and OCB modes. However,
there is some resistance to these additions because at the moment all
RFC3961 ciphers are safe for use with long term keys and repeating
cipher state; AEAD modes are not.
RXGK can be constrained such that it is safe for use with AEAD modes and
I would like to see Luke's draft be adopted if only because CTS-HMAC is
not supported by Intel QAT and GCM is. Adoption of Luke's draft would
not only benefit AuriStorFS but NFSv4 gss-krb5 as well.
My suggestion is that the kernel should provide an RFC3961 API for use
by gss_krb5 applications. AEAD modes can be added to that if and when
Luke's draft is adopted.
Jeffrey Altman