Nikos Mavrogiannopoulos nmav@gnutls.org writes:

...

Can you give some example of what you're doing?

I provide an API such as: ctx=cipher_init(cipherno, key) cipher_encrypt(ctx, data) cipher_decrypt(ctx, data) cipher_deinit()

And what does cipherno represent here? Just "AES", or does it imply a key size and/or a mode of operation too?

Does it really matter to differentiate? I mean in my code I wrap stream aead, and block ciphers equally,

With "block ciphers", I imagine you include ECB mode, not just CBC and CTR and the like?

In that case I think you need to pass contexts around as always non-const. It's not entirely clear to me how much casting there will be of the function pointers, hopefully it can be isolated.

But some casting is needed somwehere, if one is going to assign both aes128_encrypt and camellia128_crypt to variables of the same type. Within nettle, that casting is done in the initializers for the various nettle_cipher structs.

And the way I recommend (but maybe that doesn't work for you?) is to use an array of constant structs, similar in spirit to struct nettle_cipher and struct nettle_aead, rather than a big switch.

...

As far as I understand, the versions symbols thing should help. I guess sonames have to be bumped again when version symbols are introduced?

Not needed. But the version with the symbols will be binary incompatible with the previous one, so it may make sense to bump the soversion again to make that clear that they are incompatible.

Can a program linked with nettle-3.0 use a nettle-3.1 library with versioned symbols at runtime? If not, we must have another soname bump.

Would that independence work in practice given that lighoweed is linked with libnettle? I mean if you only bump only nettle, there will be a competing libhogweed with the previous version of nettle if both are installed.

I don't understand all possible cases. But I think it's nice if you don't have to upgrade a program using only interfaces from one of the libraries, just because you upgraded to a newer version of the nettle packages, and the other library had an incompatible change.

Symbol versioning would save the day there too though :)

If we introduce symbol versioning, it seems good to do it for both libraries at the same time.

Regards, /Niels

Nikos Mavrogiannopoulos nmav@gnutls.org writes:

On Tue, 2014-11-25 at 07:28 +0100, Niels Möller wrote:

...

And what does cipherno represent here? Just "AES", or does it imply a key size and/or a mode of operation too?

Yes, something like AES-128-CBC.

I see. Makes sense.

I don't have ecb mode at all since it is not used by TLS (or any other protocol). Just cbc, and gcm for now.

Also makes sense. Then you probably should use the nettle_cipher_func type as little as possible. Rather, something like

struct aes128_cbc_ctx CBC_CTX(struct aes128); nettle_crypt_func aes128_cbc_encrypt;

/* If you're going to call this function via a generic function pointer only, there's no gain to have a precise context type, it can just as well take a void * argument and cast internally. */ void aes128_cbc_encrypt (void *p...) { struct aes128_cbc_ctx *ctx = (struct aes128_cbc_ctx *) p; CBC_ENCRYPT (ctx, ...); }

const struct gnutls_cipher aes128_cbc = { .name = "aes128-cbc", .size = sizeof(aes128_cbc_ctx), .encrypt = aes128_cbc_encrypt, ... };

And to make the contexts structs exposed to library users more self-contained, they can have as first element an "is_a"-pointer to the corresponding gnutls_cipher, followed (pointer, or directly in the same object) by the internal cipher-specific context. I guess you already do something a bit like that?

...

Can a program linked with nettle-3.0 use a nettle-3.1 library with versioned symbols at runtime? If not, we must have another soname bump.

No it will not.

So nettle-3.1 must have a new soname, if it's going to use versioned symbols (which I'm leaning towards doing).

I think my patch addresses that for both.

I think it lacked a linker script for hogweed, with symbol version tracking libhogweed's major version rather than libnettle's.

What I now realize is that that mini-gmp mpz_*, gmp_* and mpn_* symbols are not exported in the script that I sent. They could be put unconditionally there, or via a configure variable.

I think it's generally a bit dangerous to make shared libraries with mini-gmp (the result is not promised to be binary compatible with regular builds). Anyone trying that really needs to know what he/she is doing. So I think a sensible default is to disable hogweed symbol versioning in that case, and possibly have a configure option to explicitly specify the linker script to use for each library.

Regards, /Niels

Nikos Mavrogiannopoulos nmav@gnutls.org writes:

The disadvantage here is that I need to define encrypt and decrypt functions for each possible cipher and mode. That was the reason for the usage of cbc_encrypt() and decrypt.

Maybe it's possible with some trickery.

For cbc modes, you could allocate a context which is a pointer to the nettle_cipher struct followed by a CBC_CTX with cipher context and iv. Then you could do

nettle_crypt_func gnutls_cbc_encrypt;

struct gnutls_cbc_ctx { const struct nettle_cipher *cipher;

/* A cipher specific context, followed by iv. Use unsigned long as an alignment hack (or use pointers and additional memory blocks). Total allocation size:

offsetof (struct gnutls_cbc_ctx, buf) + cipher->context_size + cipher->block_size. */ unsigned long buf[1]; };

void gnutls_cbc_encrypt (void *p, ...) { struct gnutls_cbc_ctx *ctx = (struct gnutls_cbc_ctx *) p; void *cipher_ctx = ctx->buf; uint8_t *iv = (uint8_t *) cipher_ctx + ctx->cipher->context_size; cbc_encrypt (cipher_ctx, ctx->cipher->encrypt, ctx->cipher->block_size, iv, ...); }

and use the same encrypt function for all ciphers in cbc mode. Not sure what's best, to go this route, or generate boilerplate code for each algorithm.

And note that the only casting above is for packing everything into the same memory block, otherwise the cbc_encrypt call needs no casts at all. (The essential casts are hidden in the initialization of the various nettle_cipher instances).

