Stricter error handling in mcrypt extension

Yes, I'm speaking of the mcrypt_generic* functions and
mcrypt_generic_init() in particular.
However, without having looked at the code, I think the issue is with
mcrypt_enc_get_iv_size() - it returns wrong values and
mcrypt_generic_init() seems to be checking against them.

Speaking of bugs in MCrypt and IVs ... in ECB mode it complains if
you don't pass an IV, even though it is ignored afterwards.

You're probably referring to mcrypt_generic here, rather than
mcrypt_encrypt. I can bring that function in line with mcrypt_encrypt,
i.e. add the same error checks and make the IV only required if the
mode requires it.

However I'm not sure what kind of return value I should use with this
function. Currently it returns a long result, which is 0 on success
and a negative number on error. However mcrypt does not define error
codes for all possible error conditions, e.g. while it has a code for
invalid key sizes, it doesn't have a code for invalid IV sizes.

Personally I'd just switch it to true/false for success/error, as the
warnings already tell you what kind of error occurred. Would that be
okay with you, Derick?

No - as that is a BC break of a deliberate (though crappy ;-)) API. The
result values of mcrypt_generic_init() (which is I think what you're
refering too) are documented too:

    The function returns a negative value on error: -3 when the key
    length was incorrect, -4 when there was a memory allocation problem
    and any other return value is an unknown error. If an error occurs a
    warning will be displayed accordingly. `FALSE` is returned if
    incorrect parameters were passed.

cheers,
Derick

--
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine

Speaking of bugs in MCrypt and IVs ... in ECB mode it complains if
you don't pass an IV, even though it is ignored afterwards.

You're probably referring to mcrypt_generic here, rather than
mcrypt_encrypt. I can bring that function in line with mcrypt_encrypt,
i.e. add the same error checks and make the IV only required if the
mode requires it.

However I'm not sure what kind of return value I should use with this
function. Currently it returns a long result, which is 0 on success
and a negative number on error. However mcrypt does not define error
codes for all possible error conditions, e.g. while it has a code for
invalid key sizes, it doesn't have a code for invalid IV sizes.

Personally I'd just switch it to true/false for success/error, as the
warnings already tell you what kind of error occurred. Would that be
okay with you, Derick?

No - as that is a BC break of a deliberate (though crappy ;-)) API. The
result values of mcrypt_generic_init() (which is I think what you're
refering too) are documented too:

    The function returns a negative value on error: -3 when the key
    length was incorrect, -4 when there was a memory allocation problem
    and any other return value is an unknown error. If an error occurs

a
warning will be displayed accordingly. FALSE is returned if
incorrect parameters were passed.

cheers,
Derick

Yeah, it would be kinda ugly to have "falsy" be success in one version and
be failure in the next...

I applied the changes to mcrypt_encrypt/mcrypt_decrypt/mcrypt_{MODE} now
and left mcrypt_generic_init() alone. Most people should be using the
encrypt/decrypt functions anyway.

Nikita