Bug #46339 for Module-Signature: \r handled badly by Module::Signature

Mon May 25 03:39:39 2009 zefram [...] fysh.org - Ticket created

Subject:	\r handled badly by Module::Signature
Date:	Mon, 25 May 2009 08:39:10 +0100
To:	bug-Module-Signature [...] rt.cpan.org
From:	Zefram <zefram [...] fysh.org>

Module::Signature attempts to canonicalise line endings in text files in order to get a representation-independent SHA1 value for the SIGNATURE file. There are a couple of problems with this. Firstly, it's giving different results on different perl versions. It appears, from my tests, that the ":crlf" layer doesn't do anything on 5.6, whereas on 5.8+ it turns "\r\n" into "\n". So a file that uses CRs gets a different SHA1 value depending on the perl version, causing false signature verification errors. I've run into this with two separate modules that have "\r\n" line endings in some of the files as distributed. I'm working around this by avoiding using CRs in distribution packages, but M::S really needs to use a different strategy on 5.6 to validate the existing signatures. Second, more fundamentally, the canonicalisation is a stupid thing to do. The difference between "\r\n" and "\n" can be significant. One of the places I've had trouble is a file of test data in XML::Easy, which needs to test the parser against the three major types of line ending. The type of ending to each line is very important in that file, and the SHA1 value in the SIGNATURE file describes a broken version of the file. More trivially, the text-mode canonicalisation will screw up on binaries. (The XML test data file, while notionally textual, is probably best treated as a binary, in fact.) By SHA1ing something other than the actual distributed file, you've opened up room for security holes. -zefram

Mon Aug 16 17:15:33 2010 ANDK [...] cpan.org - Correspondence added

I just discovered this bug and this ticket (in that order). Would it probably be feasible to use binary handles on signing and a three-pass laxness on verification? If verification does not succeed on first (binary) pass, couldn't we add a second pass with :crlf and a third one with :cr (or equivalent)? That way we could gain backwards compatibility and allow for stupid user mistakes like signing on one operating system but zipping on a different one. IMHO, this would not reduce the security very much but work around the severe brokenness that we are facing today. WDYT?

Mon Aug 16 17:15:40 2010 The RT System itself - Status changed from 'new' to 'open'

Mon Aug 16 18:22:22 2010 rafl [...] debian.org - Correspondence added

Subject:	Re: [rt.cpan.org #46339] \r handled badly by Module::Signature
Date:	Tue, 17 Aug 2010 00:21:25 +0200
To:	bug-Module-Signature [...] rt.cpan.org
From:	Florian Ragwitz <rafl [...] debian.org>

"Andreas Koenig via RT" <bug-Module-Signature@rt.cpan.org> writes: Show quoted text

> Queue: Module-Signature > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=46339 > > > I just discovered this bug and this ticket (in that order).

Sorry :-( Show quoted text

> Would it probably be feasible to use binary handles on signing and a > three-pass laxness on verification? If verification does not succeed on > first (binary) pass, couldn't we add a second pass with :crlf and a > third one with :cr (or equivalent)? That way we could gain backwards > compatibility and allow for stupid user mistakes like signing on one > operating system but zipping on a different one. > > IMHO, this would not reduce the security very much but work around the > severe brokenness that we are facing today.

I'm not sure. For cryptographic things I tend to try to err on the safe side. But then again, I've never experienced the problems this ticket describes myself, nor am I very much into the details of Module-Signature, having done only the last couple of releases. Also, I can't think of a better approach to solve this. Lacking that, I'm entirely fine with your suggestion. However, it'll have to be "patches welcome" for the moment, due to a massive shortage of round tuits.

Download (untitled)
application/pgp-signature 197b

Message body not shown because it is not plain text.

Tue Aug 17 06:38:08 2010 zefram [...] fysh.org - Correspondence added

Subject:	Re: [rt.cpan.org #46339] \r handled badly by Module::Signature
Date:	Tue, 17 Aug 2010 11:37:50 +0100
To:	Andreas Koenig via RT <bug-Module-Signature [...] rt.cpan.org>
From:	Zefram <zefram [...] fysh.org>

Andreas Koenig via RT wrote: Show quoted text

>Would it probably be feasible to use binary handles on signing and a >three-pass laxness on verification?

In the short term we certainly need some kind of laxness, but it's not a good idea as a permanent strategy. I'd like us to ultimately have binary signing with strict checking. We should have some strategy to get there. Unfortunately the current verification protocol is not amenable to extensions. -zefram

Sun Sep 19 16:32:24 2010 ANDK [...] cpan.org - Correspondence added

RT-Send-CC:

zefram [...] cpan.org, rafl [...] debian.org

I've given it a shot: http://github.com/andk/module-signature There's a new test 3-verify.t that tests four different signed directories that I think should be supported. LF files on LF systems and CRLF files on CRLF systems should of course be supported because we want binary unchanged files to be supported. But we also need to support LF files that somebody has changed to be CRLF and the other way round bacause it has already bartially been supported with the "crlf filter code. The current code base supports all four cases on verification. Signing is binary only anyway. Please lend me your critical eyes and testing competence. My favorite real world test is SCHUMACK/GDS2-3.00.tar.gz under perl 5.6.2. It's one of the distros that (1) contain some CRLF files, were (2) signed with 5.8.x. When verifying with 5.8.x and above you get OK, but with 5.6 you get a bad signature detected error because 5.6.2 for some reason ignores the :CRLF filter. With my codebase this bug seems to be resolved. Enjoy,

Fri Apr 15 16:21:08 2011 zefram [...] fysh.org - Correspondence added

Subject:	Re: [rt.cpan.org #46339] \r handled badly by Module::Signature
Date:	Fri, 15 Apr 2011 21:20:47 +0100
To:	Andreas Koenig via RT <bug-Module-Signature [...] rt.cpan.org>
From:	Zefram <zefram [...] fysh.org>

Andreas Koenig via RT wrote: Show quoted text

>I've given it a shot: > >http://github.com/andk/module-signature

Sorry about the delay in getting to this. The code change looks fine to me, as an implementation of the laxness that is necessary for the time being. I reiterate that I'd like to see a move to signing the real file content, with no munging, but that's for the future. For now, your patch resolves the false-mismatch problem. -zefram

Wed Apr 20 16:27:35 2011 CHORNY [...] cpan.org - Correspondence added

I guess it was this patch that lead to problems on Windows: http://matrix.cpantesters.org/?dist=Module-Signature+0.67 And if _tests_ will be fixed, this may lead to problems in modules that use Module::Signature in tests that are run on Windows. -- Alexandr Ciornii, http://chorny.net

Wed Apr 20 16:28:30 2011 CHORNY [...] cpan.org - Cc CHORNY added

Thu May 12 22:09:46 2011 ANDK [...] cpan.org - Correspondence added

I have prepared a 0.67_01: https://github.com/andk/module-signature More eyeballs welcome