btw. I realized that nettle-meta.h lacks definitions for 3des, des and salsa20.

des and des3 (and blowfish) are missing, because their set_key functions have a return value indicating weak keys. Maybe they should be added anyway? Either relying on casting betwen function pointers being safe with and without return value, or using a wrapper function to explicitly remove it.

Unfortunately, I don't think a C compiler can optimize

int bar (int x); void foo(int x) { (void) bar(x); /* Ignore return value */ }

to make foo simply an alias of bar (assuming the ABI calling ocnventions allow that). As far as I understand, the C standard requires that &bar != &foo, so at best foo wil be compiled into a single jmp instruction.

And salsa20 is missing because there is no nettle-meta abstraction for stream ciphers. Internally, I use nettle_aead with NULL ->update and ->digest methods to represent stream ciphers, but I'm not sure if that's suitable for a public interface. BTW, it's of course possible to represent CBC and CTR mode in the same way, as nettle_aead without authentication.

Regards, /Niels

Nikos Mavrogiannopoulos nmav@gnutls.org writes:

I've done something similar at: https://gitorious.org/gnutls/gnutls/source/0a1102311e443720fc0eb7a83f7dc1b58...

Looks reasonable. You get a pretty large nettle_cipher_st to support all variations, I see. Some comments:

_ccm_aes_encrypt looks like it supports ccm with any cipher, right?

If the aead_*crypt_func functions are intended as the primary interface for aead, I guess you'd want to either retire the auth and tag functions (and you may want additonal helper functions in nettle to make that easy?). Or implement the aead_*crypt functions in terms of the auth and tag functions for everything but ccm.

For the _gcm functions, do you see any obstacles to implementing generic _gcm functions (like you do with cbc), which passes the encrypt_block function to nettle's general gcm functions? Note that GCM_CTX intentionally puts the cipher-specific context last, so the offset should always be the same.

For function typedefs, in nettle I've chosen to let these types be non-pointers. This is a question of style, of course. If you did the same, you would be able do use them to declare functions like

static encrypt_func _cbc_encrypt;

and then the compiler will check that the definition of _cbc_encrypt matches the declaration. Which I think is nice.

Your auth_func, tag_func, set_key_func and setiv_func seem to duplicate nettle_hash_update, nettle_set_key_func and nettle_hash_digest_func. If they are intended to work with nettle's context structs (which they are, I think?), maybe it's clearer to use nettle's names for these types, to differentiate them from the function types which work with your more abstract nettle_cipher_ctx.

My impression is that there are two high level abstractions: AEAD ciphers (cipher+mode+tag), and non-aead cipher+mode (just encryption). Stream ciphers are simply ciphers with a fixed mode.

And nettle currently lacks an abstraction (of the nette-meta.h style) for the second type. nettle_cipher is for "raw" block ciphers, ECB-mode, no state.

Using struct nettle_aead could work. The update and digest methods would be NULL. The set_nonce method would be NULL for arcfour and any other traditional stream cipher, and non-NULL for block cipher modes like cbc and ctr, as well as for salsa20 and chacha. (Supporting cbc in this way has padding issues, though, not sure how to deal with that).

But using nettle_aead is not ideal naming. Maybe it should be done differently.

(And I don't think nettle should include meta objects for all possible combinations of ciphers and modes, but it could include the most important ones, and make it easy for an application to define additional combinations).

Regards, /Niels

Nikos Mavrogiannopoulos nmav@gnutls.org writes:

Do you mean something like: struct gcm_cast_st { struct gcm_key key; struct gcm_ctx gcm; char xx[1]; }; #define GCM_CTX_GET_KEY(ptr) (&((struct gcm_cast_st*)ptr)->key) #define GCM_CTX_GET_CTX(ptr) (&((struct gcm_cast_st*)ptr)->gcm) #define GCM_CTX_GET_CIPHER(ptr) ((void*)&((struct gcm_cast_st*)ptr)->xx)

It looks indeed simpler. Not sure how valid is the GCM_CTX_GET_CIPHER cast though. It would be nice if nettle's gcm.h provided such macros.

Only potential problem I see is alignment. You could make it an unsigned long array rather than a char array. And I'd drop the GET_KEY and GET_CTX macros; they're perfectly normal struct accesses.

You could perhaps use GCM_CTX, from nettle/gcm.h, with a generic cipher type, say

struct gcm_st GCM_CTX(unsigned long);

but I'm not sure that's better than making your own. Of the other GCM_* macros in nettle/gcm.h, I think they might work with the above context, provided that the passed in encrypt function takes a void * (or const void *) as the first argument.

Regards, /Niels

On Tue, Nov 25, 2014 at 10:36 AM, Niels Möller nisse@lysator.liu.se wrote:

I think it lacked a linker script for hogweed, with symbol version tracking libhogweed's major version rather than libnettle's.

Attached a patch which separates the .map files.

...

What I now realize is that that mini-gmp mpz_*, gmp_* and mpn_* symbols are not exported in the script that I sent. They could be put unconditionally there, or via a configure variable.

I think it's generally a bit dangerous to make shared libraries with mini-gmp (the result is not promised to be binary compatible with regular builds). Anyone trying that really needs to know what he/she is doing. So I think a sensible default is to disable hogweed symbol versioning in that case, and possibly have a configure option to explicitly specify the linker script to use for each library.

I'm not sure what is the correct path. Disabling the linker script would mean that more symbols will be exported than normally (all internal symbols) and there can be clashes that wouldn't be with the normal library. In any case this is not really related to the version script, it is mostly an issue of having a different soname when nettle-mini is used.

regards, Nikos