Re: [BUG] "svn propedit" loses changes in case of a network failure

[Yasuhito FUTATSUKI]

A thing I forgot to write

On 2022/09/19 15:09, Yasuhito FUTATSUKI wrote:
> On 2022/09/19 11:37, Vincent Lefevre wrote:
>> On 2022-09-19 11:02:05 +0900, Yasuhito FUTATSUKI wrote:
 
> I wrote the second paragraph just for to show where the behaivor
> comes from, i.e. where to fix, who want to fix it, and for there
> information to write a regression test. 

If you want to write a patch, the code in svn_cl__cleanup_log_msg()
in subversion/svn/util.c may be helpful, for handling error.

https://svn.apache.org/viewvc/subversion/trunk/subversion/svn/util.c?revision=1849190=markup#l256

Cheers,
-- 
Yasuhito FUTATSUKI 

Re: [BUG] "svn propedit" loses changes in case of a network failure

[Yasuhito FUTATSUKI]

On 2022/09/19 11:37, Vincent Lefevre wrote:
> On 2022-09-19 11:02:05 +0900, Yasuhito FUTATSUKI wrote:
>> I can't determine whether this behavior is a bug or not. 
>>
>> As svn_cl__propedit() calls svn_cmdline__edit_string_externally()[1]
>> for editing revprop value string with the parameter tempfail_reft is
>> NULL[2], the temporary file is removed within the function
>> svn_cmdline__edit_string_externally(), before trying to set the new
>> value.  So, it can be reproduced in any scheme, even "file" scheme,
>> by rejecting the change of revprop values in pre-revprop-change
>> hook.
> 
> Well, if the pre-revprop-change hook rejects the change, this may
> not be a bug, though I think that the new file should really be kept
> somewhere. AFAIK, the svn-commit.tmp file (log message) is kept for
> a rejected commit, so I don't see why this should be different.
> 
> But here, this is not a rejection due to a hook, but a failure due to
> some external cause, on which the user has no control (contrary to a
> hook).

Ah, second paragraph is not the reason why I can't see that it is
bug or not. It is just I don't see why the author wrote a such code.
I am still neutral.

I wrote the second paragraph just for to show where the behaivor
comes from, i.e. where to fix, who want to fix it, and for there
information to write a regression test. 

Cheers,
-- 
Yasuhito FUTATSUKI 

Re: [BUG] "svn propedit" loses changes in case of a network failure

[Vincent Lefevre]

On 2022-09-19 11:02:05 +0900, Yasuhito FUTATSUKI wrote:
> I can't determine whether this behavior is a bug or not. 
> 
> As svn_cl__propedit() calls svn_cmdline__edit_string_externally()[1]
> for editing revprop value string with the parameter tempfail_reft is
> NULL[2], the temporary file is removed within the function
> svn_cmdline__edit_string_externally(), before trying to set the new
> value.  So, it can be reproduced in any scheme, even "file" scheme,
> by rejecting the change of revprop values in pre-revprop-change
> hook.

Well, if the pre-revprop-change hook rejects the change, this may
not be a bug, though I think that the new file should really be kept
somewhere. AFAIK, the svn-commit.tmp file (log message) is kept for
a rejected commit, so I don't see why this should be different.

But here, this is not a rejection due to a hook, but a failure due to
some external cause, on which the user has no control (contrary to a
hook).

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: [BUG] "svn propedit" loses changes in case of a network failure

[Yasuhito FUTATSUKI]

Hello,

