Preserve security key signature blobs

sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentClient.java

@@ -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.config.keys.u2f.SecurityKeyPublicKey;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
 import org.apache.sshd.common.util.ValidateUtils;
 import org.apache.sshd.common.util.buffer.Buffer;
@@ -152,7 +153,11 @@ protected void process(int cmd, Buffer req, Buffer rep) throws Exception {
                 byte[] signature = result.getValue();
                 Buffer sig = new ByteArrayBuffer(algo.length() + signature.length + Long.SIZE, false);
                 sig.putString(algo);
-                sig.putBytes(signature);
+                if (signingKey instanceof SecurityKeyPublicKey<?>) {
+                    sig.putRawBytes(signature);
+                } else {
+                    sig.putBytes(signature);
+                }
                 rep.putByte(SshAgentConstants.SSH2_AGENT_SIGN_RESPONSE);
                 rep.putBytes(sig.array(), sig.rpos(), sig.available());
                 break;

sshd-core/src/main/java/org/apache/sshd/agent/common/AbstractAgentProxy.java

@@ -32,9 +32,11 @@
 import org.apache.sshd.agent.SshAgentKeyConstraint;
 import org.apache.sshd.common.SshException;
 import org.apache.sshd.common.config.keys.KeyUtils;
+import org.apache.sshd.common.config.keys.u2f.SecurityKeyPublicKey;
 import org.apache.sshd.common.session.SessionContext;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.buffer.Buffer;
+import org.apache.sshd.common.util.buffer.BufferException;
 import org.apache.sshd.common.util.buffer.BufferUtils;
 import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
 import org.apache.sshd.common.util.logging.AbstractLoggingBean;
@@ -175,7 +177,14 @@ public Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, Str
         } else {
             Buffer buf = new ByteArrayBuffer(signature);
             String algorithm = buf.getString();
-            signature = buf.getBytes();
+            if (key instanceof SecurityKeyPublicKey<?>) {
+                signature = getSecurityKeySignature(buf);
+            } else {
+                signature = buf.getBytes();
+                if (buf.available() > 0) {
+                    throw new SshException("Trailing garbage in signing response, got " + buf.available() + " extra bytes");
+                }
+            }
             if (debugEnabled) {
                 log.debug("sign({}/{})[{}] {}: {}",
                         algo, keyType, KeyUtils.getFingerPrint(key), algorithm, BufferUtils.toHex(':', signature));
@@ -184,6 +193,23 @@ public Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, Str
         }
     }
 
+    private static byte[] getSecurityKeySignature(Buffer buf) throws SshException {
+        byte[] signature = buf.getCompactData();
+        Buffer verifier = new ByteArrayBuffer(signature);
+        try {
+            verifier.getBytes();
+            verifier.getByte();
+            verifier.getUInt();
+        } catch (BufferException e) {
+            throw new SshException("Signing response too short", e);
+        }
+        if (verifier.available() > 0) {
+            throw new SshException(
+                    "Trailing garbage in signing response, got " + verifier.available() + " extra bytes");
+        }
+        return signature;
+    }
+
     @Override
     public void addIdentity(KeyPair kp, String comment, SshAgentKeyConstraint... constraints) throws IOException {
         byte cmd = SshAgentConstants.SSH2_AGENTC_ADD_IDENTITY;

sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKey.java

@@ -41,6 +41,7 @@
 import org.apache.sshd.common.SshConstants;
 import org.apache.sshd.common.config.keys.KeyUtils;
 import org.apache.sshd.common.config.keys.OpenSshCertificate;
+import org.apache.sshd.common.config.keys.u2f.SecurityKeyPublicKey;
 import org.apache.sshd.common.kex.extension.DefaultClientKexExtensionHandler;
 import org.apache.sshd.common.kex.extension.parser.HostBoundPubkeyAuthentication;
 import org.apache.sshd.common.signature.Signature;
@@ -466,7 +467,11 @@ protected byte[] appendSignature(
 
         bs.clear();
         bs.putString(signatureAlgo);
-        bs.putBytes(sig);
+        if (key instanceof SecurityKeyPublicKey<?>) {
+            bs.putRawBytes(sig);
+        } else {
+            bs.putBytes(sig);
+        }
         buffer.putBytes(bs.array(), bs.rpos(), bs.available());
         return sig;
     }

sshd-core/src/test/java/org/apache/sshd/agent/AgentUnitTest.java

@@ -20,25 +20,35 @@
 
 import java.io.IOException;
 import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PublicKey;
+import java.security.interfaces.ECPublicKey;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.Arrays;
+import java.util.Collections;
 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.config.keys.u2f.SkEcdsaPublicKey;
 import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.common.session.SessionContext;
 import org.apache.sshd.common.signature.BuiltinSignatures;
 import org.apache.sshd.common.signature.Signature;
+import org.apache.sshd.common.util.ValidateUtils;
 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.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -83,6 +93,46 @@ public void rsaSignature(String algorithm, BuiltinSignatures factory) throws Exc
         assertTrue(verifier.verify(null, signature), "Signature should validate");
     }
 
