You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by GitBox <gi...@apache.org> on 2020/03/01 02:48:01 UTC
[GitHub] [mina-sshd] JackOfMostTrades opened a new pull request #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
JackOfMostTrades opened a new pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116
This PR adds supports for the new OpenSSH "security key" key types added in the latest OpenSSH release. Details of the key types are available [here](https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.u2f).
This PR just intends to add support for verify peers using those key types. This does not support any private key operations, since that would probably involve interfacing the the hardware security key devices directly which is beyond the scope of what I'd really like to do. ;)
I've added unit tests that verify that authorized_keys files can be parsed with the new key types and that the new signature classes are behaving correctly (including correctly verifying the user presence flag). I've also manually tested that with these changes, a server started using the `DefaultAuthorizedKeysAuthenticator` will correctly accept connections from clients using these new key types.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on issue #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on issue #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#issuecomment-593129808
>> Added a new commit that should address the above comments.
Good work - I will merge it in soon (after testing it on my host) - probably tomorrow.
>> Thanks for the quick response and the thorough review!
My pleasure
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127501
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureSkECDSA.java
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.common.signature;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.impl.SkECDSAPublicKeyEntryDecoder;
+
+public class SignatureSkECDSA extends AbstractSecurityKeySignature {
+
+ public SignatureSkECDSA() {
+ super(SkECDSAPublicKeyEntryDecoder.KEY_TYPE);
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "ECDSA-SK";
Review comment:
Please make this a public literal constant of this class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127795
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/SkED25519BufferPublicKeyParser.java
##########
@@ -0,0 +1,55 @@
+/*
+ * 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.common.util.buffer.keys;
+
+import java.security.GeneralSecurityException;
+import java.security.PublicKey;
+import java.util.Collection;
+
+import org.apache.sshd.common.config.keys.impl.SkED25519PublicKeyEntryDecoder;
+import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.common.u2f.SkED25519PublicKey;
+import org.apache.sshd.common.util.buffer.Buffer;
+
+import net.i2p.crypto.eddsa.EdDSAPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkED25519BufferPublicKeyParser extends AbstractBufferPublicKeyParser<SkED25519PublicKey> {
+ public static final SkED25519BufferPublicKeyParser INSTANCE = new SkED25519BufferPublicKeyParser();
+
+ public SkED25519BufferPublicKeyParser() {
+ super(SkED25519PublicKey.class, SkED25519PublicKeyEntryDecoder.KEY_TYPE);
+ }
+
+ @Override
+ public Collection<String> getSupportedKeyTypes() {
Review comment:
Why is this override required ? All it does is call the superclass...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades closed pull request #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades closed pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127737
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/u2f/SkEcdsaPublicKey.java
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.common.u2f;
+
+import java.security.interfaces.ECPublicKey;
+
+public class SkEcdsaPublicKey implements SecurityKeyPublicKey<ECPublicKey> {
Review comment:
Please add a `toString` implementation (see previous remark on this issue as well)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades commented on issue #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades commented on issue #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#issuecomment-593130181
Create this JIRA issue for tracking: https://issues.apache.org/jira/browse/SSHD-972
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127927
##########
File path: sshd-core/src/main/java/org/apache/sshd/client/ClientBuilder.java
##########
@@ -79,7 +79,9 @@
BuiltinSignatures.nistp256,
BuiltinSignatures.nistp384,
BuiltinSignatures.nistp521,
+ BuiltinSignatures.sk_ecdsa_sha2_nistp256,
BuiltinSignatures.ed25519,
+ BuiltinSignatures.sk_ssh_ed25519,
Review comment:
I am not comfortable to adding it as default for the client (for now) - users who want this should make an "effort" (at least for now).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127405
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/SkED25519PublicKeyEntryDecoder.java
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.common.config.keys.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.sshd.common.config.keys.KeyEntryResolver;
+import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SkED25519PublicKey;
+import org.apache.sshd.common.util.security.eddsa.Ed25519PublicKeyDecoder;
+
+import net.i2p.crypto.eddsa.EdDSAPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkED25519PublicKeyEntryDecoder extends AbstractPublicKeyEntryDecoder<SkED25519PublicKey, PrivateKey> {
+ public static final String KEY_TYPE = "sk-ssh-ed25519@openssh.com";
+ public static final int MAX_APP_NAME_LENGTH = 1024;
+
+ public static final SkED25519PublicKeyEntryDecoder INSTANCE = new SkED25519PublicKeyEntryDecoder();
+
+ private static final String NO_TOUCH_REQUIRED_HEADER = "no-touch-required";
+
+ public SkED25519PublicKeyEntryDecoder() {
+ super(SkED25519PublicKey.class, PrivateKey.class, Collections.singleton(KEY_TYPE));
+ }
+
+ @Override
+ public SkED25519PublicKey decodePublicKey(
+ SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
+ throws IOException, GeneralSecurityException {
+ if (!KEY_TYPE.equals(keyType)) {
+ throw new InvalidKeySpecException("Invalid keyType: " + keyType);
+ }
+
+ boolean noTouchRequired = false;
+ if (headers.containsKey(NO_TOUCH_REQUIRED_HEADER)) {
+ noTouchRequired = Boolean.parseBoolean(headers.get(NO_TOUCH_REQUIRED_HEADER));
Review comment:
Lets use `PropertyResolverUtils.parseBoolean(...)` - please also handle _null_ and/or _IllegalArgumentException_ that may occur for its invocation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on issue #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on issue #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#issuecomment-593135834
Merged with many thanks + acknowledgement - see https://github.com/apache/mina-sshd/commit/ff22a9b00c525a54ebf2490266afb88f3e528185. Please close the PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades commented on a change in pull request
#116: Add authorized_keys/verification support for OpenSSH "security key"
key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386130066
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/SkECDSAPublicKeyEntryDecoder.java
##########
@@ -0,0 +1,110 @@
+/*
+ * 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.common.config.keys.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.interfaces.ECPublicKey;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.KeyEntryResolver;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SkEcdsaPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkECDSAPublicKeyEntryDecoder extends AbstractPublicKeyEntryDecoder<SkEcdsaPublicKey, PrivateKey> {
+ public static final String KEY_TYPE = "sk-ecdsa-sha2-nistp256@openssh.com";
+ public static final int MAX_APP_NAME_LENGTH = 1024;
+
+ public static final SkECDSAPublicKeyEntryDecoder INSTANCE = new SkECDSAPublicKeyEntryDecoder();
+
+ private static final String NO_TOUCH_REQUIRED_HEADER = "no-touch-required";
+
+ public SkECDSAPublicKeyEntryDecoder() {
+ super(SkEcdsaPublicKey.class, PrivateKey.class, Collections.singleton(KEY_TYPE));
+ }
+
+ @Override
+ public SkEcdsaPublicKey decodePublicKey(
+ SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
+ throws IOException, GeneralSecurityException {
+ if (!KEY_TYPE.equals(keyType)) {
+ throw new InvalidKeySpecException("Invalid keyType: " + keyType);
+ }
+
+ boolean noTouchRequired = false;
+ if (headers.containsKey(NO_TOUCH_REQUIRED_HEADER)) {
Review comment:
Put this and the other boolean parsing updates into a helper method in the parent class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127524
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/u2f/SkED25519PublicKey.java
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.common.u2f;
+
+import net.i2p.crypto.eddsa.EdDSAPublicKey;
+
+public class SkED25519PublicKey implements SecurityKeyPublicKey<EdDSAPublicKey> {
+
+ private static final long serialVersionUID = 4587115316266869640L;
+
+ private final String appName;
+ private final boolean noTouchRequired;
+ private final EdDSAPublicKey delegatePublicKey;
+
+ public SkED25519PublicKey(String appName, boolean noTouchRequired, EdDSAPublicKey delegatePublicKey) {
+ this.appName = appName;
+ this.noTouchRequired = noTouchRequired;
+ this.delegatePublicKey = delegatePublicKey;
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "SK-ED25519";
Review comment:
Please make this a public literal constant of this class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades commented on a change in pull request
#116: Add authorized_keys/verification support for OpenSSH "security key"
key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386130066
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/SkECDSAPublicKeyEntryDecoder.java
##########
@@ -0,0 +1,110 @@
+/*
+ * 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.common.config.keys.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.interfaces.ECPublicKey;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.KeyEntryResolver;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SkEcdsaPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkECDSAPublicKeyEntryDecoder extends AbstractPublicKeyEntryDecoder<SkEcdsaPublicKey, PrivateKey> {
+ public static final String KEY_TYPE = "sk-ecdsa-sha2-nistp256@openssh.com";
+ public static final int MAX_APP_NAME_LENGTH = 1024;
+
+ public static final SkECDSAPublicKeyEntryDecoder INSTANCE = new SkECDSAPublicKeyEntryDecoder();
+
+ private static final String NO_TOUCH_REQUIRED_HEADER = "no-touch-required";
+
+ public SkECDSAPublicKeyEntryDecoder() {
+ super(SkEcdsaPublicKey.class, PrivateKey.class, Collections.singleton(KEY_TYPE));
+ }
+
+ @Override
+ public SkEcdsaPublicKey decodePublicKey(
+ SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
+ throws IOException, GeneralSecurityException {
+ if (!KEY_TYPE.equals(keyType)) {
+ throw new InvalidKeySpecException("Invalid keyType: " + keyType);
+ }
+
+ boolean noTouchRequired = false;
+ if (headers.containsKey(NO_TOUCH_REQUIRED_HEADER)) {
Review comment:
Put this and the other boolean parsing into a helper method in the parent class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127021
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.common.signature;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.PrivateKey;
+import java.security.PublicKey;
+
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SecurityKeyPublicKey;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+public abstract class AbstractSecurityKeySignature implements Signature {
+
+ private static final int FLAG_USER_PRESENCE = 0x01;
+
+ private final String keyType;
+
+ private SecurityKeyPublicKey publicKey;
+ private MessageDigest challengeDigest;
+
+ public AbstractSecurityKeySignature(String keyType) {
+ this.keyType = keyType;
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "SK-ED25519";
+ }
+
+ @Override
+ public void initVerifier(SessionContext session, PublicKey key) throws Exception {
+ if (!(key instanceof SecurityKeyPublicKey)) {
+ throw new IllegalArgumentException("Only instances of SecurityKeyPublicKey can be used");
+ }
+ this.publicKey = (SecurityKeyPublicKey) key;
+ this.challengeDigest = MessageDigest.getInstance("SHA-256");
Review comment:
Please use `SecurityUtils.getMessageDigest`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127193
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/SkED25519PublicKeyEntryDecoder.java
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.common.config.keys.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.sshd.common.config.keys.KeyEntryResolver;
+import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SkED25519PublicKey;
+import org.apache.sshd.common.util.security.eddsa.Ed25519PublicKeyDecoder;
+
+import net.i2p.crypto.eddsa.EdDSAPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkED25519PublicKeyEntryDecoder extends AbstractPublicKeyEntryDecoder<SkED25519PublicKey, PrivateKey> {
+ public static final String KEY_TYPE = "sk-ssh-ed25519@openssh.com";
+ public static final int MAX_APP_NAME_LENGTH = 1024;
+
+ public static final SkED25519PublicKeyEntryDecoder INSTANCE = new SkED25519PublicKeyEntryDecoder();
+
+ private static final String NO_TOUCH_REQUIRED_HEADER = "no-touch-required";
+
+ public SkED25519PublicKeyEntryDecoder() {
+ super(SkED25519PublicKey.class, PrivateKey.class, Collections.singleton(KEY_TYPE));
+ }
+
+ @Override
+ public SkED25519PublicKey decodePublicKey(
+ SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
+ throws IOException, GeneralSecurityException {
+ if (!KEY_TYPE.equals(keyType)) {
+ throw new InvalidKeySpecException("Invalid keyType: " + keyType);
+ }
+
+ boolean noTouchRequired = false;
+ if (headers.containsKey(NO_TOUCH_REQUIRED_HEADER)) {
Review comment:
Let's use if (GenericUtils.isNotEmpty(headers) && headers.contains(...)) just in case null is passed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades commented on a change in pull request
#116: Add authorized_keys/verification support for OpenSSH "security key"
key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386130144
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/SkECBufferPublicKeyParser.java
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.common.util.buffer.keys;
+
+import java.security.GeneralSecurityException;
+import java.security.interfaces.ECPublicKey;
+import java.util.Collection;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.impl.SkECDSAPublicKeyEntryDecoder;
+import org.apache.sshd.common.u2f.SkEcdsaPublicKey;
+import org.apache.sshd.common.util.buffer.Buffer;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkECBufferPublicKeyParser extends AbstractBufferPublicKeyParser<SkEcdsaPublicKey> {
+ public static final SkECBufferPublicKeyParser INSTANCE = new SkECBufferPublicKeyParser();
+
+ public SkECBufferPublicKeyParser() {
+ super(SkEcdsaPublicKey.class, SkECDSAPublicKeyEntryDecoder.KEY_TYPE);
+ }
+
+ @Override
+ public Collection<String> getSupportedKeyTypes() {
Review comment:
Oops, leftover code from some debugging. Removed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386126674
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.common.signature;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.PrivateKey;
+import java.security.PublicKey;
+
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SecurityKeyPublicKey;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+public abstract class AbstractSecurityKeySignature implements Signature {
+
+ private static final int FLAG_USER_PRESENCE = 0x01;
+
+ private final String keyType;
+
+ private SecurityKeyPublicKey publicKey;
+ private MessageDigest challengeDigest;
+
+ public AbstractSecurityKeySignature(String keyType) {
Review comment:
Please make this `protected`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127394
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/SkECDSAPublicKeyEntryDecoder.java
##########
@@ -0,0 +1,110 @@
+/*
+ * 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.common.config.keys.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.interfaces.ECPublicKey;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.KeyEntryResolver;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SkEcdsaPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkECDSAPublicKeyEntryDecoder extends AbstractPublicKeyEntryDecoder<SkEcdsaPublicKey, PrivateKey> {
+ public static final String KEY_TYPE = "sk-ecdsa-sha2-nistp256@openssh.com";
+ public static final int MAX_APP_NAME_LENGTH = 1024;
+
+ public static final SkECDSAPublicKeyEntryDecoder INSTANCE = new SkECDSAPublicKeyEntryDecoder();
+
+ private static final String NO_TOUCH_REQUIRED_HEADER = "no-touch-required";
+
+ public SkECDSAPublicKeyEntryDecoder() {
+ super(SkEcdsaPublicKey.class, PrivateKey.class, Collections.singleton(KEY_TYPE));
+ }
+
+ @Override
+ public SkEcdsaPublicKey decodePublicKey(
+ SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
+ throws IOException, GeneralSecurityException {
+ if (!KEY_TYPE.equals(keyType)) {
+ throw new InvalidKeySpecException("Invalid keyType: " + keyType);
+ }
+
+ boolean noTouchRequired = false;
+ if (headers.containsKey(NO_TOUCH_REQUIRED_HEADER)) {
+ noTouchRequired = Boolean.parseBoolean(headers.get(NO_TOUCH_REQUIRED_HEADER));
Review comment:
Lets use `PropertyResolverUtils.parseBoolean(...)` - please also handle _null_ and/or _IllegalArgumentException_ that may occur for its invocation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades commented on issue #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades commented on issue #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#issuecomment-593129648
Added a new commit that should address the above comments. Thanks for the quick response and the thorough review!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] JackOfMostTrades commented on issue #116: Add
authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
JackOfMostTrades commented on issue #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#issuecomment-593138577
Excellent. Thanks again!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127536
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/u2f/SkEcdsaPublicKey.java
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.common.u2f;
+
+import java.security.interfaces.ECPublicKey;
+
+public class SkEcdsaPublicKey implements SecurityKeyPublicKey<ECPublicKey> {
+
+ private static final long serialVersionUID = -8758432826838775097L;
+
+ private final String appName;
+ private final boolean noTouchRequired;
+ private final ECPublicKey delegatePublicKey;
+
+ public SkEcdsaPublicKey(String appName, boolean noTouchRequired, ECPublicKey delegatePublicKey) {
+ this.appName = appName;
+ this.noTouchRequired = noTouchRequired;
+ this.delegatePublicKey = delegatePublicKey;
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "SK-ECDSA";
Review comment:
Please make this a public literal constant of this class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127914
##########
File path: sshd-core/src/main/java/org/apache/sshd/client/ClientBuilder.java
##########
@@ -79,7 +79,9 @@
BuiltinSignatures.nistp256,
BuiltinSignatures.nistp384,
BuiltinSignatures.nistp521,
+ BuiltinSignatures.sk_ecdsa_sha2_nistp256,
Review comment:
I am not comfortable to adding it as default for the client (for now) - users who want this should make an "effort" (at least for now).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127507
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/signature/SignatureSkED25519.java
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.common.signature;
+
+import org.apache.sshd.common.config.keys.impl.SkED25519PublicKeyEntryDecoder;
+import org.apache.sshd.common.keyprovider.KeyPairProvider;
+import org.apache.sshd.common.util.security.SecurityUtils;
+
+public class SignatureSkED25519 extends AbstractSecurityKeySignature {
+
+ public SignatureSkED25519() {
+ super(SkED25519PublicKeyEntryDecoder.KEY_TYPE);
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "ED25519-SK";
Review comment:
Please make this a public literal constant of this class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386126762
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.common.signature;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.PrivateKey;
+import java.security.PublicKey;
+
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SecurityKeyPublicKey;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+public abstract class AbstractSecurityKeySignature implements Signature {
+
+ private static final int FLAG_USER_PRESENCE = 0x01;
+
+ private final String keyType;
+
+ private SecurityKeyPublicKey publicKey;
+ private MessageDigest challengeDigest;
+
+ public AbstractSecurityKeySignature(String keyType) {
+ this.keyType = keyType;
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "SK-ED25519";
Review comment:
Please make this a public literal constant of this class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127149
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/SkECDSAPublicKeyEntryDecoder.java
##########
@@ -0,0 +1,110 @@
+/*
+ * 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.common.config.keys.impl;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.security.GeneralSecurityException;
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.interfaces.ECPublicKey;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.KeyEntryResolver;
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SkEcdsaPublicKey;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkECDSAPublicKeyEntryDecoder extends AbstractPublicKeyEntryDecoder<SkEcdsaPublicKey, PrivateKey> {
+ public static final String KEY_TYPE = "sk-ecdsa-sha2-nistp256@openssh.com";
+ public static final int MAX_APP_NAME_LENGTH = 1024;
+
+ public static final SkECDSAPublicKeyEntryDecoder INSTANCE = new SkECDSAPublicKeyEntryDecoder();
+
+ private static final String NO_TOUCH_REQUIRED_HEADER = "no-touch-required";
+
+ public SkECDSAPublicKeyEntryDecoder() {
+ super(SkEcdsaPublicKey.class, PrivateKey.class, Collections.singleton(KEY_TYPE));
+ }
+
+ @Override
+ public SkEcdsaPublicKey decodePublicKey(
+ SessionContext session, String keyType, InputStream keyData, Map<String, String> headers)
+ throws IOException, GeneralSecurityException {
+ if (!KEY_TYPE.equals(keyType)) {
+ throw new InvalidKeySpecException("Invalid keyType: " + keyType);
+ }
+
+ boolean noTouchRequired = false;
+ if (headers.containsKey(NO_TOUCH_REQUIRED_HEADER)) {
Review comment:
Let's use `if (GenericUtils.isNotEmpty(headers) && headers.contains(...))` just in case *null* is passed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127701
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/u2f/SkED25519PublicKey.java
##########
@@ -0,0 +1,66 @@
+/*
+ * 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.common.u2f;
+
+import net.i2p.crypto.eddsa.EdDSAPublicKey;
+
+public class SkED25519PublicKey implements SecurityKeyPublicKey<EdDSAPublicKey> {
Review comment:
Please add a `toString` implementation - show appName, noTouchRequired and delegatePublicKey values... - remember to use the getters:
```java
@Override
public String toString() {
return getClass().getSimpleName()
+ "[appName=" + getAppName()
+ ", noTouch=" + isNoTouch()
+ ", delegateKey=" + getDelegateKey()
+ "]";
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127776
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/SkECBufferPublicKeyParser.java
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.common.util.buffer.keys;
+
+import java.security.GeneralSecurityException;
+import java.security.interfaces.ECPublicKey;
+import java.util.Collection;
+
+import org.apache.sshd.common.cipher.ECCurves;
+import org.apache.sshd.common.config.keys.impl.SkECDSAPublicKeyEntryDecoder;
+import org.apache.sshd.common.u2f.SkEcdsaPublicKey;
+import org.apache.sshd.common.util.buffer.Buffer;
+
+/**
+ * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
+ */
+public class SkECBufferPublicKeyParser extends AbstractBufferPublicKeyParser<SkEcdsaPublicKey> {
+ public static final SkECBufferPublicKeyParser INSTANCE = new SkECBufferPublicKeyParser();
+
+ public SkECBufferPublicKeyParser() {
+ super(SkEcdsaPublicKey.class, SkECDSAPublicKeyEntryDecoder.KEY_TYPE);
+ }
+
+ @Override
+ public Collection<String> getSupportedKeyTypes() {
Review comment:
Why is this override required ? All it does is call the superclass...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #116:
Add authorized_keys/verification support for OpenSSH "security key" key types
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on a change in pull request #116: Add authorized_keys/verification support for OpenSSH "security key" key types
URL: https://github.com/apache/mina-sshd/pull/116#discussion_r386127027
##########
File path: sshd-common/src/main/java/org/apache/sshd/common/signature/AbstractSecurityKeySignature.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.common.signature;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.PrivateKey;
+import java.security.PublicKey;
+
+import org.apache.sshd.common.session.SessionContext;
+import org.apache.sshd.common.u2f.SecurityKeyPublicKey;
+import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
+
+public abstract class AbstractSecurityKeySignature implements Signature {
+
+ private static final int FLAG_USER_PRESENCE = 0x01;
+
+ private final String keyType;
+
+ private SecurityKeyPublicKey publicKey;
+ private MessageDigest challengeDigest;
+
+ public AbstractSecurityKeySignature(String keyType) {
+ this.keyType = keyType;
+ }
+
+ @Override
+ public String getAlgorithm() {
+ return "SK-ED25519";
+ }
+
+ @Override
+ public void initVerifier(SessionContext session, PublicKey key) throws Exception {
+ if (!(key instanceof SecurityKeyPublicKey)) {
+ throw new IllegalArgumentException("Only instances of SecurityKeyPublicKey can be used");
+ }
+ this.publicKey = (SecurityKeyPublicKey) key;
+ this.challengeDigest = MessageDigest.getInstance("SHA-256");
+ }
+
+ @Override
+ public void update(SessionContext session, byte[] hash, int off, int len) {
+ if (challengeDigest == null) {
+ throw new IllegalStateException("initVerifier must be called before update");
+ }
+ challengeDigest.update(hash, off, len);
+ }
+
+ protected abstract String getSignatureKeyType();
+
+ protected abstract Signature getDelegateSignature();
+
+ @Override
+ public boolean verify(SessionContext session, byte[] sig) throws Exception {
+ if (challengeDigest == null) {
+ throw new IllegalStateException("initVerifier must be called before verify");
+ }
+
+ ByteArrayBuffer data = new ByteArrayBuffer(sig);
+ String keyType = data.getString();
+ if (!this.keyType.equals(keyType)) {
+ return false;
+ }
+ byte[] rawSig = data.getBytes();
+ byte flags = data.getByte();
+ long counter = data.getUInt();
+
+ // Return false if we don't understand the flags
+ if ((flags & ~FLAG_USER_PRESENCE) != 0) {
+ return false;
+ }
+ // Check user-presence flag is present if required by the public key
+ if ((flags & FLAG_USER_PRESENCE) != FLAG_USER_PRESENCE && !publicKey.isNoTouchRequired()) {
+ return false;
+ }
+
+ // Re-encode signature in a format to match the delegate
+ ByteArrayBuffer encoded = new ByteArrayBuffer();
+ encoded.putString(getSignatureKeyType());
+ encoded.putBytes(rawSig);
+
+ MessageDigest md = MessageDigest.getInstance("SHA-256");
Review comment:
Please use `SecurityUtils.getMessageDigest`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org