Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Degrade: when ssh/config IdentityFile is secret key, raise exception java.io.StreamCorruptedException: Failed (IllegalArgumentException) to parse key entry=-----BEGIN OPENSSH PRIVATE KEY-----: Bad format (no key data delimiter): KEY----- #53

Closed
miurahr opened this issue May 5, 2024 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@miurahr
Copy link

miurahr commented May 5, 2024

Version

6.9.0.202403050737-r

Operating System

Linux/Unix

Bug description

Issue #25 changes to try a file path specified in IdentityFile then try with an file extension ".pub".
When user use traditional configuration that IdentityFile path is secret key on file system, JGit and apache MINA sshd library raise an exception and show a stack trace.
A problem is only raised when user also configure IdentiesOnly = yes.

Here is gerrit entry of the change
https://eclipse.gerrithub.io/c/eclipse-jgit/jgit/+/1177073/6/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/internal/transport/sshd/JGitPublicKeyAuthentication.java

In previous code,

Path p = Paths.get(s + ".pub"); //$NON-NLS-1$
if (Files.isRegularFile(p, LinkOption.NOFOLLOW_LINKS)) {
	return AuthorizedKeyEntry.readAuthorizedKeys(p).get(0).resolvePublicKey(null,	PublicKeyEntryResolver.IGNORING);
}

It just adding ".pub" and check it, then no exception was observed.

Actual behavior

Record stack trace in log file.

Expected behavior

Run without exception or don't show a stack trace.

I think we can check existence of public key before try a file path of IdentityFile specified.

Approach 1. Check existence of file path
Path p = Paths.get(s + ".pub"); //$NON-NLS-1$ and if exists, try it first.

Approach 2. Check specified path endsWith ".pub"

There is not necessary to put a file name rule in a new approach that file system hold only a public key and secret key is in HSM, approach 2 is not stable for future. Old style always has "foo.pub" and "foo" key pair, so approach 1 is better.

Relevant log output

Cannot read public key from file /home/miurahr/.ssh/id_ed25519 
java.io.StreamCorruptedException: Failed (IllegalArgumentException) to parse key entry=-----BEGIN OPENSSH PRIVATE KEY-----: Bad format (no key data delimiter): KEY----- 
	at org.apache.sshd.common.config.keys.AuthorizedKeyEntry.readAuthorizedKeys(AuthorizedKeyEntry.java:247) 
 	at org.apache.sshd.common.config.keys.AuthorizedKeyEntry.readAuthorizedKeys(AuthorizedKeyEntry.java:226) 
 	at org.apache.sshd.common.config.keys.AuthorizedKeyEntry.readAuthorizedKeys(AuthorizedKeyEntry.java:211) 
 	at org.apache.sshd.common.config.keys.AuthorizedKeyEntry.readAuthorizedKeys(AuthorizedKeyEntry.java:195) 
	at org.apache.sshd.common.config.keys.KeyUtils.loadPublicKey(KeyUtils.java:342) 
	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication$KeyIterator.readPublicKey(JGitPublicKeyAuthentication.java:353) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication$KeyIterator.lambda$0(JGitPublicKeyAuthentication.java:330) 
 	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) 
 	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625) 
 	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) 
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) 
 	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) 
 	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) 
 	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication$KeyIterator.getExplicitKeys(JGitPublicKeyAuthentication.java:337) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication$KeyIterator.identitiesOnly(JGitPublicKeyAuthentication.java:434) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication$KeyIterator.initializeAgentIdentities(JGitPublicKeyAuthentication.java:383) 
 	at org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator.<init>(UserAuthPublicKeyIterator.java:59) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication$KeyIterator.<init>(JGitPublicKeyAuthentication.java:320) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication.createPublicKeyIterator(JGitPublicKeyAuthentication.java:137) 
 	at org.apache.sshd.client.auth.pubkey.UserAuthPublicKey.init(UserAuthPublicKey.java:109) 
 	at org.eclipse.jgit.internal.transport.sshd.JGitPublicKeyAuthentication.init(JGitPublicKeyAuthentication.java:123) 
 	at org.apache.sshd.client.session.ClientUserAuthService.tryNext(ClientUserAuthService.java:410) 
 	at org.apache.sshd.client.session.ClientUserAuthService.processUserAuth(ClientUserAuthService.java:331) 
 	at org.apache.sshd.client.session.ClientUserAuthService.process(ClientUserAuthService.java:267) 
 	at org.apache.sshd.common.session.helpers.CurrentService.process(CurrentService.java:109) 
 	at org.apache.sshd.common.session.helpers.AbstractSession.doHandleMessage(AbstractSession.java:624) 
 	at org.apache.sshd.common.session.helpers.AbstractSession.lambda$handleMessage$0(AbstractSession.java:545) 
 	at org.apache.sshd.common.util.threads.ThreadUtils.runAsInternal(ThreadUtils.java:68) 
 	at org.apache.sshd.common.session.helpers.AbstractSession.handleMessage(AbstractSession.java:544) 
 	at org.apache.sshd.common.session.helpers.AbstractSession.decode(AbstractSession.java:1688) 
	at org.apache.sshd.common.session.helpers.AbstractSession.messageReceived(AbstractSession.java:505) 
	at org.eclipse.jgit.internal.transport.sshd.JGitClientSession.messageReceived(JGitClientSession.java:208) 
 	at org.apache.sshd.common.session.helpers.AbstractSessionIoHandler.messageReceived(AbstractSessionIoHandler.java:64) 
 	at org.apache.sshd.common.io.nio2.Nio2Session.handleReadCycleCompletion(Nio2Session.java:409) 
 	at org.apache.sshd.common.io.nio2.Nio2Session$1.onCompleted(Nio2Session.java:382) 
 	at org.apache.sshd.common.io.nio2.Nio2Session$1.onCompleted(Nio2Session.java:377) 
 	at org.apache.sshd.common.io.nio2.Nio2CompletionHandler.lambda$completed$0(Nio2CompletionHandler.java:38) 
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:318) 
	at org.apache.sshd.common.io.nio2.Nio2CompletionHandler.completed(Nio2CompletionHandler.java:37) 
	at java.base/sun.nio.ch.Invoker.invokeUnchecked(Invoker.java:129) 
 	at java.base/sun.nio.ch.Invoker$2.run(Invoker.java:221) 
 	at java.base/sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:113) 
 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) 
 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) 
 	at java.base/java.lang.Thread.run(Thread.java:840)

Other information

No response

miurahr added a commit to miurahr/jgit that referenced this issue May 5, 2024
@miurahr
Copy link
Author

miurahr commented May 5, 2024

In the change in #25, it try to catch exception when trying reading a private key, but it did not catch because apache sshd raises StreamCorruptedException but not GeneralSecurityException.

			} catch (GeneralSecurityException e) {
				// ignore in case this is not a derived key path, as in most
				// cases this specifies a private key
				if (isDerived) {
					log.warn("{}", //$NON-NLS-1$
							format(SshdText.get().cannotReadPublicKey, keyFile),
							e);
				}

@tomaswolf
Copy link
Contributor

Gerrit change 1194667 will fix that.

@tomaswolf tomaswolf added this to the 6.10.0 milestone May 14, 2024
@tomaswolf tomaswolf self-assigned this May 14, 2024
@tomaswolf tomaswolf added the bug Something isn't working label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants