Suggested changes to PurpleCipherOps and need for code review

[Ethan Blanton]

Tomasz Wasilczyk spake unto us the following wisdom:
> 2013/5/1 Ethan Blanton <elb at pidgin.im>:
> >> My suggestions:
> >> - merge set_key() with set_key_with_len() (using ssize_t len instead
> >> of size_t, if we don't want to provide length)
> >> - add get_digest_size(), if we don't want to guess buffer length for digest()
> >> - add (optional) length parameter for set_salt()
> >
> > All of this seems fine to me.  I'm not sure the lengths should be
> > optional, but I'd have to see how you intend to derive them to be
> > sure.  (Note that strlen() isn't really reasonable here.)
> 
> 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.

Yeah, constant key lengths are the norm for symmetric encryption
algorithms; however, if anything has a variable length, we ought to
use it everywhere.  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.

> >> - add output_size parameter to encrypt() and decrypt() to prevent
> >> buffer overflow (outlen is used to store actual data length after
> >> encryption/decryption)
> >
> > A common technique for this is to use outlen as an in/out parameter;
> > it contains the buffer size at call time, and the bytes written at
> > return time.  It may or may not be applicable here.
> 
> I was thinking of it, but I see two problems with it:
> - API changes (turning outlen from only-out to in-out parameter)
> without changing function definitions - its really easy to omit it
> when updating a plugin from 2.x.y to 3.0.0 API

Fair, and important.

> - it doesn't let specifying it by sizeof parameter, for example
> encrypt(..., buff, sizeof(buff), &outlen)

Also fair, but of limited concern to me.

Ethan