Nikos Mavrogiannopoulos nmav@redhat.com writes:

On Thu, 2017-09-28 at 21:48 +0200, Niels Möller wrote:

...

To get minimal ABI breakage, I also suspect we would need a release branch where I revert recent changes that grow the size of struct ecc_curve; my idea is to introduce functions returning pointers to the instances of this struct. But as long as applications are using the data symbols directly as advertised in ecc-curves.h, executables with R_X86_64_COPY relocations will break, in the same way as arrays like nettle_hashes.

I agree that this is the most important to address. I don't think I have any good suggestion in addressing that.

Below is my work-in-progress patch. Comments appreciated.

diff --git a/ChangeLog b/ChangeLog index baf6c13..da614f6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,8 +1,40 @@ +2017-04-09 Niels Möller nisse@lysator.liu.se + + * ecc-curve.h (nettle_get_secp_192r1, nettle_get_secp_224r1) + (nettle_get_secp_256r1, nettle_get_secp_384r1) + (nettle_get_secp_521r1): New functions, returning a pointer to + corresponding structure. + (nettle_secp_192r1, nettle_secp_224r1, nettle_secp_256r1) + (nettle_secp_384r1, nettle_secp_521r1): Redefined as macros, + calling the corresponding function. + + * nettle-meta.h (nettle_ciphers, nettle_aeads, nettle_armors): New + macros, analogous to below change to nettle_hashes. + + * nettle-meta-ciphers.c (nettle_get_ciphers): New function. + + * nettle-meta-aeads.c (nettle_get_aeads): New function. + + * nettle-meta-armors.c (nettle_get_armors): New function. + +2017-01-12 Niels Möller nisse@lysator.liu.se + + * tools/nettle-hash.c (find_algorithm): Deleted function. + (main): Replaced by call to nettle_lookup_hash. + + * testsuite/meta-hash-test.c (test_main): Use nettle_lookup_hash. + + * nettle-meta.h (nettle_hashes): New macro, expanding to a call to + nettle_get_hashes. Direct access to the array causes the array + size to leak into the ABI, since a plain un-relocatable executable + linking with libnettle.so gets copy relocations for any referenced + data items in the shared library. + + * nettle-meta-hashes.c (nettle_get_hashes): New function.

2017-10-23 Niels Möller nisse@lysator.liu.se

diff --git a/Makefile.in b/Makefile.in index 9201451..6a0c13e 100644 --- a/Makefile.in +++ b/Makefile.in @@ -110,6 +110,7 @@ nettle_SOURCES = aes-decrypt-internal.c aes-decrypt.c \ md2.c md2-meta.c md4.c md4-meta.c \ md5.c md5-compress.c md5-compat.c md5-meta.c \ memeql-sec.c memxor.c memxor3.c \ + nettle-lookup-hash.c \ nettle-meta-aeads.c nettle-meta-armors.c \ nettle-meta-ciphers.c nettle-meta-hashes.c \ pbkdf2.c pbkdf2-hmac-sha1.c pbkdf2-hmac-sha256.c \ diff --git a/ecc-192.c b/ecc-192.c index 5c52b04..57a7650 100644 --- a/ecc-192.c +++ b/ecc-192.c @@ -172,3 +172,7 @@ const struct ecc_curve nettle_secp_192r1 = ecc_table };

+const struct ecc_curve *nettle_get_secp_192r1(void) +{ + return &nettle_secp_192r1; +} diff --git a/ecc-224.c b/ecc-224.c index cdb4219..f665db6 100644 --- a/ecc-224.c +++ b/ecc-224.c @@ -123,3 +123,8 @@ const struct ecc_curve nettle_secp_224r1 = ecc_unit, ecc_table }; + +const struct ecc_curve *nettle_get_secp_224r1(void) +{ + return &nettle_secp_224r1; +} diff --git a/ecc-256.c b/ecc-256.c index e757985..bf5b6b8 100644 --- a/ecc-256.c +++ b/ecc-256.c @@ -300,3 +300,8 @@ const struct ecc_curve nettle_secp_256r1 = ecc_unit, ecc_table }; + +const struct ecc_curve *nettle_get_secp_256r1(void) +{ + return &nettle_secp_256r1; +} diff --git a/ecc-384.c b/ecc-384.c index a393c61..cba0178 100644 --- a/ecc-384.c +++ b/ecc-384.c @@ -208,3 +208,8 @@ const struct ecc_curve nettle_secp_384r1 = ecc_unit, ecc_table }; + +const struct ecc_curve *nettle_get_secp_384r1(void) +{ + return &nettle_secp_384r1; +} diff --git a/ecc-521.c b/ecc-521.c index 1a08f20..39a1871 100644 --- a/ecc-521.c +++ b/ecc-521.c @@ -137,3 +137,7 @@ const struct ecc_curve nettle_secp_521r1 = ecc_table };

