Re: Addition to EGO keys

[TheJackiMonster]

Hi,

On Thu, 2020-11-05 at 14:56 +0100, Schanzenbach, Martin wrote:
> Hi,
> 
> > On 5. Nov 2020, at 14:29, TheJackiMonster <
> > thejackimonster@gmail.com> wrote:
> > 
> > Hi,
> > 
> > the newest changes of the identity service handling EGO keys
> > allowed
> > two different types of keypairs which makes it somewhat more
> > difficult
> > to interact with them.
> > 
> > So I would propose following functions to be added:
> > 
> > ssize_t GNUNET_IDENTITY_read_key_from_buffer(struct
> > GNUNET_IDENTITY_PublicKey *key, const void* buffer)
> > 
> > ssize_t GNUNET_IDENTITY_write_key_to_buffer(const struct
> > GNUNET_IDENTITY_PublicKey *key, void* buffer)
> 
> Is there anything wrong with GNUNET_GNSRECORD_identity_from_data
> and GNUNET_GNSRECORD_data_from_identity ?

I have now looked into the function and it actually contains different
requirements for reading and writing the keys. I also found an issue
which could come up by using one memcpy instead of separate calls for
'type' and key data.

> 
> The "struct GNUNET_IDENTITY_PublicKey" is already a serializ(ed/able)
> data
> structure. So there is not really a need to have those functions.
> If you have a buffer, it must be sizeof (struct
> GNUNET_IDENTITY_PublicKey) to hold
> a public key. Anything less/more is probably faulty data.

The functions I have written do specifically ensure the key is written
or read compact as possible without reading the type separate upfront.

> 
> Of course, you could propose to simply move the GNSRECORD functions
> to identity.
> The reason why it is in GNSRECORD, is because the "short"
> serialization which depends on the
> key length is actually only required by GNS (records).

So I let those functions use the ones I have written now because they
allowed bigger buffers as well and contained other checks which could
maybe be replaced with asserts in the future.

> 
> > 
> > // both would depend on GNUNET_IDENTITY_key_get_length(...) which
> > currently exists
> > 
> > int GNUNET_IDENTITY_public_key_verify(uint32_t purpose, const
> > struct
> > GNUNET_CRYPTO_EccSignaturePurpose *validate, const struct
> > GNUNET_IDENTITY_Signature *sig, const struct
> > GNUNET_IDENTITY_PublicKey
> > *pub)
> > 
> > int GNUNET_IDENTITY_private_key_sign(const struct
> > GNUNET_IDENTITY_PrivateKey *priv, const struct
> > GNUNET_CRYPTO_EccSignaturePurpose *purpose, struct
> > GNUNET_IDENTITY_Signature *sig)
> > 
> > // and GNUNET_IDENTITY_Signature would look like following:
> > 
> > struct GNUNET_IDENTITY_Signature {
> >  uint32_t type;
> >  union {
> >    struct GNUNET_CRYPTO_EcdsaSignature ecdsa_signature;
> >    struct GNUNET_CRYPTO_EddsaSignature eddsa_signature;
> >  }
> > }
> 
> Yes, this is something I omitted because you will have to change a
> lot of code
> to make this consistent. And you will have the same serialization
> issue as
> signature sizes may vary.
> In most cases, you do not want the signature to be a union but as
> short as possible.
> Which means find all occurrences in the GNUnet code that uses
> signatures and use
> apply serialization function.

I could already change some occurences where signatures get used. I
think it won't be necessary to change all signatures but only the ones
using EGO keys. So I think I got most cases which would be necessary.

One of the cases which I'd like to change would be publishing files but
this could require more changes and cause incompatibility, I think.

> 
> > 
> > At least similar changes would ease the use of EGOs for
> > authentification which will be used for example in the messenger
> > service. I am also open for ideas to handle it differently. Maybe
> > there
> > is planned something else with EGOs or how authentification should
> > be
> > handled? Otherwise I would open a commit to add those functions
> > which
> > would pretty much check for the type of public/private key and use
> > existing functions from GNUnet.
> 
> You can try, but make sure tests still pass for all components :)

I have run all tests and even though one test fails and one test skips.
That's the same result with the same messages in the log, I get from
the main branch currently. So I would think, I didn't break anything.

> 
> > 
> > I also wanted to ask how I could use the EGO keys for encryption of
> > private messages. I know the functionality for signatures and
> > verification but I would like to use them for encryption.
> 
> You do not use the EC key directly as you would for RSA.
> Instead, you should use ECDHE. Say you want to encrypt for "Bob":
> 
> - Get the public key P_Bob from Bob
> - Generate an ephemeral ECDH key pair P_ecdh, K_ecdh
> - Calculate k = ECDH(P_Bob, K_ecdh)
> - Use k to symmetrically encrypt message M: C = AES(k, M)
> - Send (P_ecdh,C) to Bob
> - Bob calculates k = ECDH(K_Bob, P_ecdh)
> - Bob decrypts M = AES(k, C)
> 
> > 
> > If there is a systematic reason to not use EGO keys for that
> > purpose,
> > how would I use asymmetric encryption with GNUnet then? I assume I
> > would use libsodium directly otherwise?
> > 
> 
> You can encapsulate the encryption for EGOs, of course, just like
> signatures as you proposed.

I've implemented as mentioned and added some cleanup for keys and
hashes after they were used. I'm not sure if that's necessary for local
variables but I thought it's better in the code to be removed than
forgotten and never added later. ^^'

All changes are in the branch 'jacki/identity'. If there is no general
problem with them, I would like to merge to the main branch.

BR
Jacki

> 
> BR
> Martin
> 
> > Happy Hacking
> > Jacki
>

signature.asc
Description: This is a digitally signed message part