On Tue, 26 Feb 2019, Colin Watson wrote:
On Wed, Feb 13, 2019 at 10:36:34AM +0200, Harry Sintonen wrote:The recent openssh upstream fix to "check in scp client that filenames sent during remote->local directory copies satisfy the wildcard specified by the user" (*) had an unfortunate side effect of breaking a legitimate use case of scp: deliberately copying the source directory permissions over the target directory. This is achieved by using syntax: "dir/.".Hi, Have you already reported this directly upstream (bugzilla.mindrot.org)?
No.
I'd expect that to be the best approach here, since the upstream master branch exhibits the same problem, and especially since the changes were in response to your security vulnerability report. We can of course cherry-pick regression fixes that have landed upstream.
Yeah likely would be the best course of action. If someone feels like forwarding this to them, please do so.
Note that it was actually https://anongit.mindrot.org/openssh.git/commit/?id=6010c0303a422a9c5fa8860c061bf7105eb7f8b2 that broke this.
Indeed, I now see that this patch was indeed different from the one I suggested initially. At the time I didn't check for "." myself either, so I also added check for it. However, I then discovered the issue we have now. So I adjusted my patches to cater this specific syntax, separating the check for the "." and adding some checks to allow it in certain situations ("dir/." etc). It's even commented in the patch, so it should be quite evident what the code is doing there and why.
How exactly this was missed in the official patches is a mystery to me.While incomplete, GNU extension dependant and clearly not as consise as the "official", my patch at least handles the "dir/." case properly:
https://sintonen.fi/advisories/scp-name-validator.patchPS. This is by no means a suggestion to replace the upstream patches with my mess. It merely tries to demonstrate how I handled this particular issue.
In part I feel responsible for this bug, but then on the other hand I did try to handle this correctly while upstream did not. I find the whole thing a bit depressing.