Thomas Wolf [Fri, 5 Oct 2018 19:35:16 +0000 (21:35 +0200)]
Apache MINA sshd client: properly handle HostKeyAlgorithms config
By default sshd will use its default built-in list, which matches
the one of openssh (as far as the algorithms exist in sshd at all).
But it doesn't handle HostKeyAlgorithms from the ssh config at all.
Implement this as in openssh, including the '+' and '-' modifiers
and reordering the default if there are known host keys for a
server already.
Add tests for the reordering.
Also use a more robust reader for the known hosts file. The default
aborts on the first error.
Bug: 520927
Change-Id: Ib1684440bfe2e96140536aa1a93c4bd4a0d35916 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Wed, 3 Oct 2018 06:27:40 +0000 (08:27 +0200)]
Apache MINA sshd client: respect NumberOfPasswordPrompts
Set the internal property on the session as defined in the ssh config.
Note that NumberOfPasswordPrompts in openssh applies independently to
both user logins in keyboard-interactive authentication _and_ to
passphrases for identity files (encrypted keys). Apache MINA sshd uses
the setting only for keyboard-interactive authentication, but not for
identity file passphrase prompts. For identity files, it asks exactly
once. This has been reported as issue SSHD-850 upstream.[1]
Thomas Wolf [Tue, 2 Oct 2018 20:39:40 +0000 (22:39 +0200)]
Apache MINA sshd client: add gssapi-with-mic authentication
sshd does support gssapi-with-mic on the server side, but has no
built-in client-side support for this authentication mechanism.
Add our own implementation for it, following RFC 4462.[1] To avoid
needlessly re-trying mechanisms that aren't even configured on the
client, we disable mechanisms that fail on the very first attempt
to use them.
Since we have no real Kerberos5 test setup, this cannot be fully
tested in CI. The disabling of the authentication mechanism and
that it is skipped when not successful _is_ tested.
[1] https://www.ietf.org/rfc/rfc4462.txt
Bug: 520927
Change-Id: I5d0cdb14103588a57c52f927df541b589ab88d88 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Mon, 1 Oct 2018 21:27:10 +0000 (23:27 +0200)]
Add ssh tests for PreferredAuthentications
Tests that it works with unknown authentications in the list, and
fails if there are no common authentications between server and
client. The latter also tests that the ssh config setting is taken
into account at all.
And promptly the JGit sshd client didn't. Add a fix for this. It's
a tiny bit hacky: Apache MINA looks up a custom property set on a
hierarchy of "PropertyResolver"s starting with the session. On the
session itself this property can never be set since it's read
already in the session constructor before anyone had any chance
to set it. The next element in the resolver hierarchy is the sshd
SshClient, and so we set that property there. Since we use one
SshClient and one ClientSession per JGit SshdSession, this is OK.
Bug: 520927
Change-Id: I62446fc1fffde125a8965c030240f0918ae234b7 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 23 Sep 2018 14:45:45 +0000 (16:45 +0200)]
Apache MINA sshd client
Add a new ssh client implementation based on Apach MINA sshd 2.0.0.
This implementation uses JGit's own config file parser and host entry
resolver. Code inspection of the Apache MINA implementation revealed
a few bugs or idiosyncrasies that immediately would re-introduce bugs
already fixed in the past in JGit.
Apache MINA sshd is not without quirks either, and I had to configure
and override more than I had expected. But at least it was all doable
in clean ways.
Apache MINA boasts support for Bouncy Castle, so in theory this should
open the way to using more ssh key algorithms, such as ed25519.
The implementation is in a separate bundle and is still not used in
the core org.eclipse.jgit bundle. The tests re-use the ssh tests from
the core test bundle.
Bug: 520927
Change-Id: Ib35e73c35799140fe050d1ff4fb18d0d3596580e Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Sun, 23 Sep 2018 13:44:10 +0000 (15:44 +0200)]
Add more ssh tests: pushing, known_host file handling, etc.
Add support for git-receive-pack to the ssh git server and add two
new tests for pushing.
This actually uncovered an undocumented requirement in TransportSftp:
the FTP rename operation assumes POSIX semantics, i.e., that the
target is removed. This works as written only for servers that
support and advertise the "posix-rename@openssh.com" FTP extension.
Our little Apache MINA server does not advertise this extension.
Fix the FtpChannel implementation for Jsch to handle this case in a
meaningful way so that it can pass the new "push over sftp" test.
Add more tests to test the behavior of server host key checking.
Also refactor the tests generally to separate better the test
framework from the actual tests.
Bug: 520927
Change-Id: Ia4bb85e17ddacde7b36ee8c2d5d454bbfa66dfc3 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Mon, 17 Sep 2018 18:36:36 +0000 (20:36 +0200)]
TransportSftp: eliminate dependency on Jsch
Introduce an FtpChannel abstraction, which can be obtained from a
RemoteSession. In JSchSession, wrap a JSch ChannelSftp as such an
FtpChannel. The JSch-specific SftpException is also mapped to a
generic FtpException. Rewrite TransportSftp to use only the new
abstraction layer.
This makes it possible to provide alternate ssh/sftp implementations.
Bug: 520927
Change-Id: I379026f7d4122f34931df909a28e73c02cd8a1da Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Jonathan Nieder [Fri, 9 Nov 2018 02:11:57 +0000 (18:11 -0800)]
Simplify RevWalk#iterator by factoring out common code
Factor out a helper that calls next() and tunnels IOException in a
RuntimeException, similar to TunnelException.tunnel(RevWalk::next) in
Guava terms[1].
This should make the code a little more readable. No functional
change intended.
Jonathan Tan [Thu, 8 Nov 2018 22:46:10 +0000 (14:46 -0800)]
DepthGenerator: fix multi-child boundary handling
Suppose that a repository has the following commit graph:
B C
\ /
A
and it was cloned with --shallow-exclude=A. DepthGenerator does not mark
C as shallow, causing an invalid repository to be produced on the
client, because A is not sent. (A similar issue occurs when
--shallow-since is used to exclude A but neither B nor C.)
This happens whenever an excluded commit has more than one child that is
to be sent to the client. Fix DepthGenerator to handle this case
correctly.
While we're editing DepthWalk.Commit, fix the documentation of
DepthWalk.Commit#isBoundary.
Change-Id: I7068abf0fe0c864d1b0e56e1616dad1aa8719411 Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Thomas Wolf [Fri, 21 Sep 2018 20:43:34 +0000 (22:43 +0200)]
Factor out a JSch-independent ssh config parser
Move the bulk of the basic parsing and host entry handling into a
new class OpenSshConfigFile that has no dependencies on any concrete
ssh implementation. Make the existing OpenSshConfig use the new
parser.
Introduce a new class SshConstants collecting all the various ssh-
related string literals. Also use TreeMaps with a case-insensitive
key comparator instead of converting keys to uppercase. Add a test
to verify that keys are matched case-insensitively.
Most of the parsing code was simply moved, except that the new
parser supports looking up entries given host name, port, and user
name, and can thus handle more %-substitutions correctly. This
feature is not yet used and cannot be used with JSch since JSch
only has a ConfigRepository.getConfig(String) interface.
The split is still worth the trouble as it opens the way to using
another ssh client altogether. Apache MINA sshd, for instance,
resolves host entries giving host name, port, and user name.
(Apache MINA has a built-in ssh config handling, but that has
problems, too: its pattern matching is case-insensitive, and its
merging of host entries if several match is not the same as in
OpenSsh. But with this refactoring, it will be possible to plug in
OpenSshConfigFile into an Apache MINA sshd client without dragging
along JSch.)
One test case that doesn't make sense anymore has been removed. It
tested that repeatedly querying for a host entry returned the same
object. That is no longer true since the caching has been moved to
a deeper level.
Bug: 520927
Change-Id: I6381d52b29099595e6eaf8b05c786aeeaefbf9cc Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Fri, 14 Sep 2018 20:40:08 +0000 (22:40 +0200)]
Ssh tests with an Apache MINA sshd test git server
Add a simple ssh git server based on Apache MINA sshd, and use it
in new tests that verify ssh operations and in particular a number
of bugs that had cropped up over time in JSch.
The git server supports fetching only, and sftp access.
The tests are all in an abstract base class; the concrete JschSshTest
class only provides ssh-specific test setup. So the same tests could
be run easily also with some other ssh client.
Bug: 520927
Change-Id: Ide6687b717fb497a29fc83f22b07390a26dfce1d Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Thu, 1 Nov 2018 12:53:25 +0000 (13:53 +0100)]
Fix test setup for Eclipse Photon and greater
When a source folder is marked as a test folder, JDT requires that it
has an output folder different from the one used for regular sources.
Therefore give the test folders in org.eclipse.jgit.test a separate
output folder "bin-tst".
Moreover JDT reports errors if non-test classes have dependencies on
test classes. Therefore remove the "test" annotation from
org.eclipse.jgit.junit.
Change-Id: Ib527439ff5b7d7b570b8a60819ecaa70f59c63a3 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Thomas Wolf [Fri, 2 Nov 2018 17:38:23 +0000 (18:38 +0100)]
Update JGit target platforms to Orbit I20181102163257
Remove the S20181031145145 Orbit definition; that one was for our
purposes broken (mistakes in the org.apache.sshd-core and
net.i2p.crypto.eddsa receipes).
Change-Id: I8246d0837bc80165dd23780236b58fd9fe2fe0bc Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Ivan Frade [Sat, 27 Oct 2018 00:03:40 +0000 (17:03 -0700)]
RepoCommand: Preserve executable bit in <copyfile>
The copyfile entry in the manifest file copies the contents of the file
but doesn't keep the executable flag. This is inconsistent with repo
tool behaviour, plus is natural to expect that the copy of a executable
file is executable.
Transfer the executable bit when copying the file, aligning the
RepoCommand with repo tool and user expectations.
Change-Id: I01b24f482d5939e01d496f032388b3a5c02a912a Signed-off-by: Ivan Frade <ifrade@google.com>
Ivan Frade [Tue, 30 Oct 2018 18:51:49 +0000 (11:51 -0700)]
RepoCommand.RemoteReader: Add method to read contents and mode of file
The RepoCommand.RemoteReader interface doesn't offer access to the mode
of a file. Caller can only default to mark the copied objects as regular
files, losing e.g. the executable bit (if set).
Add a new method readFileWithMode that returns the contents and mode of
the remote file. It supersedes the readFile method, that is marked as
deprecated.
Now callers can set correctly the file mode of the copied file.
Change-Id: I8fce01e4bc5707434c0cbc4aebbae1b6b64756f0 Signed-off-by: Ivan Frade <ifrade@google.com>
Matthias Sohn [Sun, 28 Oct 2018 23:40:00 +0000 (00:40 +0100)]
Silence API errors for new methods added to DepthWalk
237abe6a added method getDeepenNots() with a default implementation and
method getDeepenNotFlag() to the interface DepthWalk. This affects
implementers which is ok in minor release following OSGi semantic
versioning.
Change-Id: I1c872da261fc6825e1e310127761b8b8a6d397d4 Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Michael Keppler [Sat, 27 Oct 2018 19:55:39 +0000 (21:55 +0200)]
Fix typos
s/occured/occurred/
s/stablished/established/
Change-Id: Ib052bef60bd18043001a30ed43754ee1a5182016 Signed-off-by: Michael Keppler <Michael.Keppler@gmx.de> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
Ivan Frade [Fri, 21 Sep 2018 19:08:33 +0000 (12:08 -0700)]
ObjectIdMatcher: Custom matcher for sets of ObjectIds
Parsed requests represent object ids (SHA1) in ObjectId instances but tests
use strings for those ids because they are easier to define.
Create a custom matcher that hides the conversion from string to
ObjectId. Note that this reverses the existing code conversion (it was
transforming ObjectIds into string).
This produces more readable code, consistent with the other hamcrest
assertions.
Change-Id: I47ba1d25557d791fe74fb93c740ff7de9923cc00 Signed-off-by: Ivan Frade <ifrade@google.com>
Jonathan Tan [Mon, 1 Oct 2018 22:50:47 +0000 (15:50 -0700)]
UploadPack: Implement deepen-not for protocol v2
This allows clients to use the --shallow-exclude parameter (producing a
"deepen-not <ref>" line when communicating with the server) in their fetch
commands when fetching against a JGit server using protocol v2.
Note that the implementation in this commit is somewhat inefficient, as
described in the TODO comment in DepthGenerator.
Change-Id: I9fad3ed9276b624d8f668356ffd99a067dc67ef7 Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Jonathan Tan [Tue, 2 Oct 2018 22:18:43 +0000 (15:18 -0700)]
Throw error when deepen-since excludes all commits
In C Git, when a client fetches with "git fetch --shallow-since=<date>
origin <ref>", and all commits reachable from <ref> are older than
<date>, the server dies with a message "no commits selected for shallow
requests". That is, (1) the --shallow-since filter applies to the commit
pointed to by the ref itself, and (2) there is a check that at least one
commit is not filtered out. (The pack-protocol.txt documentation does
not describe this, but the C implementation does this.)
The implementation in commit 1bb430dc21 ("UploadPack: support
deepen-since in protocol v2", 2018-09-27) does neither (1) nor (2), so
do both of these.
Change-Id: I9946327a71627626ecce34ca2d017d2add8867fc Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Markus Keller [Tue, 16 Oct 2018 14:25:56 +0000 (16:25 +0200)]
Make PrePushHook properly terminate ref lines
All of the input lines passed to pre-push hook scripts must be properly
terminated by '\n', so that normal shell scripts like the git-supplied
pre-push.sample work properly, even when pushing just a single branch.
With the old code, hook scripts that use the following pattern didn't
process the last line, because 'read' has a non-zero exit status when
EOF is encountered:
while read local_ref local_sha remote_ref remote_sha; do ... done
Change-Id: Id899662ed3fedef6c314fc4b2ddf91a6dcb98cbb Signed-off-by: Markus Keller <markus.kell.r@gmail.com>
Ned Twigg [Tue, 21 Aug 2018 21:44:28 +0000 (14:44 -0700)]
CheckoutCommand: force flag now allows overwrite
Before this commit, a force checkout would fail if there
were any conflicting files. After this commit, a force
checkout will overwrite the conflicting files, as expected.
Making this work required fixing a bug in DirCacheCheckout.
Before this commit, when DirCacheCheckout had
failOnConflict=false, it would delete all conflicting files
from the working copy and just leave them missing. After
this commit, DirCacheCheckout overwrites conflicting files
with the merge tree.
This change in DirCacheCheckout causes "reset --hard" and
"revert --abort" to behave as expected (previously they
would simply delete conflicting files, now they will be
overwritten from the merge tree).
Change-Id: If7e328ee792ef6511ab7d9c26d8d77c39210ec9f Signed-off-by: Ned Twigg <ned.twigg@diffplug.com>
Ivan Frade [Wed, 17 Oct 2018 23:52:30 +0000 (16:52 -0700)]
UploadPack: Return correct peer user agent on v2 requests
UploadPack.getPeerUserAgent() doesn't produce the expected results for
protocol v2 requests. In v2, the agent reported in the request (in an
"agent=" line) is not in the clientCapabilities but in a field on its
own. This makes getPeerUserAgent default to the transport user agent.
Making "agent" a shared property between protocol v0/v1 and v2 fixes the
problem, simplifies the function and harmonizes the implementation
between protocol versions.
In a follow up commit the "agent" will be identified on parsing time,
instead of taking it from the client capabilities.
Change-Id: Idf9825ec4e0b81a1458c8e3701f3e28aafd8a32a Signed-off-by: Ivan Frade <ifrade@google.com>
Ivan Frade [Thu, 26 Jul 2018 22:49:09 +0000 (15:49 -0700)]
Accept protocol v2 server options on fetch and ls-refs requests
In protocol v2, a command request can be followed by server options
(lines like "agent=<>" and "server-option=<>"), but current code
doesn't accept those lines.
Advertise the "server-option" capability, parse the lines and add
them to the request objects.
Other code in JGit can see this options and act accordingly via the
protocol v2 hooks.
This should not require any change in the client side.
Change-Id: If3946390f9cc02d29644b6ca52534b6f757bda9f Signed-off-by: Ivan Frade <ifrade@google.com>
Saša Živkov [Thu, 4 Oct 2018 12:08:41 +0000 (14:08 +0200)]
ssh: Prefer algorithms of the known host keys
JSch prefers ssh-rsa key type. When the remote server supports ssh-rsa
key type then this key type will be used even if the known_hosts file
contains a host key for that host, but with different key type.
This caused an unexpected UnknownHostKey error.
To fix the issue first scan the known_hosts, the HostKeyRepository in
JSch API, for any already existing host keys for the target host and
modify the default session settings to prefer their algorithms. However,
do this only if there is no HostKeyAlgorithms setting active.
David Pursehouse [Wed, 17 Oct 2018 01:40:36 +0000 (10:40 +0900)]
Disable Eclipse warning about unrecognized @SuppressWarnings value
The code base has several @SuppressWarnings annotations to suppress
warnings raised by Error Prone, but those are not recognized by
Eclipse and there is currently no way to tell it about them [1].
David Pursehouse [Fri, 12 Oct 2018 02:07:48 +0000 (11:07 +0900)]
UnionInputStreamTest: Suppress ErrorProne warning about missing synchronized
Error Prone reports that the unsynchronized method skip overrides the
synchronized method in ByteArrayInputStream [1].
This is a test class, so we can just suppress the warning as recommended
in [1].
Note that the suppression causes a warning in Eclipse, because it doesn't
recognize the "UnsynchronizedOverridesSynchronized" as a valid value for
the @SuppressWarnings annotation [2].
Ivan Frade [Tue, 18 Sep 2018 22:52:48 +0000 (15:52 -0700)]
UploadPack: Use request instead of field for depth
One more step in removing state from UploadPack, using the request
object instead.
Unfortunately, hooks get from UploadPack information about the current
request. Changing the hooks to receive the request is a public API
change, so at the moment lets keep a reference to the current request.
This kills half the benefit of using a request object vs fields, but
at least we still get better modularity.
Change-Id: I86803d876a621b727c66ee73f2880c93190629e9 Signed-off-by: Ivan Frade <ifrade@google.com>
Ivan Frade [Tue, 16 Oct 2018 20:56:43 +0000 (13:56 -0700)]
UploadPack: Use request in computeShallowUnshallow
All data required in this function is available in the request object.
Use that object instead of class members. This reduces class state and
is more readable.
Make the function use a request object and remove the now unnecessary
field "deepenNotRefs".
Change-Id: If861e44c2860a78cf19f456d1b3feb7ddc314cce Signed-off-by: Ivan Frade <ifrade@google.com>
Ivan Frade [Tue, 16 Oct 2018 19:51:00 +0000 (12:51 -0700)]
Move deepenSince and deepenNotRefs up to FetchRequest
These properties are protocol v2 specific, but they have clear default
no-op values and having them in the common superclass simplifies client
code.
Move properties deepenSince and deepenNotRefs up to FetchRequest. In
FetchV0Request, they are initialized with their no-op values (0 for
deepenSince and empty list for deepenNotRefs)
Change-Id: I9d46a6dfbe29ebd794b5a6482033cdc70d411a23 Signed-off-by: Ivan Frade <ifrade@google.com>
projects / jgit.git / log