bug#56322: Ruby packaging issues
2022/08/24 20:38, Maxime Devos: > We have a bunch of old rubies packaged, maybe it can be generated with > one of the old versions? Though possibly the old versions have the > same problem, I haven't checked. Older rubies need ruby to compile too, I checked. To totally getting rid of parse.c is not easy. > If not: fully properly generating it might not be possible, but > something in-between could be an option: > > 1. First, use the pre-generated parse.c. > 2. Once ruby is built, regenerate the parse.c, and verify that it is >the same as the old parse.c (ignoring the timestamp) > >> What's to gain by this? > > (1) I would assume it is much easier to hide malware in a generated > file like parse.c than in the real source code (*) (IIRC, the .c code > generated by bison is much longer than the .y). By generating the > parse.c, the potential issue is side-stepped; any security reviewers > wouldn't even have to look at parse.c because the pre-generated > parse.c isn't used, it's regenerated. By using one ruby to support compiling the others said security reviewer can focus on one particular parse.c. It's big but reviewing it seems doable but I am no security reviewer. > (2) Also: generators like Bison can have bugs, fixed in later > versions. Now imagine that Bison had, say, a buffer overflow bug, and > that distro's just used the pre-generated parse.c. Then once a fixed > version of Bison comes out, we would have to check every package to > see if it has a pre-generated parser. It would be much less stressful > to just always generate parsers from source, then once the version of > Bison in Guix is updated then all packages automatically get the > buffer overflow fix. > > I don't think my in-between proposal helps much with (1) in case of a > competent attacker (though it could stop some insufficiently > sophisticated attacks where the parse.c malware doesn't try to subvert > the later check), but it still helps with (2) -- it at least detects > if ruby used an old bison (and hence that a patch might be in order) The two phase build approach (first building with parse.c and then using that ruby as native-input for a package with parse.c removed) seems to work but with some notes. Rubies 2.7 and up work fine with bison current in guix (bison-3.7.6) but ruby-2.6 (and possibly down) don't because they trigger some incompatibility between bison-3.5.1 (stated as parse.c generator in ruby-2.6) and bison-3.7.6. I tried bison-3.0 from gnu/packages/bison for ruby-2.6 and it works but using that kinda defeats the ".. automatically get the buffer overflow fix" argument. I'd say, it doesn't really matter for ruby-2.6 and down since they are EOL anyway and should at some point be removed from guix. I'll post a patch after this message for feedback. In it a new package is introduced based on ruby-2.7 named baseruby which is compiled with the parse.c from the tarball, ruby-2.7 and up will delete parse.c before build and have extra native-inputs on baseruby and bison to support the magic. Cheers, Remco
bug#56322: Ruby packaging issues
On 24-08-2022 17:24, Remco van 't Veer wrote: * Ruby contains some things generated by bison or such. It seems the generated parse.c file (from parse.y) is included in the tarballs as a service to workaround a bootstrap problem; generating the parser requires ruby. See also: https://github.com/ruby/ruby/blob/master/common.mk#L910 See my other reply, and also: Even if generating the .c from the .y from the Ruby code would be ideal, at least generating the .c from the .y (and using the pre-generated the .y) is still an improvement, it would at least help with the 'buggy bison' scenario. Greetings, Maxime. OpenPGP_0x49E3EE22191725EE.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
bug#56322: Ruby packaging issues
On 24-08-2022 17:24, Remco van 't Veer wrote: [...] * Ruby bundles zlib. Can you point out where it is in the source tree? Looking at the sources I can only find a (very beefy) wrapper around zlib which seems to implement all kinds of zlib stuff but also depends on the zlib library. I dunno how to determine if this is bundling or not. https://github.com/ruby/ruby/blob/master/ext/zlib/zlib.c I probably confused the wrapper for a local copy of zlib, nevermind. There's a zlib-1.2.11-mswin.patch though, I wonder what's up with that. * Ruby contains some things generated by bison or such. It seems the generated parse.c file (from parse.y) is included in the tarballs as a service to workaround a bootstrap problem; generating the parser requires ruby. See also: https://github.com/ruby/ruby/blob/master/common.mk#L910 I don't know how to deal with this properly. The only thing I can think of is compiling in two phases: first with the supplied parse.c and after without. Or try it with mruby as a native-input but that seems to require ruby to compile too. We have a bunch of old rubies packaged, maybe it can be generated with one of the old versions? Though possibly the old versions have the same problem, I haven't checked. If not: fully properly generating it might not be possible, but something in-between could be an option: 1. First, use the pre-generated parse.c. 2. Once ruby is built, regenerate the parse.c, and verify that it is the same as the old parse.c (ignoring the timestamp) What's to gain by this? (1) I would assume it is much easier to hide malware in a generated file like parse.c than in the real source code (*) (IIRC, the .c code generated by bison is much longer than the .y). By generating the parse.c, the potential issue is side-stepped; any security reviewers wouldn't even have to look at parse.c because the pre-generated parse.c isn't used, it's regenerated. (2) Also: generators like Bison can have bugs, fixed in later versions. Now imagine that Bison had, say, a buffer overflow bug, and that distro's just used the pre-generated parse.c. Then once a fixed version of Bison comes out, we would have to check every package to see if it has a pre-generated parser. It would be much less stressful to just always generate parsers from source, then once the version of Bison in Guix is updated then all packages automatically get the buffer overflow fix. I don't think my in-between proposal helps much with (1) in case of a competent attacker (though it could stop some insufficiently sophisticated attacks where the parse.c malware doesn't try to subvert the later check), but it still helps with (2) -- it at least detects if ruby used an old bison (and hence that a patch might be in order) Greetings, Maxime. (*) Caveat: I don't have any statistics on this. OpenPGP_0x49E3EE22191725EE.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
bug#56322: Ruby packaging issues
Maxime Devos wrote on 30 Jun 13:15 +0200 > I noticed that: > > * Ruby has Autotools ./configure scripts that aren't regenerated. I'll make a patch for that. I also noticed unbundling of libffi is not applied to all versions, will fix that too. > * Ruby bundles zlib. Can you point out where it is in the source tree? Looking at the sources I can only find a (very beefy) wrapper around zlib which seems to implement all kinds of zlib stuff but also depends on the zlib library. I dunno how to determine if this is bundling or not. https://github.com/ruby/ruby/blob/master/ext/zlib/zlib.c > * Ruby contains some things generated by bison or such. It seems the generated parse.c file (from parse.y) is included in the tarballs as a service to workaround a bootstrap problem; generating the parser requires ruby. See also: https://github.com/ruby/ruby/blob/master/common.mk#L910 I don't know how to deal with this properly. The only thing I can think of is compiling in two phases: first with the supplied parse.c and after without. Or try it with mruby as a native-input but that seems to require ruby to compile too. What's to gain by this? Cheers, Remco
bug#56322: Ruby packaging issues
I noticed that: * Ruby has Autotools ./configure scripts that aren't regenerated. * Ruby bundles zlib. * Ruby contains some things generated by bison or such. Greetings, Maxime. signature.asc Description: This is a digitally signed message part
