Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
On Tue, May 09, 2017 at 06:48:03PM +0200, Johan Corveleyn wrote: > If needed, admins > can (re-)enable rep-sharing for an existing repository (as long as a > collision hasn't been committed yet), right? Sure. However, any content committed while rep-sharing was disabled will not be considered during collision detection.
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
On Tue, May 09, 2017 at 12:27:40PM -0400, Mark Phippard wrote: > From the best I can tell we have no plan on how or when we could support > this in the working copy. I have also seen a lot of people express > interest in the hook scripts to block the sha1 collisions and not any real > conversation about wanting to fully support storing collisions. With that > in mind, why not just simplify this and say we have no plans to provide > support for supporting two files with the same sha1 and put the code in > place in 1.9.x to block this from happening. This removes the need for > people to add hooks and it solves the problem. Indeed. I have committed this change (with an updated test) in r1794611.
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
On Tue, May 9, 2017 at 6:27 PM, Mark Phippard wrote: > On Tue, May 9, 2017 at 12:08 PM, Stefan Sperling wrote: >> >> On Tue, May 09, 2017 at 03:44:22PM +, Daniel Shahaf wrote: >> > Stefan Sperling wrote on Tue, May 09, 2017 at 15:25:23 +0200: >> > > This could be further extended by the config knob to give users a >> > > choice. >> > > I don't see a good way of adding such a knob in a patch release, >> > > though. >> > >> > Just give the knob a name that indicates it's not forward compatible? >> > >> > For illustration, if the knob in 1.10 will be called "foo", then the >> > knob in 1.9.6 could be named "SVN_NFC_foo", where the prefix stands for >> > "svn not forward compatible". >> >> For a patch release I would generally prefer a simple solution that >> does not add knobs. A fix that people can install and forget about >> is going to be appreciated the most after all the hype and worrying >> this problem has caused. >> >> And I wonder who would really want to tweak such a knob in the first >> place. >> People who really wish to store SHA1 collisions in their FSFS repository >> could just disable rep-sharing, couldn't they? > > > > From the best I can tell we have no plan on how or when we could support > this in the working copy. I have also seen a lot of people express interest > in the hook scripts to block the sha1 collisions and not any real > conversation about wanting to fully support storing collisions. With that > in mind, why not just simplify this and say we have no plans to provide > support for supporting two files with the same sha1 and put the code in > place in 1.9.x to block this from happening. This removes the need for > people to add hooks and it solves the problem. > > If and when someone can state enough of a need for storing files with the > same sha1 let them step forward with a proposal and code for how to do this. > Until then it seems like just flat out blocking this from happening is > better then a half hearted attempt to support it. > > This approach does not need any knobs or hooks. Yeah, I was pushing on IRC for making it configurable (with some hope that a future client release might support it, and then a 1.10 repository could be "opened up" if the svn admin wanted to allow collisions). But I'm reconsidering. I guess you guys are right ... nobody is really asking today for full support of sha1 collisions. So better to keep it simple now, and flat out block them (and leave "full sha1 collision support" for some distant future when someone really wants / needs it. And like Stefan said: one can always disable rep-sharing. Speaking of which: we must make it really clear in docs that this collision rejection will only work with if rep-sharing is on. If needed, admins can (re-)enable rep-sharing for an existing repository (as long as a collision hasn't been committed yet), right? -- Johan
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
On Tue, May 9, 2017 at 12:08 PM, Stefan Sperling wrote: > On Tue, May 09, 2017 at 03:44:22PM +, Daniel Shahaf wrote: > > Stefan Sperling wrote on Tue, May 09, 2017 at 15:25:23 +0200: > > > This could be further extended by the config knob to give users a > choice. > > > I don't see a good way of adding such a knob in a patch release, > though. > > > > Just give the knob a name that indicates it's not forward compatible? > > > > For illustration, if the knob in 1.10 will be called "foo", then the > > knob in 1.9.6 could be named "SVN_NFC_foo", where the prefix stands for > > "svn not forward compatible". > > For a patch release I would generally prefer a simple solution that > does not add knobs. A fix that people can install and forget about > is going to be appreciated the most after all the hype and worrying > this problem has caused. > > And I wonder who would really want to tweak such a knob in the first place. > People who really wish to store SHA1 collisions in their FSFS repository > could just disable rep-sharing, couldn't they? > >From the best I can tell we have no plan on how or when we could support this in the working copy. I have also seen a lot of people express interest in the hook scripts to block the sha1 collisions and not any real conversation about wanting to fully support storing collisions. With that in mind, why not just simplify this and say we have no plans to provide support for supporting two files with the same sha1 and put the code in place in 1.9.x to block this from happening. This removes the need for people to add hooks and it solves the problem. If and when someone can state enough of a need for storing files with the same sha1 let them step forward with a proposal and code for how to do this. Until then it seems like just flat out blocking this from happening is better then a half hearted attempt to support it. This approach does not need any knobs or hooks. -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
On Tue, May 09, 2017 at 03:44:22PM +, Daniel Shahaf wrote: > Stefan Sperling wrote on Tue, May 09, 2017 at 15:25:23 +0200: > > This could be further extended by the config knob to give users a choice. > > I don't see a good way of adding such a knob in a patch release, though. > > Just give the knob a name that indicates it's not forward compatible? > > For illustration, if the knob in 1.10 will be called "foo", then the > knob in 1.9.6 could be named "SVN_NFC_foo", where the prefix stands for > "svn not forward compatible". For a patch release I would generally prefer a simple solution that does not add knobs. A fix that people can install and forget about is going to be appreciated the most after all the hype and worrying this problem has caused. And I wonder who would really want to tweak such a knob in the first place. People who really wish to store SHA1 collisions in their FSFS repository could just disable rep-sharing, couldn't they?
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
Stefan Sperling wrote on Tue, May 09, 2017 at 15:25:23 +0200: > This could be further extended by the config knob to give users a choice. > I don't see a good way of adding such a knob in a patch release, though. Just give the knob a name that indicates it's not forward compatible? For illustration, if the knob in 1.10 will be called "foo", then the knob in 1.9.6 could be named "SVN_NFC_foo", where the prefix stands for "svn not forward compatible". > Perhaps that could be done for 1.10.
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
+1 on rejection. On Tue, May 9, 2017 at 3:37 PM, Mark Phippard wrote: > On Tue, May 9, 2017 at 9:25 AM, Stefan Sperling wrote: >> >> On IRC, Branko and Johan raised concerns about the proposed backport. >> >> The proposed backport allows files with SHA1 collisions into the >> repository >> and avoids de-duplication of such content by the rep-cache. It fixes the >> integrity problem with the rep-cache but other problems remain. > > > This approach makes a lot of sense to me. > > > > -- > Thanks > > Mark Phippard > http://markphip.blogspot.com/ -- Jacek Materna CTO Assembla 210-410-7661
Re: [PATCH] reject SHA1 collisions (was: Re: Progress on SHA-1 fixes in patch releases?)
On Tue, May 9, 2017 at 9:25 AM, Stefan Sperling wrote: > On IRC, Branko and Johan raised concerns about the proposed backport. > > The proposed backport allows files with SHA1 collisions into the repository > and avoids de-duplication of such content by the rep-cache. It fixes the > integrity problem with the rep-cache but other problems remain. > This approach makes a lot of sense to me. -- Thanks Mark Phippard http://markphip.blogspot.com/
