Daniel Kahn Gillmor <[email protected]> writes:

> this is quite a bit of code duplication across bindings.

Could you be a bit more concrete? Which variant of wrappers and weak-key
interface are you thinking about?

Before getting into specifics, I'd like to point out that the structs
declared in nettle-meta.h are not intended as a fully general algorithm
framework (to do that, one could, e.g., implement an interface on the
same level of abstraction as libgcrypt's, on top of nettle).

Besides missing des and blowfish, it doesn't provide a nettle_cipher
struct for every possible key size for all algorithms, and there's also
no mechanism to query possible key sizes. To me, the lack of a struct
nettle_cipher for des and blowfish-128 is comparable to the lack of
struct nettle_cipher for cast, arcfour and blowfish with 80 bit key. And
to support them all in a reasonable way would require something quite
different from the current struct nettle_cipher.

nettle-meta.h is intended to provide the simplest algorithm abstraction
possible, to cover the simple cases. And also as inspiration for
extended frameworks, either more general or more application specific.

This works better for hash algorithm (more regular properties than for
the ciphers), and in the Pike bindings, I use nettle_hash as is, but I
don't use nettle_cipher.

> Here's a proposal for (2) which i'll name "2a";  I believe it does
> involve an ABI+API bump to libnettle, but should allow for a reduction
> in the amount of code for all bindings (which in turn might make the
> creation of future bindings more likely, thereby getting the nettle
> goodness out to more people).  I know i'd be more likely to maintain
> additional bindings if they are smaller/simpler.
>
> redefine nettle_set_key_func to return an int instead of a void:
>
> typedef int nettle_set_key_func(void *ctx,
>                               unsigned length,
>                               const uint8_t *key);
>
> For the ciphers which have no weak keys, create wrapper functions around
> their set_key functions which always return 1, and use those wrapper
> functions to populate the standard nettle_cipher objects.
>
> Add a wrapper function around des_set_key and des3_set_key that includes
> a key length argument; add corresponding nettle_cipher objects for des
> and des3.

I could consider this, but I'm not convinced that it really solves an
important problem. To me, a language-specific wrapper like, e.g.,

  void
  pike_blowfish_set_key(void *ctx,
                        ptrdiff_t length, const char *key,
                        int force)
  {
    if (length < BLOWFISH_MIN_KEY_SIZE || length > BLOWFISH_MAX_KEY_SIZE)
      Pike_error("BLOWFISH_Info: Bad keysize for BLOWFISH.\n");
    if (!blowfish_set_key(ctx, length, (const uint8_t *)key) && !force)
      Pike_error("BLOWFISH_Info: Key is weak.\n");
  }

seems more useful than a language agnostic wrapper

  int 
  aes_set_encrypt_key_wrapper (struct aes_ctx *ctx,
                               unsigned length, const uint8_t *key)
  {
    aes_set_encrypt_key (ctx, length, key);
    return 1;
  }

which lacks adequate error handling for bad key sizes. And if we extend
nettle_cipher to include a description of valid key sizes, and/or
introduce additional error codes for the set_key functions to signal
different types of bad keys, then we get quite far from the current
minimalistic flavor of nettle.

> And here is "2b", a more involved proposal for (2) -- it's a bigger
> ABI+API change, but the exposed API becomes more normalized:

I'm not going to do this. The low level cipher interface is not intended
to normalize away important differences between ciphers.

In particular, 

* I don't like dummy return values (which are either unnecessarily
  checked, making code ugly, or get people into the habit if ignoring
  return values). "Can't fail" (except by abort()) is a very simplifying
  property of a function, and then it shouldn't return an error code.

* I also don't like dummy function arguments, used like

  int des_set_key (struct des_ctx *ctx, 
                   unsigned length, const uint8_t *key)
  {
    assert (length == DES_KEY_SIZE);
    ...
  }

  (except in an optional wrapper function, where the above would be the
  right thing).

Remember that the C interface is intended to be nice also for
applications that use just one or two algorithms. These applications
should not suffer from cruft intended to unify the interface with some
other algorithm which the applications couldn't care less about. An
important case would be applications that use only the newer algorithms
like aes or camellia. They shouldn't have to bother about a return value
introduced just because it's needed for des.

BTW: There's another easy alternative which we could call (3): Keep
nettle_set_key_func as is. Introduce wrappers for des and blowfish which
just ignore weak keys, and provide nettle_cipher structs using these
wrappers. Then you get des and blowfish sans weak key detection, using
the same interface as the other ciphers. I think this would fit
reasonably with the nettle design principles. Question is: Would anybody
find it useful? For a general language binding, I would expect that one
would want to have the possibility to detect weak keys.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Reply via email to