[
https://issues.apache.org/jira/browse/CAMEL-26066?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Federico Mariani updated CAMEL-26066:
-------------------------------------
Description:
The scp producer sends the name of the file it writes inside the scp protocol,
on a line that ends with a line feed: {{D0775 0 <dir>}} for each directory in
the name, then {{C0<chmod> <size> <name>}} for the file
({{ScpOperations.writeFile}}, main faa015319ee7, lines 458 and 488).
The name comes from the {{CamelFileName}} header or the {{fileName}} option.
The producer checks that it stays inside the starting directory, and the
{{chmod}} option must be three octal digits, but nothing checks the name for a
line feed or another control character, which file systems allow in a file
name. A line feed would end the protocol line early, and the remote scp would
read the rest of the name as the next line.
*Proposal*:
* refuse a name with a line feed, a carriage return or another control
character with a {{GenericFileOperationFailedException}}, before anything is
sent;
* test it against the embedded SSH server of the camel-jsch tests;
* mention it in the scp docs.
_Claude Code on behalf of davsclaus_
----
*Update:* the fix ([PR #29285|https://github.com/apache/camel/pull/29285])
refuses only a line feed and a NUL character, with a
{{GenericFileOperationFailedException}} before scp is started on the remote
host. The check covers the directory names as well as the file name.
* *Line feed:* it ends the protocol line early, so the remote scp writes the
file under another name, or leaves a stray directory when the line feed is in a
directory name.
* *NUL:* it is never part of a valid name. Apache SSHD refuses it after scp has
started. The OpenSSH scp sink before 10.6 reads the line as a C string and cuts
the name at the NUL; OpenSSH 10.6p1 refuses it as well (openssh-portable
[e27b6cc2|https://github.com/openssh/openssh-portable/commit/e27b6cc2e6e8ba03808f7078740e5ba800515d4d],
"Disallow nul byte in received scp -O filename").
A carriage return, a tab and the other control characters are still accepted.
The scp sinks of Apache SSHD and OpenSSH end a protocol line on the line feed
only, so such a name is written as given, and refusing it would change the
behaviour without fixing a bug.
_Claude Code on behalf of Federico Mariani_
h3. Update: the scp producer could report success before the file was written
While this issue was reopened for a test that failed on the Jenkins builds, a
second problem in the scp producer was fixed under it
(https://github.com/apache/camel/pull/29333, backported to camel-4.22.x by
https://github.com/apache/camel/pull/29334 and to camel-4.18.x by
https://github.com/apache/camel/pull/29335).
The remote scp sends a first acknowledgement as soon as it starts, and the
producer took the channel streams only after connecting, so JSch could drop
that acknowledgement or not, depending on timing. When it was kept, every later
acknowledgement was read one step behind. The exchange then completed before
the server had written the file, and an error that the server reported for the
file, for example when it could not write it, was not seen, so the exchange
succeeded.
The producer now takes the streams before it connects and reads that first
acknowledgement, so an exchange completes once the server has written the file,
and a server error for the file fails the exchange.
_Claude Code on behalf of Federico Mariani_
was:
The scp producer sends the name of the file it writes inside the scp protocol,
on a line that ends with a line feed: {{D0775 0 <dir>}} for each directory in
the name, then {{C0<chmod> <size> <name>}} for the file
({{ScpOperations.writeFile}}, main faa015319ee7, lines 458 and 488).
The name comes from the {{CamelFileName}} header or the {{fileName}} option.
The producer checks that it stays inside the starting directory, and the
{{chmod}} option must be three octal digits, but nothing checks the name for a
line feed or another control character, which file systems allow in a file
name. A line feed would end the protocol line early, and the remote scp would
read the rest of the name as the next line.
*Proposal*:
* refuse a name with a line feed, a carriage return or another control
character with a {{GenericFileOperationFailedException}}, before anything is
sent;
* test it against the embedded SSH server of the camel-jsch tests;
* mention it in the scp docs.
_Claude Code on behalf of davsclaus_
----
*Update:* the fix ([PR #29285|https://github.com/apache/camel/pull/29285])
refuses only a line feed and a NUL character, with a
{{GenericFileOperationFailedException}} before scp is started on the remote
host. The check covers the directory names as well as the file name.
* *Line feed:* it ends the protocol line early, so the remote scp writes the
file under another name, or leaves a stray directory when the line feed is in a
directory name.
* *NUL:* it is never part of a valid name. Apache SSHD refuses it after scp has
started. The OpenSSH scp sink before 10.6 reads the line as a C string and cuts
the name at the NUL; OpenSSH 10.6p1 refuses it as well (openssh-portable
[e27b6cc2|https://github.com/openssh/openssh-portable/commit/e27b6cc2e6e8ba03808f7078740e5ba800515d4d],
"Disallow nul byte in received scp -O filename").
A carriage return, a tab and the other control characters are still accepted.
The scp sinks of Apache SSHD and OpenSSH end a protocol line on the line feed
only, so such a name is written as given, and refusing it would change the
behaviour without fixing a bug.
_Claude Code on behalf of Federico Mariani_
> camel-jsch - the scp producer should refuse a file name with a line feed or a
> NUL character
> -------------------------------------------------------------------------------------------
>
> Key: CAMEL-26066
> URL: https://issues.apache.org/jira/browse/CAMEL-26066
> Project: Camel
> Issue Type: Improvement
> Components: camel-jsch
> Reporter: Claus Ibsen
> Assignee: Federico Mariani
> Priority: Major
> Fix For: 4.18.6, 4.22.3, 4.24.0
>
>
> The scp producer sends the name of the file it writes inside the scp
> protocol, on a line that ends with a line feed: {{D0775 0 <dir>}} for each
> directory in the name, then {{C0<chmod> <size> <name>}} for the file
> ({{ScpOperations.writeFile}}, main faa015319ee7, lines 458 and 488).
> The name comes from the {{CamelFileName}} header or the {{fileName}} option.
> The producer checks that it stays inside the starting directory, and the
> {{chmod}} option must be three octal digits, but nothing checks the name for
> a line feed or another control character, which file systems allow in a file
> name. A line feed would end the protocol line early, and the remote scp would
> read the rest of the name as the next line.
> *Proposal*:
> * refuse a name with a line feed, a carriage return or another control
> character with a {{GenericFileOperationFailedException}}, before anything is
> sent;
> * test it against the embedded SSH server of the camel-jsch tests;
> * mention it in the scp docs.
> _Claude Code on behalf of davsclaus_
> ----
> *Update:* the fix ([PR #29285|https://github.com/apache/camel/pull/29285])
> refuses only a line feed and a NUL character, with a
> {{GenericFileOperationFailedException}} before scp is started on the remote
> host. The check covers the directory names as well as the file name.
> * *Line feed:* it ends the protocol line early, so the remote scp writes the
> file under another name, or leaves a stray directory when the line feed is in
> a directory name.
> * *NUL:* it is never part of a valid name. Apache SSHD refuses it after scp
> has started. The OpenSSH scp sink before 10.6 reads the line as a C string
> and cuts the name at the NUL; OpenSSH 10.6p1 refuses it as well
> (openssh-portable
> [e27b6cc2|https://github.com/openssh/openssh-portable/commit/e27b6cc2e6e8ba03808f7078740e5ba800515d4d],
> "Disallow nul byte in received scp -O filename").
> A carriage return, a tab and the other control characters are still accepted.
> The scp sinks of Apache SSHD and OpenSSH end a protocol line on the line feed
> only, so such a name is written as given, and refusing it would change the
> behaviour without fixing a bug.
> _Claude Code on behalf of Federico Mariani_
> h3. Update: the scp producer could report success before the file was written
> While this issue was reopened for a test that failed on the Jenkins builds, a
> second problem in the scp producer was fixed under it
> (https://github.com/apache/camel/pull/29333, backported to camel-4.22.x by
> https://github.com/apache/camel/pull/29334 and to camel-4.18.x by
> https://github.com/apache/camel/pull/29335).
> The remote scp sends a first acknowledgement as soon as it starts, and the
> producer took the channel streams only after connecting, so JSch could drop
> that acknowledgement or not, depending on timing. When it was kept, every
> later acknowledgement was read one step behind. The exchange then completed
> before the server had written the file, and an error that the server reported
> for the file, for example when it could not write it, was not seen, so the
> exchange succeeded.
> The producer now takes the streams before it connects and reads that first
> acknowledgement, so an exchange completes once the server has written the
> file, and a server error for the file fails the exchange.
> _Claude Code on behalf of Federico Mariani_
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
