[sane-devel] 39ceeae6 breaks md5 auth

[James Ring]

Hi Olaf,

On Tue, Jan 2, 2018 at 11:15 PM, Olaf Meeuwissen
<paddy-hack at member.fsf.org> wrote:
> Hi James,
>
> Thanks for the report.
>
> James Ring writes:
>
>> Confirmed that with the offending patch, md5.c produces incorrect
>> digests for known input/output pairs. We should roll it back.
>>
>> Also I couldn't reproduce (with gcc 7.2.0) the compiler warning that
>> the original change was supposed to fix.
>
> Hmm, neither can I.  In neither the debian-8-mini nor debian-9-mini CI
> environments.  But you can still see it in the CI logs for the last
> pipeline that ran before the offending commit.
>
>   https://gitlab.com/sane-project/backends/pipelines/4150861
>
> Only the fedora-24-clang log doesn't have it.

Unrelated note: if I read those logs correctly, the CI doesn't run
`make check`, which maybe they should. It would also be nice to have a
test case that exercises authentication, though I suspect it is an
infrequently used feature. I tried adding one, but I couldn't figure
out how to update the makefiles to include my test.

>> On Tue, Jan 2, 2018 at 9:57 AM, James Ring <sjr at jdns.org> wrote:
>>> [...snip...]
>>>
>>> Reverting that commit restores the functionality. I haven't figured
>>> out what the problem is from a cursory inspection of the code, I'll
>>> continue staring at it.
>
> I was about to revert the commit but looking at it now I'm wondering
> what I was thinking when I committed that :-(  Changing the pointer
> type to something of a different size *obviously* screws up the array
> indexing!
>
> I've cooked up a fix for that (based on e895ee55).  Could you give the
> attached patch a try?

Yes, this works! I can't help wondering if it might be better to
introduce an external dependency on, e.g. libxcrypt or libssl to
implement md5? That way the sane project is not maintaining an md5
fork.

Thank you for your help!

James