On 2022/09/19 9:57, Vincent Lefevre wrote:
> On 2022-09-18 01:59:35 +0200, Vincent Lefevre wrote:
>> On 2022-09-18 01:41:38 +0200, Vincent Lefevre wrote:
>>> With svn 1.14.2 under Debian/unstable, I wanted to edit a log message
>>> with
>>>
>>>   svn pe --revprop svn:log -r 151946
>>>
>>> (not just a minor change, I was replacing text by a much longer text),
>>> but got an immediate error from SSH:
>>
>> Note: after quitting the editor.
> 
> The issue can be reproduced by creating a wrapper script to ssh and
> using
> 
>   SVN_SSH=/path/to/this/script svn propedit ...
>
> Before quitting the editor, change this script to "exit 1" in order
> to simulate a ssh failure. The /tmp/svn-prop.tmp file the user has
> modified is no longer there, and there does not seem to be a backup
> anywhere else (the error message doesn't tell anyway).
>
> Note: I'm using the "svn+ssh" scheme. I don't know whether this bug
> also occurs with the https scheme (I cannot try).

I can't determine whether this behavior is a bug or not. 

As svn_cl__propedit() calls svn_cmdline__edit_string_externally()[1]
for editing revprop value string with the parameter tempfail_reft is
NULL[2], the temporary file is removed within the function
svn_cmdline__edit_string_externally(), before trying to set the new
value.  So, it can be reproduced in any scheme, even "file" scheme,
by rejecting the change of revprop values in pre-revprop-change
hook.

[1] subversion/include/private/svn_cmdline_private.h, line 1688
https://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_cmdline_private.h?annotate=1821224#l168
[2] subversion/svn/propedit-cmd.c,line 143
https://svn.apache.org/viewvc/subversion/trunk/subversion/svn/propedit-cmd.c?annotate=1885997#l143

Cheers,
-- 
Yasuhito FUTATSUKI 

Re: [BUG] "svn propedit" loses changes in case of a network failure

[Vincent Lefevre]

On 2022-09-18 01:59:35 +0200, Vincent Lefevre wrote:
> On 2022-09-18 01:41:38 +0200, Vincent Lefevre wrote:
> > With svn 1.14.2 under Debian/unstable, I wanted to edit a log message
> > with
> > 
> >   svn pe --revprop svn:log -r 151946
> > 
> > (not just a minor change, I was replacing text by a much longer text),
> > but got an immediate error from SSH:
> 
> Note: after quitting the editor.

The issue can be reproduced by creating a wrapper script to ssh and
using

  SVN_SSH=/path/to/this/script svn propedit ...

Before quitting the editor, change this script to "exit 1" in order
to simulate a ssh failure. The /tmp/svn-prop.tmp file the user has
modified is no longer there, and there does not seem to be a backup
anywhere else (the error message doesn't tell anyway).

Note: I'm using the "svn+ssh" scheme. I don't know whether this bug
also occurs with the https scheme (I cannot try).

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: [BUG] "svn propedit" loses changes in case of a network failure

[Vincent Lefevre]

On 2022-09-18 01:41:38 +0200, Vincent Lefevre wrote:
> With svn 1.14.2 under Debian/unstable, I wanted to edit a log message
> with
> 
>   svn pe --revprop svn:log -r 151946
> 
> (not just a minor change, I was replacing text by a much longer text),
> but got an immediate error from SSH:

Note: after quitting the editor.

FYI, this means that Subversion is vulnerable to a remote attack.
Here are the details. The logs of my server show:

Sep 18 01:24:09 joooj sshd[141287]: error: kex_exchange_identification: 
Connection closed by remote host
Sep 18 01:24:09 joooj sshd[141287]: Connection closed by 197.5.145.64 port 58377
Sep 18 01:24:10 joooj sshd[615]: error: beginning MaxStartups throttling
Sep 18 01:24:10 joooj sshd[615]: drop connection #10 from [197.5.145.64]:58387 
on [155.133.131.76]:22 past MaxStartups

This "beginning MaxStartups throttling" was due to 197.5.145.64 only,
but this means that all the other IP addresses that attempt to
connect are concerned. There is protection by fail2ban, but it works
by looking at the logs, meaning that it takes a few seconds to react:

2022-09-18 01:24:11,513 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,514 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,539 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,540 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,568 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,569 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,569 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,592 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,592 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,608 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,636 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,663 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,697 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,698 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,698 fail2ban.actions[603]: NOTICE  [sshd] Ban 
197.5.145.64
2022-09-18 01:24:11,742 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,963 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,966 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:11,966 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:11
2022-09-18 01:24:13,972 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,972 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,973 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,974 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,975 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,975 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,978 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,979 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,981 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,982 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,983 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,984 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,985 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,986 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,987 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13
2022-09-18 01:24:13,988 fail2ban.filter [603]: INFO[sshd] Found 
197.5.145.64 - 2022-09-18 01:24:13

[BUG] "svn propedit" loses changes in case of a network failure

[Vincent Lefevre]

With svn 1.14.2 under Debian/unstable, I wanted to edit a log message
with

  svn pe --revprop svn:log -r 151946

(not just a minor change, I was replacing text by a much longer text),
but got an immediate error from SSH:

kex_exchange_identification: read: Connection reset by peer
Connection reset by 155.133.131.76 port 22
svn: E170013: Unable to connect to a repository at URL 'svn+ssh://mysvn'
svn: E210002: To better debug SSH connection problems, remove the -q option 
from 'ssh' in the [tunnels] section of your Subversion configuration file.
svn: E210002: Network connection closed unexpectedly

Subversion apparently does not keep a copy of the text (contrary to
the case of a commit, which leaves a svn-commit.tmp file), so the
whole new text was lost!!!

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)