Suggested changes to PurpleCipherOps and need for code review

[Tomasz Wasilczyk]

I did all changes I posted here, but I would like to do one more.

Currently, purple_cipher_context_reset is hardly usable, because it
resets both state (IV, digest context) and configuration (key, ops).
When using simple ciphers, like md5, it's not a problem, but for hmac
it is - we need to set key and hash type every time, when we want to
digest a value. I suggest doing a void, but significant change in API:

libpurple/cipher.h:
 /**
- * Resets a cipher context to it's default value
- * @note If you have set an IV you will have to set it after resetting
+ * Resets a cipher context to it's default value, but doesn't touch stateless
+ * configuration.
+ *
+ * That means, IV and digest context will be wiped out, but keys, ops or salt
+ * will remain untouched.
  *
  * @param context The context to reset
  * @param extra   Extra data for the specific cipher
  */
 void purple_cipher_context_reset(PurpleCipherContext *context, gpointer extra);

Is anyone against such change? I think this version will be more
useful and cleaner to use. I'm not completely sure about salt, but
it's still stateless part, so I would leave it.

Tomek

2013/5/1 Ethan Blanton <elb at pidgin.im>:
> Tomasz Wasilczyk spake unto us the following wisdom:
>> 2013/5/1 Ethan Blanton <elb at pidgin.im>:
>> > Tomasz Wasilczyk spake unto us the following wisdom:
>> >> I'm also not sure about it. There are ciphers with constant
>> >> key/salt/whatever lengths (I guess, that current API was designed for
>> >> them), but it would be a good idea to double-check it here.
>> >
>> > (...) Salts, in particular, can be of arbitrary length
>> > in either case.  Check and make sure that "salt" doesn't mean "IV" in
>> > this case, though, because IVs are of length predetermined by the
>> > algorithm.
>>
>> I was aware of salt used specifically in PBKDF2 algorithm - it may be
>> totally variable-length.
>
> Sure.
>
>> Anyway, I'm not sure, if length for IV should be provided or not. This
>> is constant length buffer, but may be still accidentally used with
>> buffer of wrong length (again, double checking).
>
> Agreed, providing length for everything is perfectly reasonable.
>
> Ethan
>