+    @Test
+    public void securityKeySignatureBlob() throws Exception {
+        KeyPairGenerator generator = SecurityUtils.getKeyPairGenerator("EC");
+        generator.initialize(256);
+        KeyPair pair = generator.generateKeyPair();
+        ECPublicKey ecPublicKey = ValidateUtils.checkInstanceOf(
+                pair.getPublic(), ECPublicKey.class, "Expected an ECPublicKey");
+        SkEcdsaPublicKey key = new SkEcdsaPublicKey("ssh", false, false, ecPublicKey);
+
+        byte[] rawSignature = { 1, 2, 3, 4, 5 };
+        byte flags = 1;
+        long counter = 42;
+        byte[] signature = createSkSignature(rawSignature, flags, counter);
+
+        SshAgent agent = new StaticSignatureAgent(key, signature);
+        Server server = new Server(agent);
+        Client client = new Client(server);
+        server.setClient(client);
+
+        Map.Entry<String, byte[]> result = client.sign(null, key, key.getKeyType(), new byte[] { 'd', 'a', 't', 'a' });
+        assertEquals(key.getKeyType(), result.getKey(), "Unexpected signature algorithm");
+        assertSkSignature(rawSignature, flags, counter, result.getValue());
+    }
+
+    private static byte[] createSkSignature(byte[] rawSignature, byte flags, long counter) {
+        ByteArrayBuffer buffer = new ByteArrayBuffer();
+        buffer.putBytes(rawSignature);
+        buffer.putByte(flags);
+        buffer.putUInt(counter);
+        return buffer.getCompactData();
+    }
+
+    private static void assertSkSignature(byte[] rawSignature, byte flags, long counter, byte[] signature) {
+        ByteArrayBuffer buffer = new ByteArrayBuffer(signature);
+        assertArrayEquals(rawSignature, buffer.getBytes());
+        assertEquals(flags, buffer.getByte());
+        assertEquals(counter, buffer.getUInt());
+        assertEquals(0, buffer.available());
+    }
+
     private static class Server extends AbstractAgentClient {
 
         private Client client;
@@ -132,4 +182,49 @@ void setResult(byte[] data) {
             result = data;
         }
     }
+
+    private static class StaticSignatureAgent implements SshAgent {
+        private final PublicKey key;
+        private final byte[] signature;
+
+        StaticSignatureAgent(PublicKey key, byte[] signature) {
+            this.key = key;
+            this.signature = signature.clone();
+        }
+
+        @Override
+        public Iterable<? extends Map.Entry<PublicKey, String>> getIdentities() {
+            return Collections.singletonList(new SimpleImmutableEntry<>(key, "security key"));
+        }
+
+        @Override
+        public Map.Entry<String, byte[]> sign(SessionContext session, PublicKey key, String algo, byte[] data) {
+            return new SimpleImmutableEntry<>(KeyUtils.getKeyType(key), signature.clone());
+        }
+
+        @Override
+        public void addIdentity(KeyPair key, String comment, SshAgentKeyConstraint... constraints) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public void removeIdentity(PublicKey key) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public void removeAllIdentities() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean isOpen() {
+            return true;
+        }
+
+        @Override
+        public void close() {
+            // Nothing to close.
+        }
+    }
 }

sshd-core/src/test/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKeySkTest.java

