> On May 31, 2023, at 1:02 PM, David Howells <dhowells@xxxxxxxxxx> 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
