Hi, I finally got back to this patch again. I made changes to the pull request. The changelog:
- Got rid of the hpke_ctx structure and put everything as function parameters. I read your email then where you suggest to have a context for the symmetric part, which I think makes sense, but I want your input on the contextless version before making any changes. It may be easier to review at this stage. - All the non-BASE mode was removed - All the algorithms other than P256, SHA256 and AES128 were removed - The structures for the algorithms were put as public (in hpke.h) - I got an idea to get rid of the hpke_setup_* functions and leave the process to the user. The process is straightforward and leaves the API a lot cleaner. The contents of the setup functions can be used in the documentation as an example for setting up a sender/receiver. - Because of the change above the function hpke_encap, hpke_decap and hpke_key_schedule moved to the public header file. - The tests were adjusted too, but without much time invested in it Please take a look and let me know what do you think. Regards Norbert Pócs On Tue, Dec 13, 2022 at 4:48 PM Norbert Pocs <[email protected]> wrote: > Sorry for the late reply, I got snowed in by work. > I will continue to work on this in the New year. > > We need an object responsible for the symmetric encryption/decryption, >> similar to the Context type in the spec. It's not clear to me if we >> need separate types for sender and receiver (as suggested in the spec) >> or if we can use the same type. >> > > iirc it does not have to be a separate type. > > As far as I see, it doesn't need to know what KDF or KEM was used when >> the context was created. >> > > No, the process is separated after the shared key. > > From the docs: >> >> : @deftypefun int hpke_seal (struct hpke_ctx @var{ctx}, uint8_t >> *@var{aad}, size_t @var{aad_len}, uint8_t *@var{pt}, size_t @var{pt_len}, >> uint8_t **@var{ct}, size_t *@var{ct_len}) >> : Encrypts a plaintext using an hpke context and an >> : optional additional authenticated data into ciphertext. >> : @end deftypefun >> >> I take it hpke_ctx argument should be a pointer, right? >> > > Yes it is, only a typo in the docs. > > >> Is it non-const because it needs to update the sequence number? >> > > Yes. > > >> The aad and pt arguments should be const, I think. pt_len will be known >> by the >> caller, but how can it query for the size of the corresponding cipher >> text? >> > > The ciphertext as well as the size of the ciphertext are output > arguments from the function. The variable is used to return the size > of the ciphertext. > > For other aead modes, nettle use a single length argument, and >> it's the lenght of the destination (ct for encrypto, pt for decrypt), >> > > I am adding it to the TODO list. > > * Then we need some way(s) way to create/initialize such a context, for >> sender and receiver. >> >> One way would be to have general function, which gets kdf, kem and >> aead to use as arguments. >> > > This is how it is done in the PR atm. > > Or would could take out some or all of that >> parameterization and have one function for each configuration. >> > > Would this approach bring anything valuable in difference to the first > approach? > > I'm afraid I don't know what is or isn't approved in FIPS 140-3. I think >> point validation might be simpler with curve25519 than with the secp >> curves. I'd prefer initial algorithm based on what you think is most >> useful for applications. >> > > curve25519 looks good to me, other algs can be HKDF-SHA256, AES-128-GCM. > > I think they should be public in that sense. However, there are some >> subtle details. Either they are completely public: You declare the types >> in the public hpke.h, and declare (as const extern struct ...) >> instances, details becomes part of the nettle ABI. This is like, e.g., >> const struct nettle_hash nettle_sha256. >> >> Or you only forward declare the type in hpke.h, like >> >> struct hpke_kem; >> >> which actual contents in the private hpke-internal.h. >> >> In that case, you cannot publicly declare constants (because in some >> caes that can result in copy relocations making the size of the struct >> leak into the abi, even though contents of the struct was intended to be >> private). Instead, you have to declare accessors, like >> >> const struct hpke_kem *hpke_get_curve25519(void); >> >> This is like struct ecc_curve is handled. >> > > I understand now. I will rework it to be "private" with accessors you are > describing above. > > Not sure what's the right way. But a context with lots of setter methods >> is rather different from most other things in Nettle. >> > > It will at least help with the rework of the context struct. > > >> I often find that it makes things clearer to have separation between the >> separate stages/layers, e.g.: First do algorithm parameterization, >> resulting in a struct representing an instantiation of hpke mode. Then >> use a (const) struct like that + keys to create a session with the >> result of the key schedule. And then use that to process one or more >> messages, with another struct representing per-message state. (Not a >> perfect fit for hpke, but I hope you get the idea. In this case the >> "session" thing would correspond to the "context" in the spec, and if it >> is responsible for the message sequence number, it has to be mutable). >> > > This needs some digestion; a lot of information at once. I have a grasp of > your > vision now and I think more questions will be raised when I sit down and > start working on it. > Until then Merry Christmas and see you in the New Year! > > Regards > Norbert Pócs > > > On Wed, Nov 16, 2022 at 1:20 PM Niels Möller <[email protected]> wrote: > >> Norbert Pocs <[email protected]> writes: >> >> > My idea about the API was: set up a context, do encryption with the >> created >> > context; >> > as mentioned in the standard [2]. >> > The documentation was updated to cover all the functions listed in >> hpke.h >> > In short: >> > - hpke_setup_* sets up the encryption context for sender or receiver >> > in the given mode >> > - hpke_seal: encryption >> > - hpke_open: decryption >> > - export_secret: derives a secret from the internal secret >> >> I've read the spec now, and I think I've have a better understanding of >> what it's doing, although I don't yet grasp all the details. >> >> Let me sketch how I think an api for it could look like. >> >> * We need an object responsible for the symmetric encryption/decryption, >> similar to the Context type in the spec. It's not clear to me if we >> need separate types for sender and receiver (as suggested in the spec) >> or if we can use the same type. >> >> This one needs to hold a context for an AEAD algorithm initialized >> with a secret key, function pointers for doing the actual >> encrypt/decrypt (possibly a const struct nettle_aead*), and the >> message sequence number (could perhaps be an uin64_t, regardless of >> algorithm). >> >> As far as I see, it doesn't need to know what KDF or KEM was used when >> the context was created. >> >> The context should have associated functions like >> encrypt_msg/decrypt_message, maybe "streaming" functions like update, >> encrypt/decrypt/digest, and perhaps some way to access the "export >> secret" feature. >> >> From the docs: >> >> : @deftypefun int hpke_seal (struct hpke_ctx @var{ctx}, uint8_t >> *@var{aad}, size_t @var{aad_len}, uint8_t *@var{pt}, size_t @var{pt_len}, >> uint8_t **@var{ct}, size_t *@var{ct_len}) >> : Encrypts a plaintext using an hpke context and an >> : optional additional authenticated data into ciphertext. >> : @end deftypefun >> >> I take it hpke_ctx argument should be a pointer, right? Is it >> non-const because it needs to update the sequence number? The aad and >> pt arguments should be const, I think. pt_len will be known by the >> caller, but how can it query for the size of the corresponding cipher >> text? For other aead modes, nettle use a single length argument, and >> it's the lenght of the destination (ct for encrypto, pt for decrypt), >> see https://www.lysator.liu.se/~nisse/nettle/nettle.html#Conventions. >> >> * Then we need some way(s) way to create/initialize such a context, for >> sender and receiver. >> >> One way would be to have general function, which gets kdf, kem and >> aead to use as arguments. Or would could take out some or all of that >> parameterization and have one function for each configuration. >> >> E.g., we could have something like >> >> hpke_init_curve25519_send(struct hpke_context *ctx, >> const uint8_t *receiver_pubkey, >> const struct hkpe_kdf *kdf, >> const struct nettle_aead *aead); >> >> to initialize the sender's context for sending to a receiver with a >> curve25519 public key. Now there are a few things missing: I think it >> needs to know the hpke id for the aead, and we should also pass in >> some parameters to specify how to get randomness for the ephemeral keys. >> >> If possible, it's also nice if allocation can be left to the caller, >> e.g., something like >> >> hpke_init_curve25519_send(struct hpke_context *ctx, >> const struct nettle_aead *aead, >> void *aead_ctx, >> const uint8_t *receiver_pubkey, >> const struct hkpe_kdf *kdf); >> >> * It may make sense to have one object that is responsible for a hpke >> configuration (mainly, which algorithms and which mode to use, maybe >> randomness source), and have functions which takes such a (const) >> objects + relevant keys and initializes a context like above. >> >> > To narrow the scope of the review I suggest to start with the base mode >> > and for the algorithms perhaps with one of the FIPS 140-3 approved >> ones, but >> > otherwise I have not much preference in the algorithms. >> >> I'm afraid I don't know what is or isn't approved in FIPS 140-3. I think >> point validation might be simpler with curve25519 than with the secp >> curves. I'd prefer initial algorithm based on what you think is most >> useful for applications. >> >> Starting with base mode only makes sense to me. >> >> > I am a bit confused on the external structs. How I understood was that >> the >> > users can use it for the context creation - which algorithms they >> prefer to >> > use. Therefore it should be public so they can pass it to >> `hpke_ctx_init` >> > or do I understand it wrong? >> >> I think they should be public in that sense. However, there are some >> subtle details. Either they are completely public: You declare the types >> in the public hpke.h, and declare (as const extern struct ...) >> instances, details becomes part of the nettle ABI. This is like, e.g., >> const struct nettle_hash nettle_sha256. >> >> Or you only forward declare the type in hpke.h, like >> >> struct hpke_kem; >> >> which actual contents in the private hpke-internal.h. >> >> In that case, you cannot publicly declare constants (because in some >> caes that can result in copy relocations making the size of the struct >> leak into the abi, even though contents of the struct was intended to be >> private). Instead, you have to declare accessors, like >> >> const struct hpke_kem *hpke_get_curve25519(void); >> >> This is like struct ecc_curve is handled. >> >> > Is there a function which can clear memory space after a private key? >> > I see `gmp_free`, but no comments to it. >> >> I think you have to just use memset. >> >> >> * There's quite a lot in struct hpke_ctx. I think it would be good to >> >> think about what parts are (i) algorithm parameterization, (ii) >> >> related to key schedule, (iii) per message state? Can some parts be >> >> moved from the state to inputs or outputs of relevant functions? >> > I didn't want to have enormous function parameters everywhere, that's >> > why I moved the variables there. If you are okay with bulkier function >> > arguments I'll rework this. >> >> Not sure what's the right way. But a context with lots of setter methods >> is rather different from most other things in Nettle. >> >> I often find that it makes things clearer to have separation between the >> separate stages/layers, e.g.: First do algorithm parameterization, >> resulting in a struct representing an instantiation of hpke mode. Then >> use a (const) struct like that + keys to create a session with the >> result of the key schedule. And then use that to process one or more >> messages, with another struct representing per-message state. (Not a >> perfect fit for hpke, but I hope you get the idea. In this case the >> "session" thing would correspond to the "context" in the spec, and if it >> is responsible for the message sequence number, it has to be mutable). >> >> Regards, >> /Niels >> >> -- >> Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. >> Internet email is subject to wholesale government surveillance. >> >> _______________________________________________ nettle-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected]