@@ -0,0 +1,125 @@
+/*
+ * 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.client.auth.pubkey;
+
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.interfaces.ECPublicKey;
+import java.util.AbstractMap.SimpleImmutableEntry;
+import java.util.Collections;
+import java.util.Map;
+
+import org.apache.sshd.client.session.ClientSession;
+import org.apache.sshd.common.auth.AbstractUserAuthServiceFactory;
+import org.apache.sshd.common.config.keys.u2f.SkEcdsaPublicKey;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.util.ValidateUtils;
+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.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Unit tests for client-side sk-* authentication packet encoding.
+ */
+@Tag("NoIoTestCase")
+public class UserAuthPublicKeySkTest extends BaseTestSupport {
+
+    @Test
+    public void securityKeySignatureBlobIsNotWrapped() throws Exception {
+        byte[] rawSignature = { 1, 2, 3, 4, 5 };
+        byte flags = 1;
+        long counter = 42;
+
+        KeyPairGenerator generator = SecurityUtils.getKeyPairGenerator("EC");
+        generator.initialize(256);
+        KeyPair pair = generator.generateKeyPair();
+        ECPublicKey ecPublicKey = ValidateUtils.checkInstanceOf(
+                pair.getPublic(), ECPublicKey.class, "Expected an ECPublicKey");
+        SkEcdsaPublicKey sk = new SkEcdsaPublicKey("ssh", false, false, ecPublicKey);
+        byte[] skSignature = createSkSignature(rawSignature, flags, counter);
+
+        ClientSession session = Mockito.mock(ClientSession.class);
+        Mockito.when(session.getSessionId()).thenReturn(new byte[] { 9, 8, 7, 6 });
+
+        ExposedUserAuthPublicKey auth = new ExposedUserAuthPublicKey(new StaticSignatureIdentity(sk, skSignature));
+        byte[] actual = auth.appendSignature(
+                session, AbstractUserAuthServiceFactory.DEFAULT_NAME, UserAuthPublicKey.NAME, "testuser", sk.getKeyType(), sk);
+
+        assertSkSignature(sk, rawSignature, flags, counter, actual);
+    }
+
+    private static byte[] createSkSignature(byte[] rawSignature, byte flags, long counter) {
+        ByteArrayBuffer buffer = new ByteArrayBuffer();
+        buffer.putBytes(rawSignature);
+        buffer.putByte(flags);
+        buffer.putUInt(counter);
+        return buffer.getCompactData();
+    }
+
+    private static void assertSkSignature(
+            SkEcdsaPublicKey key, byte[] rawSignature, byte flags, long counter, byte[] signature) {
+        ByteArrayBuffer buffer = new ByteArrayBuffer(signature);
+        assertEquals(key.getKeyType(), buffer.getString());
+        assertArrayEquals(rawSignature, buffer.getBytes());
+        assertEquals(flags, buffer.getByte());
+        assertEquals(counter, buffer.getUInt());
+        assertEquals(0, buffer.available());
+    }
+
+    private static class ExposedUserAuthPublicKey extends UserAuthPublicKey {
+        ExposedUserAuthPublicKey(PublicKeyIdentity identity) {
+            super(Collections.emptyList());
+            current = identity;
+        }
+
+        byte[] appendSignature(
+                ClientSession session, String service, String name, String username, String algo, SkEcdsaPublicKey key)
+                throws Exception {
+            ByteArrayBuffer buffer = new ByteArrayBuffer();
+            super.appendSignature(session, service, name, username, algo, key, null, buffer);
+            return buffer.getBytes();
+        }
+    }
+
+    private static class StaticSignatureIdentity implements PublicKeyIdentity {
+        private final SkEcdsaPublicKey key;
+        private final byte[] signature;
+
+        StaticSignatureIdentity(SkEcdsaPublicKey key, byte[] signature) {
+            this.key = key;
+            this.signature = signature.clone();
+        }
+
+        @Override
+        public KeyPair getKeyIdentity() {
+            return new KeyPair(key, null);
+        }
+
+        @Override
+        public Map.Entry<String, byte[]> sign(SessionContext session, String algo, byte[] data) {
+            return new SimpleImmutableEntry<>(key.getKeyType(), signature.clone());
+        }
+    }
+}