+const struct ecc_curve *nettle_get_secp_521r1(void) +{ + return &nettle_secp_521r1; +} diff --git a/ecc-curve.h b/ecc-curve.h index 574c9f2..f3fd63e 100644 --- a/ecc-curve.h +++ b/ecc-curve.h @@ -41,11 +41,26 @@ extern "C" { /* The contents of this struct is internal. */ struct ecc_curve;

-extern const struct ecc_curve nettle_secp_192r1; -extern const struct ecc_curve nettle_secp_224r1; -extern const struct ecc_curve nettle_secp_256r1; -extern const struct ecc_curve nettle_secp_384r1; -extern const struct ecc_curve nettle_secp_521r1; +#ifdef __GNUC__ +#define NETTLE_PURE __attribute__((pure)) +#else +#define NETTLE_PURE +#endif + +const struct ecc_curve * NETTLE_PURE nettle_get_secp_192r1(void); +const struct ecc_curve * NETTLE_PURE nettle_get_secp_224r1(void); +const struct ecc_curve * NETTLE_PURE nettle_get_secp_256r1(void); +const struct ecc_curve * NETTLE_PURE nettle_get_secp_384r1(void); +const struct ecc_curve * NETTLE_PURE nettle_get_secp_521r1(void); + +#undef NETTLE_PURE + +/* For backwards compatibility */ +#define nettle_secp_192r1 (*nettle_get_secp_192r1()) +#define nettle_secp_224r1 (*nettle_get_secp_224r1()) +#define nettle_secp_256r1 (*nettle_get_secp_256r1()) +#define nettle_secp_384r1 (*nettle_get_secp_384r1()) +#define nettle_secp_521r1 (*nettle_get_secp_521r1())

#ifdef __cplusplus } diff --git a/ecc-internal.h b/ecc-internal.h index ce1e34f..4b45199 100644 --- a/ecc-internal.h +++ b/ecc-internal.h @@ -73,6 +73,21 @@ #define sec_modinv _nettle_sec_modinv #define curve25519_eh_to_x _nettle_curve25519_eh_to_x

+/* FIXME: Rename with leading underscore, but keep current name (and + size!) for now, for ABI compatibility with nettle-3.1, soname + libhogweed.so.4. */ +#undef nettle_secp_192r1 +#undef nettle_secp_224r1 +#undef nettle_secp_256r1 +#undef nettle_secp_384r1 +#undef nettle_secp_521r1 + +extern const struct ecc_curve nettle_secp_192r1; +extern const struct ecc_curve nettle_secp_224r1; +extern const struct ecc_curve nettle_secp_256r1; +extern const struct ecc_curve nettle_secp_384r1; +extern const struct ecc_curve nettle_secp_521r1; + /* Keep this structure internal for now. It's misnamed (since it's really implementing the equivalent twisted Edwards curve, with different coordinates). And we're not quite ready to provide diff --git a/nettle-lookup-hash.c b/nettle-lookup-hash.c new file mode 100644 index 0000000..98cd4ae --- /dev/null +++ b/nettle-lookup-hash.c @@ -0,0 +1,51 @@ +/* nettle-lookup-hash.c + + Copyright (C) 2016 Niels Möller. + + This file is part of GNU Nettle. + + GNU Nettle is free software: you can redistribute it and/or + modify it under the terms of either: + + * the GNU Lesser General Public License as published by the Free + Software Foundation; either version 3 of the License, or (at your + option) any later version. + + or + + * the GNU General Public License as published by the Free + Software Foundation; either version 2 of the License, or (at your + option) any later version. + + or both in parallel, as here. + + GNU Nettle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received copies of the GNU General Public License and + the GNU Lesser General Public License along with this program. If + not, see http://www.gnu.org/licenses/. +*/ + +#if HAVE_CONFIG_H +# include "config.h" +#endif + +#include <stddef.h> +#include <string.h> + +#include "nettle-meta.h" + +#undef nettle_hashes + +const struct nettle_hash * +nettle_lookup_hash (const char *name) +{ + unsigned i; + for (i = 0; nettle_hashes[i]; i++) + if (!strcmp (name, nettle_hashes[i]->name)) + return nettle_hashes[i]; + return NULL; +} diff --git a/nettle-meta-aeads.c b/nettle-meta-aeads.c index 8c05264..51866b6 100644 --- a/nettle-meta-aeads.c +++ b/nettle-meta-aeads.c @@ -37,6 +37,8 @@

#include "nettle-meta.h"

+#undef nettle_aeads + const struct nettle_aead * const nettle_aeads[] = { &nettle_gcm_aes128, &nettle_gcm_aes192, @@ -47,3 +49,9 @@ const struct nettle_aead * const nettle_aeads[] = { &nettle_chacha_poly1305, NULL }; + +const struct nettle_aead * const * +nettle_get_aeads (void) +{ + return nettle_aeads; +} diff --git a/nettle-meta-armors.c b/nettle-meta-armors.c index 9b6c341..b8a81bc 100644 --- a/nettle-meta-armors.c +++ b/nettle-meta-armors.c @@ -36,9 +36,17 @@ #include <stddef.h> #include "nettle-meta.h"

+#undef nettle_armors + const struct nettle_armor * const nettle_armors[] = { &nettle_base64, &nettle_base64url, &nettle_base16, NULL }; + +const struct nettle_armor * const * +nettle_get_armors (void) +{ + return nettle_armors; +} diff --git a/nettle-meta-ciphers.c b/nettle-meta-ciphers.c index 802fa14..ef905d8 100644 --- a/nettle-meta-ciphers.c +++ b/nettle-meta-ciphers.c @@ -36,6 +36,8 @@ #include <stddef.h> #include "nettle-meta.h"

+#undef nettle_ciphers + const struct nettle_cipher * const nettle_ciphers[] = { &nettle_aes128, &nettle_aes192, @@ -56,3 +58,9 @@ const struct nettle_cipher * const nettle_ciphers[] = { &nettle_arctwo_gutmann128, NULL }; + +const struct nettle_cipher * const * +nettle_get_ciphers (void) +{ + return nettle_ciphers; +} diff --git a/nettle-meta-hashes.c b/nettle-meta-hashes.c index 2220968..5df6f79 100644 --- a/nettle-meta-hashes.c +++ b/nettle-meta-hashes.c @@ -34,8 +34,11 @@ #endif

#include <stddef.h> + #include "nettle-meta.h"

+#undef nettle_hashes + const struct nettle_hash * const nettle_hashes[] = { &nettle_md2, &nettle_md4, @@ -52,3 +55,9 @@ const struct nettle_hash * const nettle_hashes[] = { &nettle_sha3_512, NULL }; + +const struct nettle_hash * const * +nettle_get_hashes (void) +{ + return nettle_hashes; +} diff --git a/nettle-meta.h b/nettle-meta.h index 14b5e48..0d16a2b 100644 --- a/nettle-meta.h +++ b/nettle-meta.h @@ -60,9 +60,20 @@ struct nettle_cipher nettle_cipher_func *decrypt; };

+/* FIXME: Rename with leading underscore, but keep current name (and + size!) for now, for ABI compatibility with nettle-3.1, soname + libnettle.so.6. */ /* null-terminated list of ciphers implemented by this version of nettle */ extern const struct nettle_cipher * const nettle_ciphers[];

+const struct nettle_cipher * const * +#ifdef __GNUC__ +__attribute__((pure)) +#endif +nettle_get_ciphers (void); + +#define nettle_ciphers (nettle_get_ciphers()) + extern const struct nettle_cipher nettle_aes128; extern const struct nettle_cipher nettle_aes192; extern const struct nettle_cipher nettle_aes256; @@ -114,9 +125,23 @@ struct nettle_hash (nettle_hash_digest_func *) name##_digest \ }

+/* FIXME: Rename with leading underscore, but keep current name (and + size!) for now, for ABI compatibility with nettle-3.1, soname + libnettle.so.6. */ /* null-terminated list of digests implemented by this version of nettle */ extern const struct nettle_hash * const nettle_hashes[];

+const struct nettle_hash * const * +#ifdef __GNUC__ +__attribute__((pure)) +#endif +nettle_get_hashes (void); + +#define nettle_hashes (nettle_get_hashes()) + +const struct nettle_hash * +nettle_lookup_hash (const char *name); + extern const struct nettle_hash nettle_md2; extern const struct nettle_hash nettle_md4; extern const struct nettle_hash nettle_md5; @@ -155,10 +180,21 @@ struct nettle_aead nettle_hash_digest_func *digest; };

+/* FIXME: Rename with leading underscore, but keep current name (and + size!) for now, for ABI compatibility with nettle-3.1, soname + libnettle.so.6. */ /* null-terminated list of aead constructions implemented by this version of nettle */ extern const struct nettle_aead * const nettle_aeads[];

+const struct nettle_aead * const * +#ifdef __GNUC__ +__attribute__((pure)) +#endif +nettle_get_aeads (void); + +#define nettle_aeads (nettle_get_aeads()) + extern const struct nettle_aead nettle_gcm_aes128; extern const struct nettle_aead nettle_gcm_aes192; extern const struct nettle_aead nettle_gcm_aes256; @@ -216,9 +252,20 @@ struct nettle_armor (nettle_armor_decode_final_func *) name##_decode_final, \ }

+/* FIXME: Rename with leading underscore, but keep current name (and + size!) for now, for ABI compatibility with nettle-3.1, soname + libnettle.so.6. */ /* null-terminated list of armor schemes implemented by this version of nettle */ extern const struct nettle_armor * const nettle_armors[];

+const struct nettle_armor * const * +#ifdef __GNUC__ +__attribute__((pure)) +#endif +nettle_get_armors (void); + +#define nettle_armors (nettle_get_armors()) + extern const struct nettle_armor nettle_base64; extern const struct nettle_armor nettle_base64url; extern const struct nettle_armor nettle_base16; diff --git a/testsuite/meta-hash-test.c b/testsuite/meta-hash-test.c index f7fa536..4754f66 100644 --- a/testsuite/meta-hash-test.c +++ b/testsuite/meta-hash-test.c @@ -23,21 +23,16 @@ const char* hashes[] = { void test_main(void) { - int i,j; + int i; int count = sizeof(hashes)/sizeof(*hashes); for (i = 0; i < count; i++) { - for (j = 0; NULL != nettle_hashes[j]; j++) { - if (0 == strcmp(hashes[i], nettle_hashes[j]->name)) - break; - } - ASSERT(NULL != nettle_hashes[j]); /* make sure we found a matching hash */ + /* make sure we found a matching hash */ + ASSERT(nettle_lookup_hash(hashes[i]) != NULL); } - j = 0; - while (NULL != nettle_hashes[j]) - j++; - ASSERT(j == count); /* we are not missing testing any hashes */ - for (j = 0; NULL != nettle_hashes[j]; j++) { - ASSERT(nettle_hashes[j]->digest_size <= NETTLE_MAX_HASH_DIGEST_SIZE); - ASSERT(nettle_hashes[j]->context_size <= NETTLE_MAX_HASH_CONTEXT_SIZE); + + for (i = 0; NULL != nettle_hashes[i]; i++) { + ASSERT(nettle_hashes[i]->digest_size <= NETTLE_MAX_HASH_DIGEST_SIZE); + ASSERT(nettle_hashes[i]->context_size <= NETTLE_MAX_HASH_CONTEXT_SIZE); } + ASSERT(i == count); /* we are not missing testing any hashes */ } diff --git a/tools/nettle-hash.c b/tools/nettle-hash.c index 488dff3..2419992 100644 --- a/tools/nettle-hash.c +++ b/tools/nettle-hash.c @@ -60,19 +60,6 @@ list_algorithms (void) alg->name, alg->digest_size, alg->block_size, alg->context_size); };

-static const struct nettle_hash * -find_algorithm (const char *name) -{ - const struct nettle_hash *alg; - unsigned i; - - for (i = 0; (alg = nettle_hashes[i]); i++) - if (!strcmp(name, alg->name)) - return alg; - - return NULL; -} - /* Also in examples/io.c */ static int hash_file(const struct nettle_hash *hash, void *ctx, FILE *f) @@ -211,7 +198,7 @@ main (int argc, char **argv) die("Algorithm argument (-a option) is mandatory.\n" "See nettle-hash --help for further information.\n");

- alg = find_algorithm (alg_name); + alg = nettle_lookup_hash (alg_name); if (!alg) die("Hash algorithm `%s' not supported or .\n" "Use nettle-hash --list to list available algorithms.\n",