Skip to content

Commit

Permalink
GH-364: Use flags for RSA signature requests in AbstractAgentClient
Browse files Browse the repository at this point in the history
Respect the flags in OpenSSH signing requests. These flags tell what
kind of signature should be produced for an RSA key: ssh-rsa,
rsa-sha2-256, or rsa-sha2-512.

Previous code only set the flags on the client side, but ignored
them in the server side and thus always answered with an ssh-rsa
signature.

Bug: #364
  • Loading branch information
tomaswolf committed May 2, 2023
1 parent 9f5c03a commit d4b951a
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 2 deletions.
3 changes: 2 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
* [GH-300](https://github.com/apache/mina-sshd/issues/300) Read the channel id in `SSH_MSG_CHANNEL_OPEN_CONFIRMATION` as unsigned int.
* [GH-313](https://github.com/apache/mina-sshd/issues/313) Log exceptions in the SFTP subsystem before sending a failure status reply.
* [GH-322](https://github.com/apache/mina-sshd/issues/322) Add basic Android O/S awareness.
* [GH-325](https://github.com/apache/mina-sshd/issues/325) SftpFileSystemProvider: fix deletions of symlinks through `Files.delete()`.
* [GH-325](https://github.com/apache/mina-sshd/issues/325) `SftpFileSystemProvider`: fix deletions of symlinks through `Files.delete()`.
* [GH-364](https://github.com/apache/mina-sshd/issues/364) `AbstractAgentClient`: respect flags for RSA signature requests.


* [SSHD-1295](https://issues.apache.org/jira/browse/SSHD-1295) Fix cancellation of futures and add options to cancel futures on time-outs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.sshd.agent.SshAgent;
import org.apache.sshd.agent.SshAgentConstants;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.BufferUtils;
Expand Down Expand Up @@ -130,7 +131,23 @@ protected void process(int cmd, Buffer req, Buffer rep) throws Exception {
KeyUtils.getKeyType(signingKey),
"Cannot resolve key type of %s",
signingKey.getClass().getSimpleName());
Map.Entry<String, byte[]> result = agent.sign(null, signingKey, keyType, data);
String signatureAlgorithm = keyType;
if (KeyPairProvider.SSH_RSA.equals(keyType)) {
switch (flags) {
case 4:
signatureAlgorithm = KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS;
break;
case 2:
signatureAlgorithm = KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS;
break;
case 0:
break;
default:
throw new IllegalArgumentException("SSH2_AGENTC_SIGN_REQUEST: Unknown flag value 0x"
+ Integer.toHexString(flags) + ", only 0, 2, or 4 are allowed.");
}
}
Map.Entry<String, byte[]> result = agent.sign(null, signingKey, signatureAlgorithm, data);
String algo = result.getKey();
byte[] signature = result.getValue();
Buffer sig = new ByteArrayBuffer(algo.length() + signature.length + Long.SIZE, false);
Expand Down
134 changes: 134 additions & 0 deletions sshd-core/src/test/java/org/apache/sshd/agent/AgentUnitTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.sshd.agent;

import java.io.IOException;
import java.security.KeyPair;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import org.apache.sshd.agent.common.AbstractAgentClient;
import org.apache.sshd.agent.common.AbstractAgentProxy;
import org.apache.sshd.agent.local.AgentImpl;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.keyprovider.KeyPairProvider;
import org.apache.sshd.common.signature.BuiltinSignatures;
import org.apache.sshd.common.signature.Signature;
import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
import org.apache.sshd.common.util.security.SecurityUtils;
import org.apache.sshd.util.test.BaseTestSupport;
import org.apache.sshd.util.test.NoIoTestCase;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

/**
* Simple short-circuited test for {@link AbstractAgentClient} and {@link AbstractAgentProxy}.
*/
@Category(NoIoTestCase.class)
@RunWith(Parameterized.class)
public class AgentUnitTest extends BaseTestSupport {

private String algorithm;

private BuiltinSignatures factory;

public AgentUnitTest(String algorithm, BuiltinSignatures factory) {
this.algorithm = algorithm;
this.factory = factory;
}

@Parameterized.Parameters(name = "{0}")
public static List<Object[]> getParameters() {
return Arrays.asList(new Object[] { KeyUtils.RSA_SHA512_KEY_TYPE_ALIAS, BuiltinSignatures.rsaSHA512 },
new Object[] { KeyUtils.RSA_SHA256_KEY_TYPE_ALIAS, BuiltinSignatures.rsaSHA256 },
new Object[] { KeyPairProvider.SSH_RSA, BuiltinSignatures.rsa });
}

@Test
public void testRsaSignature() throws Exception {
SshAgent agent = new AgentImpl();
KeyPair pair = SecurityUtils.getKeyPairGenerator(KeyUtils.RSA_ALGORITHM).generateKeyPair();
agent.addIdentity(pair, "test key");
Server server = new Server(agent);
Client client = new Client(server);
server.setClient(client);
byte[] data = { 'd', 'a', 't', 'a' };
Map.Entry<String, byte[]> result = client.sign(null, pair.getPublic(), algorithm, data);
assertEquals("Unexpected signature algorithm", algorithm, result.getKey());
byte[] signature = result.getValue();
Signature verifier = factory.get();
verifier.initVerifier(null, pair.getPublic());
verifier.update(null, data);
assertTrue("Signature should validate", verifier.verify(null, signature));
}

private static class Server extends AbstractAgentClient {

private Client client;

protected Server(SshAgent agent) {
super(agent);
}

@Override
protected void reply(Buffer buf) throws IOException {
client.setResult(buf.getCompactData());
}

void setClient(Client client) {
this.client = client;
}

void request(byte[] data) throws IOException {
messageReceived(ByteArrayBuffer.getCompactClone(data));
}
}

private static class Client extends AbstractAgentProxy {

private final Server server;

private byte[] result;

protected Client(Server server) {
super(null);
this.server = server;
}

@Override
protected Buffer request(Buffer buffer) throws IOException {
server.request(buffer.getCompactData());
Buffer received = ByteArrayBuffer.getCompactClone(result);
return new ByteArrayBuffer(received.getBytes());
}

@Override
public boolean isOpen() {
return true;
}

void setResult(byte[] data) {
result = data;
}
}
}

0 comments on commit d4b951a

Please sign in to comment.