[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409762691 ## File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java ## @@ -0,0 +1,262 @@ +/* + * 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; + +import java.security.PrivateKey; +import java.security.PublicKey; +import java.util.List; + +public final class OpenSshCertificate implements PublicKey, PrivateKey { Review comment: I would prefer the builder over setters, because IMO it should not be possible to change parts of the certificate later on, wdyt? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409761749 ## File path: sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java ## @@ -367,9 +382,26 @@ protected String resolveAvailableSignaturesProposal(FactoryManager proposedManag KeyPairProvider kpp = getKeyPairProvider(); boolean debugEnabled = log.isDebugEnabled(); -Iterable provided; +Set provided = null; try { -provided = (kpp == null) ? null : kpp.getKeyTypes(this); +if (kpp != null) { +provided = StreamSupport.stream(kpp.getKeyTypes(this).spliterator(), false) +.collect(Collectors.toSet()); + +HostKeyCertificateProvider hostKeyCertificateProvider = getHostKeyCertificateProvider(); +if (hostKeyCertificateProvider != null) { +Iterable certificates = hostKeyCertificateProvider.loadCertificates(this); +for (OpenSshCertificate certificate : certificates) { +// Add the certificate alg only if the corresponding keyPair type is available +if (provided.contains(certificate.getRawKeyType())) { +provided.add(certificate.getKeyType()); +} else { Review comment: Not sure if I understand the issue. The call to `kpp.getKeyTypes(this)` did not change. Loading all provided certificates should not be an issue and is required to get their type? (They should not be password protected since they are public.) 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409752593 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { Review comment: >A certificate is considered valid if: The <= should be a strict <, but with the negation it should be correct besides this. Is it more clear this way? `if (!(openSshKey.getValidAfter() <= now && now < openSshKey.getValidBefore())) {` 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409752593 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { Review comment: >A certificate is considered valid if: The <= should be a strict <, but with the negation it should be correct besides this. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409752593 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { Review comment: >A certificate is considered valid if: The <= should be a strict <, but with the negation it should be correct besides this. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409744999 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/KeyPairProvider.java ## @@ -122,7 +132,7 @@ default KeyPair loadKey(SessionContext session, String type) /** * @param session The {@link SessionContext} for invoking this load command - may * be {@code null} if not invoked within a session context (e.g., offline tool). - * @return The available {@link Iterable} key types in preferred order - never {@code null} Review comment: This diff is very misleading. The line 125 was not changed, the change was in the doc of org.apache.sshd.common.keyprovider.KeyPairProvider#getKeyTypes in line 135, which returns a HashSet, which does NOT keep any order. The order is important in the other "part" later in AbstractServerSession: "expected" vs. "available" signatures... 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409725691 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, CA expired: " ++ openSshKey.getValidAfter() + "-" + openSshKey.getValidBefore()); +} + +SocketAddress connectSocketAddress = getClientSession().getConnectAddress(); +if (connectSocketAddress instanceof SshdSocketAddress) { +connectSocketAddress = ((SshdSocketAddress) connectSocketAddress).toInetSocketAddress(); +} +if (connectSocketAddress instanceof InetSocketAddress) { +String hostName = ((InetSocketAddress) connectSocketAddress).getHostString(); +if (!openSshKey.getPrincipals().contains(hostName)) { Review comment: Good to know - let's place a comment in the code explaining this 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates
FliegenKLATSCH commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409724904 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, CA expired: " ++ openSshKey.getValidAfter() + "-" + openSshKey.getValidBefore()); +} + +SocketAddress connectSocketAddress = getClientSession().getConnectAddress(); +if (connectSocketAddress instanceof SshdSocketAddress) { +connectSocketAddress = ((SshdSocketAddress) connectSocketAddress).toInetSocketAddress(); +} +if (connectSocketAddress instanceof InetSocketAddress) { +String hostName = ((InetSocketAddress) connectSocketAddress).getHostString(); +if (!openSshKey.getPrincipals().contains(hostName)) { Review comment: The IP would have to be in the principals of the certificate in the case you want to connect with the IP. OpenSSH also does not do a reverse lookup and I also would not expect this. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Commented] (SSHD-660) Add support for authentication using signed client/server keys
[ https://issues.apache.org/jira/browse/SSHD-660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084992#comment-17084992 ] Lyor Goldstein commented on SSHD-660: - Great - thx > Add support for authentication using signed client/server keys > -- > > Key: SSHD-660 > URL: https://issues.apache.org/jira/browse/SSHD-660 > Project: MINA SSHD > Issue Type: Improvement >Reporter: Lyor Goldstein >Priority: Minor > > Similar to _HostCertificate_ and _TrustedUserCAKeys_ configuration values - > see https://ef.gy/hardening-ssh -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409649556 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, CA expired: " ++ openSshKey.getValidAfter() + "-" + openSshKey.getValidBefore()); +} + +SocketAddress connectSocketAddress = getClientSession().getConnectAddress(); +if (connectSocketAddress instanceof SshdSocketAddress) { +connectSocketAddress = ((SshdSocketAddress) connectSocketAddress).toInetSocketAddress(); +} +if (connectSocketAddress instanceof InetSocketAddress) { +String hostName = ((InetSocketAddress) connectSocketAddress).getHostString(); +if (!openSshKey.getPrincipals().contains(hostName)) { Review comment: The code is assuming that the host name is the connect address - but what if user used an IP address instead of a fully-qualified host name to connect ? Do we do a reverse DNS ? (I am not too keen on it). 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409648503 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, CA expired: " ++ openSshKey.getValidAfter() + "-" + openSshKey.getValidBefore()); +} + +SocketAddress connectSocketAddress = getClientSession().getConnectAddress(); +if (connectSocketAddress instanceof SshdSocketAddress) { +connectSocketAddress = ((SshdSocketAddress) connectSocketAddress).toInetSocketAddress(); +} +if (connectSocketAddress instanceof InetSocketAddress) { +String hostName = ((InetSocketAddress) connectSocketAddress).getHostString(); +if (!openSshKey.getPrincipals().contains(hostName)) { Review comment: What if `getPrincipals` returns _null_ ? Perhaps better: ```java Collection principals = key.getPrincipals(); if (GenericUtils.isEmpty(principals) || (!principals.contains(hostName))) { throw... } ``` 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409645833 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { Review comment: This condition seems wrong - shouldn't it be the other way around ? to quote from the spec >> valid after <= current time < valid before 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409645833 ## File path: sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java ## @@ -154,4 +168,54 @@ public boolean next(int cmd, Buffer buffer) throws Exception { return true; } + +protected void verifyCertificate(Session session, OpenSshCertificate openSshKey) throws Exception { +PublicKey signatureKey = openSshKey.getCaPubKey(); +String keyAlg = KeyUtils.getKeyType(signatureKey); +Signature verif = ValidateUtils.checkNotNull( +NamedFactory.create(session.getSignatureFactories(), keyAlg), +"No verifier located for algorithm=%s", keyAlg); +verif.initVerifier(session, signatureKey); +verif.update(session, openSshKey.getMessage()); +if (!verif.verify(session, openSshKey.getSignature())) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange CA signature verification failed for key type=" + keyAlg); +} + +if (openSshKey.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED, +"KeyExchange signature verification failed, not a host key (2): " ++ openSshKey.getType()); +} + +long now = System.currentTimeMillis() / 1000; +if (now <= openSshKey.getValidAfter() || now >= openSshKey.getValidBefore()) { Review comment: This condition seems wrong - shouldn't it be the other way around ? I.e., it is valid if `before <= now <= after` 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409643495 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/KeyPairProvider.java ## @@ -122,7 +132,7 @@ default KeyPair loadKey(SessionContext session, String type) /** * @param session The {@link SessionContext} for invoking this load command - may * be {@code null} if not invoked within a session context (e.g., offline tool). - * @return The available {@link Iterable} key types in preferred order - never {@code null} Review comment: Please note that **order** is assumed here so to remove this documentation and/or behavior is a major change - especially for KEX. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409642359 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/FileHostKeyCertificateProvider.java ## @@ -0,0 +1,88 @@ +/* + * 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.keyprovider; + +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.security.GeneralSecurityException; +import java.security.PublicKey; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.stream.StreamSupport; + +import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.config.keys.PublicKeyEntry; +import org.apache.sshd.common.session.SessionContext; +import org.apache.sshd.common.util.ValidateUtils; +import org.apache.sshd.common.util.io.IoUtils; +import org.apache.sshd.common.util.logging.AbstractLoggingBean; + +public class FileHostKeyCertificateProvider extends AbstractLoggingBean implements HostKeyCertificateProvider { +private Collection files; + +public FileHostKeyCertificateProvider() { +super(); +} + +public FileHostKeyCertificateProvider(Path path) { +this(Collections.singletonList(Objects.requireNonNull(path, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Path... files) { +this(Arrays.asList(ValidateUtils.checkNotNullAndNotEmpty(files, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Collection files) { +this.files = files; +} + +public Collection getPaths() { +return files; +} + +@Override +public Iterable loadCertificates(SessionContext session) throws IOException, GeneralSecurityException { + +List certificates = new ArrayList<>(); +for (Path file : files) { +List lines = IoUtils.readAllLines(new FileInputStream(file.toFile())); +for (String line : lines) { +PublicKeyEntry publicKeyEntry = PublicKeyEntry.parsePublicKeyEntry(line); + +PublicKey publicKey = publicKeyEntry.resolvePublicKey(session, null, null); + +certificates.add((OpenSshCertificate) publicKey); +} +} + +return certificates; +} + +@Override +public OpenSshCertificate loadCertificate(SessionContext session, String keyType) throws IOException, GeneralSecurityException { +return StreamSupport.stream(loadCertificates(session).spliterator(), false) +.filter(pubKey -> pubKey.getKeyType().equals(keyType)) Review comment: Let;s use: `filter(pubKey -> Objects.equals(keyType, pubKey.getKeyType()))` 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409641226 ## File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/impl/OpenSSHCertificateDecoder.java ## @@ -0,0 +1,115 @@ +/* + * 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.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.security.GeneralSecurityException; +import java.security.KeyFactory; +import java.security.KeyPairGenerator; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Objects; + +import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.keyprovider.KeyPairProvider; +import org.apache.sshd.common.session.SessionContext; +import org.apache.sshd.common.util.buffer.ByteArrayBuffer; +import org.apache.sshd.common.util.buffer.keys.OpenSSHCertPublicKeyParser; +import org.apache.sshd.common.util.io.IoUtils; + +/** + * @author mailto:dev@mina.apache.org;>Apache MINA SSHD Project + */ +public class OpenSSHCertificateDecoder extends AbstractPublicKeyEntryDecoder { +public static final OpenSSHCertificateDecoder INSTANCE = new OpenSSHCertificateDecoder(); + +public OpenSSHCertificateDecoder() { +super(OpenSshCertificate.class, OpenSshCertificate.class, +Collections.unmodifiableList(Arrays.asList( +KeyPairProvider.SSH_RSA_CERT, +KeyPairProvider.SSH_DSS_CERT, +KeyPairProvider.SSH_ED25519_CERT, +KeyPairProvider.SSH_ECDSA_SHA2_NISTP256_CERT, +KeyPairProvider.SSH_ECDSA_SHA2_NISTP384_CERT, +KeyPairProvider.SSH_ECDSA_SHA2_NISTP521_CERT +))); +} + +@Override +public OpenSshCertificate decodePublicKey( +SessionContext session, String keyType, InputStream keyData, Map headers) +throws IOException, GeneralSecurityException { + +byte[] bytes = IoUtils.toByteArray(keyData); + +ByteArrayBuffer buffer = new ByteArrayBuffer(bytes); + +OpenSshCertificate cert = (OpenSshCertificate) OpenSSHCertPublicKeyParser.INSTANCE.getRawPublicKey(keyType, buffer); + +if (cert.getType() != OpenSshCertificate.SSH_CERT_TYPE_HOST) { +throw new GeneralSecurityException("The provided certificate is not a Host certificate."); +} + +return cert; +} + +@Override +public String encodePublicKey(OutputStream s, OpenSshCertificate key) throws IOException { +Objects.requireNonNull(key, "No public key provided"); + +ByteArrayBuffer buffer = new ByteArrayBuffer(); +buffer.putRawPublicKeyBytes(key); +s.write(buffer.getCompactData()); + +return key.getKeyType(); +} + +@Override +public OpenSshCertificate clonePublicKey(OpenSshCertificate key) throws GeneralSecurityException { +try { +ByteArrayOutputStream outStream = new ByteArrayOutputStream(); +String keyType = encodePublicKey(outStream, key); +InputStream inStream = new ByteArrayInputStream(outStream.toByteArray()); +return decodePublicKey(null, keyType, inStream, null); +} catch (IOException e) { +throw new GeneralSecurityException("Unable to clone key.", e); +} +} Review comment: Even though we are dealing with byte-array streams that do no require closing, as a matter of coding style I prefer to have them in try-with-resource blocks: ```java try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { ... try (InputStream inStream = new ByteArrayInputStream(outStream.toByteArray())) { ... return ; } } ``` This is an automated message from the Apache Git Service. To respond to the message, please log on
[jira] [Commented] (SSHD-660) Add support for authentication using signed client/server keys
[ https://issues.apache.org/jira/browse/SSHD-660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084950#comment-17084950 ] FliegenKLATSCH commented on SSHD-660: - I did not check out the authentication yet, I am mostly interested in the host key algorithms. To setup this you need to create a CA: $ ssh-keygen -b 4096 -t rsa -f example-com-ca -C "CA key for example.com" and sign the public key: $ ssh-keygen -s example-com-ca -h -n host.example.com -V +52w -I host.example.com-key host-key.pub On the server, add the following line into the sshd_config (the certificate was created by the previous command with the -cert.pub suffix): HostCertificate /etc/ssh/ssh_host_rsa_key-cert.pub And on the client into the known_hosts: @cert-authority *.example.com If you connect with openssh client you can use -vvv to see which host key algorithm is used or enforce a specific one with -o HostKeyAlgorithms=ssh-rsa-cert-...@openssh.com > Add support for authentication using signed client/server keys > -- > > Key: SSHD-660 > URL: https://issues.apache.org/jira/browse/SSHD-660 > Project: MINA SSHD > Issue Type: Improvement >Reporter: Lyor Goldstein >Priority: Minor > > Similar to _HostCertificate_ and _TrustedUserCAKeys_ configuration values - > see https://ef.gy/hardening-ssh -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Commented] (DIRMINA-1123) Receive buffer size is never set for NIO acceptor
[ https://issues.apache.org/jira/browse/DIRMINA-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084902#comment-17084902 ] Jonathan Valliere commented on DIRMINA-1123: Merged into 2.1.X > Receive buffer size is never set for NIO acceptor > - > > Key: DIRMINA-1123 > URL: https://issues.apache.org/jira/browse/DIRMINA-1123 > Project: MINA > Issue Type: Bug > Components: Transport >Affects Versions: 2.0.22, 2.1.3 >Reporter: Marcin L >Assignee: Jonathan Valliere >Priority: Minor > Fix For: 2.1.4 > > Attachments: case dumps.png, case1.pcap, case2.pcap, case3.pcap > > > Acceptor window size can't be increased beyond OS defaults. > It seems the receive buffer size is properly set for > org.apache.mina.transport.socket.nio.NioSocketConnector, but it is not set at > all for org.apache.mina.transport.socket.nio.NioSocketAcceptor before socket > is bound. > org.apache.mina.core.polling.AbstractPollingIoAcceptor.Acceptor#registerHandles > comment states that receive buffer size should be initialised, but then > org.apache.mina.transport.socket.nio.NioSocketAcceptor#open does not do it., -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Commented] (DIRMINA-1123) Receive buffer size is never set for NIO acceptor
[ https://issues.apache.org/jira/browse/DIRMINA-1123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084821#comment-17084821 ] Marcin L commented on DIRMINA-1123: --- Thanks for the patch. It does work on Linux. Linux CongoJack 4.15.0-96-generic #97-Ubuntu SMP Wed Apr 1 03:25:46 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux 13:11:46.044102 IP localhost.39518 > localhost.12345: Flags [S], seq 138388208, win 65535, options [mss 65495,sackOK,TS val 845656780 ecr 0,nop,*wscale 2*], length 0 13:11:46.044114 IP localhost.12345 > localhost.39518: Flags [S.], seq 2264454457, ack 138388209, win 65535, options [mss 65495,sackOK,TS val 845656780 ecr 845656780,nop,*wscale 1*], length 0 > Receive buffer size is never set for NIO acceptor > - > > Key: DIRMINA-1123 > URL: https://issues.apache.org/jira/browse/DIRMINA-1123 > Project: MINA > Issue Type: Bug > Components: Transport >Affects Versions: 2.0.22, 2.1.3 >Reporter: Marcin L >Assignee: Jonathan Valliere >Priority: Minor > Fix For: 2.1.4 > > Attachments: case dumps.png, case1.pcap, case2.pcap, case3.pcap > > > Acceptor window size can't be increased beyond OS defaults. > It seems the receive buffer size is properly set for > org.apache.mina.transport.socket.nio.NioSocketConnector, but it is not set at > all for org.apache.mina.transport.socket.nio.NioSocketAcceptor before socket > is bound. > org.apache.mina.core.polling.AbstractPollingIoAcceptor.Acceptor#registerHandles > comment states that receive buffer size should be initialised, but then > org.apache.mina.transport.socket.nio.NioSocketAcceptor#open does not do it., -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Commented] (DIRMINA-1079) MINA fails to connect from behind a proxy if endpoint is not resolved
[ https://issues.apache.org/jira/browse/DIRMINA-1079?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084726#comment-17084726 ] Emmanuel Lécharny commented on DIRMINA-1079: No, not yet. Under the water atm :/ > MINA fails to connect from behind a proxy if endpoint is not resolved > - > > Key: DIRMINA-1079 > URL: https://issues.apache.org/jira/browse/DIRMINA-1079 > Project: MINA > Issue Type: Bug > Components: Handler >Affects Versions: 2.0.16 >Reporter: Anton Novikov >Assignee: Jonathan Valliere >Priority: Major > Attachments: DIRMINA-1079.patch > > > MINA fails to connect from behind a proxy if endpoint address is not > resolved. This happens for both HTTP and SOCKS proxy and it seems that the > reason for this are typos in HttpProxyRequest and SocksProxyRequest. The > following changes seem to fix the issue: > HttpProxyRequest line 105: replace {{if (!endpointAddress.isUnresolved()) {}} > with {{if (endpointAddress.isUnresolved()) {}} > SocksProxyRequest line 162: replace {{if (adr != null && !adr.isUnresolved()) > {}} with {{if (adr != null && adr.isUnresolved()) {}} > Note the negation is gone in both cases -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Resolved] (SSHD-977) Apply consistent logging policy to caught exceptions
[ https://issues.apache.org/jira/browse/SSHD-977?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Lyor Goldstein resolved SSHD-977. - Fix Version/s: 2.4.1 Resolution: Fixed > Apply consistent logging policy to caught exceptions > > > Key: SSHD-977 > URL: https://issues.apache.org/jira/browse/SSHD-977 > Project: MINA SSHD > Issue Type: Improvement >Reporter: Lyor Goldstein >Assignee: Lyor Goldstein >Priority: Major > Fix For: 2.4.1 > > > See [https://github.com/apache/mina-sshd/pull/120] as an example: > * Log at ERROR/WARNING level the basic exception details (class + message) > * If DEBUG enabled then also log at ERROR/WARNING level the actual stack trace -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Comment Edited] (SSHD-660) Add support for authentication using signed client/server keys
[ https://issues.apache.org/jira/browse/SSHD-660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084583#comment-17084583 ] Lyor Goldstein edited comment on SSHD-660 at 4/16/20, 7:08 AM: --- {quote} For the unit tests I don't really know how and what is best to test.. Are there some integration tests which I overlook? Would be the easiest to spawn a server and let a client ping? {quote} For unit testing it is enough to spawn a server and let a client ping - there are plenty of examples in the current code how to do this. {quote} I tested it against `OpenSSH_7.9p1 Debian-10+deb10u2, OpenSSL 1.1.1d 10 Sep 2019` (default Debian Buster) and with `OpenSSH_8.1p1, LibreSSL 2.7.3` (default macOS) client. {quote} Can you provide some reference documentation how to set up these servers to execute this type of authentication ? was (Author: lgoldstein): {quote} For the unit tests I don't really know how and what is best to test.. Are there some integration tests which I overlook? Would be the easiest to spawn a server and let a client ping? {quote} For unit testing it is enough to spawn a server and let a client ping - there are plenty of examples in the current code how to do this. {quote} I tested it against `OpenSSH_7.9p1 Debian-10+deb10u2, OpenSSL 1.1.1d 10 Sep 2019` (default Debian Buster) and with `OpenSSH_8.1p1, LibreSSL 2.7.3` (default macOS) client. {quote} Can you provide some reference documentation how to set up these server to execute this type of authentication ? > Add support for authentication using signed client/server keys > -- > > Key: SSHD-660 > URL: https://issues.apache.org/jira/browse/SSHD-660 > Project: MINA SSHD > Issue Type: Improvement >Reporter: Lyor Goldstein >Priority: Minor > > Similar to _HostCertificate_ and _TrustedUserCAKeys_ configuration values - > see https://ef.gy/hardening-ssh -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409327626 ## File path: sshd-core/src/main/java/org/apache/sshd/server/session/AbstractServerSession.java ## @@ -367,9 +382,26 @@ protected String resolveAvailableSignaturesProposal(FactoryManager proposedManag KeyPairProvider kpp = getKeyPairProvider(); boolean debugEnabled = log.isDebugEnabled(); -Iterable provided; +Set provided = null; try { -provided = (kpp == null) ? null : kpp.getKeyTypes(this); +if (kpp != null) { +provided = StreamSupport.stream(kpp.getKeyTypes(this).spliterator(), false) +.collect(Collectors.toSet()); + +HostKeyCertificateProvider hostKeyCertificateProvider = getHostKeyCertificateProvider(); +if (hostKeyCertificateProvider != null) { +Iterable certificates = hostKeyCertificateProvider.loadCertificates(this); +for (OpenSshCertificate certificate : certificates) { +// Add the certificate alg only if the corresponding keyPair type is available +if (provided.contains(certificate.getRawKeyType())) { +provided.add(certificate.getKeyType()); +} else { Review comment: This code interferes with a lazy-load feature (see SSHD-860) that we implemented since it consumes all available key types thus defeating the purpose of the lazy-load. We need to figure out a way to preserve that capability 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409323422 ## File path: sshd-common/src/main/java/org/apache/sshd/common/util/buffer/keys/OpenSSHCertPublicKeyParser.java ## @@ -0,0 +1,104 @@ +/* + * 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.Arrays; +import java.util.Collection; +import java.util.List; + +import org.apache.sshd.common.SshException; +import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.keyprovider.KeyPairProvider; +import org.apache.sshd.common.util.buffer.Buffer; +import org.apache.sshd.common.util.buffer.ByteArrayBuffer; + +public class OpenSSHCertPublicKeyParser extends AbstractBufferPublicKeyParser { + +public static final OpenSSHCertPublicKeyParser INSTANCE = new OpenSSHCertPublicKeyParser(PublicKey.class, +Arrays.asList( +KeyPairProvider.SSH_RSA_CERT, +KeyPairProvider.SSH_DSS_CERT, +KeyPairProvider.SSH_ECDSA_SHA2_NISTP256_CERT, +KeyPairProvider.SSH_ECDSA_SHA2_NISTP384_CERT, +KeyPairProvider.SSH_ECDSA_SHA2_NISTP521_CERT, +KeyPairProvider.SSH_ED25519_CERT +)); + +public OpenSSHCertPublicKeyParser(Class keyClass, Collection supported) { +super(keyClass, supported); +} + +@Override +public PublicKey getRawPublicKey(String keyType, Buffer buffer) throws GeneralSecurityException { + +byte[] nonce = buffer.getBytes(); + +String rawKeyType = OpenSshCertificate.getRawKeyType(keyType); +PublicKey publicKey = DEFAULT.getRawPublicKey(rawKeyType, buffer); + +long serial = buffer.getLong(); +int userOrHostType = buffer.getInt(); + +String id = buffer.getString(); + +List vPrincipals = new ByteArrayBuffer(buffer.getBytes()).getNameList(); +long vAfter = buffer.getLong(); +long vBefore = buffer.getLong(); + +List criticalOptions = buffer.getNameList(); +List extensions = buffer.getNameList(); + +String reserved = buffer.getString(); + +PublicKey signatureKey; +try { +signatureKey = buffer.getPublicKey(); +} catch (SshException ex) { +throw new GeneralSecurityException("Could not parse public CA key.", ex); +} + +byte[] message = buffer.getBytesConsumed(); +byte[] signature = buffer.getBytes(); + +if (buffer.rpos() != buffer.wpos()) { +throw new GeneralSecurityException("KeyExchange signature verification failed, got more data than expected: " Review comment: Let's add the ID of the key to the exception message - for debugging purposes 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409321614 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/KeyPairProvider.java ## @@ -175,7 +185,7 @@ public KeyPair loadKey(SessionContext session, String type) { @Override public Iterable getKeyTypes(SessionContext session) { // use a LinkedHashSet so as to preserve the order but avoid duplicates -Collection types = new LinkedHashSet<>(); +Set types = new LinkedHashSet<>(); Review comment: Please revert this type to a `Collection` as the original code... 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409321129 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/FileHostKeyCertificateProvider.java ## @@ -0,0 +1,88 @@ +/* + * 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.keyprovider; + +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.security.GeneralSecurityException; +import java.security.PublicKey; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.stream.StreamSupport; + +import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.config.keys.PublicKeyEntry; +import org.apache.sshd.common.session.SessionContext; +import org.apache.sshd.common.util.ValidateUtils; +import org.apache.sshd.common.util.io.IoUtils; +import org.apache.sshd.common.util.logging.AbstractLoggingBean; + +public class FileHostKeyCertificateProvider extends AbstractLoggingBean implements HostKeyCertificateProvider { +private Collection files; + +public FileHostKeyCertificateProvider() { +super(); +} + +public FileHostKeyCertificateProvider(Path path) { +this(Collections.singletonList(Objects.requireNonNull(path, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Path... files) { +this(Arrays.asList(ValidateUtils.checkNotNullAndNotEmpty(files, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Collection files) { +this.files = files; +} + +public Collection getPaths() { +return files; +} + +@Override +public Iterable loadCertificates(SessionContext session) throws IOException, GeneralSecurityException { + +List certificates = new ArrayList<>(); +for (Path file : files) { +List lines = IoUtils.readAllLines(new FileInputStream(file.toFile())); +for (String line : lines) { +PublicKeyEntry publicKeyEntry = PublicKeyEntry.parsePublicKeyEntry(line); + +PublicKey publicKey = publicKeyEntry.resolvePublicKey(session, null, null); + +certificates.add((OpenSshCertificate) publicKey); Review comment: Let's check that it is indeed an `OpenSshCertificate` rather than risk `ClassCastException`: ```java PublicKey publicKey = . if (publicKey == null) { continue; } if (!(publicKey instanceof OpenSshCertificate)) { throw new InvalidKeyException("not and OpenSshCertificate..."); } ``` 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409320966 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/FileHostKeyCertificateProvider.java ## @@ -0,0 +1,88 @@ +/* + * 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.keyprovider; + +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.security.GeneralSecurityException; +import java.security.PublicKey; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.stream.StreamSupport; + +import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.config.keys.PublicKeyEntry; +import org.apache.sshd.common.session.SessionContext; +import org.apache.sshd.common.util.ValidateUtils; +import org.apache.sshd.common.util.io.IoUtils; +import org.apache.sshd.common.util.logging.AbstractLoggingBean; + +public class FileHostKeyCertificateProvider extends AbstractLoggingBean implements HostKeyCertificateProvider { +private Collection files; + +public FileHostKeyCertificateProvider() { +super(); +} + +public FileHostKeyCertificateProvider(Path path) { +this(Collections.singletonList(Objects.requireNonNull(path, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Path... files) { +this(Arrays.asList(ValidateUtils.checkNotNullAndNotEmpty(files, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Collection files) { +this.files = files; +} + +public Collection getPaths() { +return files; +} + +@Override +public Iterable loadCertificates(SessionContext session) throws IOException, GeneralSecurityException { + +List certificates = new ArrayList<>(); +for (Path file : files) { +List lines = IoUtils.readAllLines(new FileInputStream(file.toFile())); Review comment: Please use try-with-resource on the input stream - this code opens a stream but does not close it. Better yet - use `Files.readAllLines(file, StandardCharset.UTF8)` 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409321005 ## File path: sshd-common/src/main/java/org/apache/sshd/common/keyprovider/FileHostKeyCertificateProvider.java ## @@ -0,0 +1,88 @@ +/* + * 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.keyprovider; + +import java.io.FileInputStream; +import java.io.IOException; +import java.nio.file.Path; +import java.security.GeneralSecurityException; +import java.security.PublicKey; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.stream.StreamSupport; + +import org.apache.sshd.common.config.keys.OpenSshCertificate; +import org.apache.sshd.common.config.keys.PublicKeyEntry; +import org.apache.sshd.common.session.SessionContext; +import org.apache.sshd.common.util.ValidateUtils; +import org.apache.sshd.common.util.io.IoUtils; +import org.apache.sshd.common.util.logging.AbstractLoggingBean; + +public class FileHostKeyCertificateProvider extends AbstractLoggingBean implements HostKeyCertificateProvider { +private Collection files; + +public FileHostKeyCertificateProvider() { +super(); +} + +public FileHostKeyCertificateProvider(Path path) { +this(Collections.singletonList(Objects.requireNonNull(path, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Path... files) { +this(Arrays.asList(ValidateUtils.checkNotNullAndNotEmpty(files, "No path provided"))); +} + +public FileHostKeyCertificateProvider(Collection files) { +this.files = files; +} + +public Collection getPaths() { +return files; +} + +@Override +public Iterable loadCertificates(SessionContext session) throws IOException, GeneralSecurityException { + +List certificates = new ArrayList<>(); +for (Path file : files) { +List lines = IoUtils.readAllLines(new FileInputStream(file.toFile())); +for (String line : lines) { +PublicKeyEntry publicKeyEntry = PublicKeyEntry.parsePublicKeyEntry(line); Review comment: What if the line is empty ? What if the line is a comment ? 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409318128 ## File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java ## @@ -0,0 +1,262 @@ +/* + * 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; + +import java.security.PrivateKey; +import java.security.PublicKey; +import java.util.List; + +public final class OpenSshCertificate implements PublicKey, PrivateKey { +public static final int SSH_CERT_TYPE_USER = 1; +public static final int SSH_CERT_TYPE_HOST = 2; + +private static final long serialVersionUID = -3592634724148744943L; + +private String keyType; +private byte[] nonce; +private PublicKey serverHostKey; +private long serial; +private int type; +private String id; +private List principals; +private long validAfter; +private long validBefore; +private List criticalOptions; +private List extensions; +private String reserved; +private PublicKey caPubKey; +private byte[] message; +private byte[] signature; + +private byte[] rawData; + +private OpenSshCertificate() { +} + +public static String getRawKeyType(String keyType) { +return keyType.split("@")[0].substring(0, keyType.indexOf("-cert")); +} + +public String getRawKeyType() { +return getRawKeyType(keyType); +} + +public byte[] getNonce() { +return nonce; +} + +public String getKeyType() { +return keyType; +} + +public PublicKey getServerHostKey() { +return serverHostKey; +} + +public long getSerial() { +return serial; +} + +public int getType() { +return type; +} + +public String getId() { +return id; +} + +public List getPrincipals() { +return principals; +} + +public long getValidAfter() { +return validAfter; +} + +public long getValidBefore() { +return validBefore; +} + +public List getCriticalOptions() { +return criticalOptions; +} + +public List getExtensions() { +return extensions; +} + +public String getReserved() { +return reserved; +} + +public PublicKey getCaPubKey() { +return caPubKey; +} + +public byte[] getMessage() { +return message; +} + +public byte[] getSignature() { +return signature; +} + +public void setRawData(byte[] rawData) { +this.rawData = rawData; +} + +public byte[] getRawData() { +return rawData; +} + +@Override +public String getAlgorithm() { +return null; +} + +@Override +public String getFormat() { +return null; +} + +@Override +public byte[] getEncoded() { +return new byte[0]; +} + +public static final class OpenSshPublicKeyBuilder { +private String keyType; +private byte[] nonce; +private PublicKey serverHostKey; +private long serial; +private int type; +private String id; +private List principals; +private long validAfter; +private long validBefore; +private List criticalOptions; +private List extensions; +private String reserved; +private PublicKey caPubKey; +private byte[] message; +private byte[] signature; + +private OpenSshPublicKeyBuilder() { +} + +public static OpenSshPublicKeyBuilder anOpenSshCertificate() { +return new OpenSshPublicKeyBuilder(); +} + +public OpenSshPublicKeyBuilder withKeyType(String keyType) { +this.keyType = keyType; +return this; +} + +public OpenSshPublicKeyBuilder withNonce(byte[] nonce) { +this.nonce = nonce; +return this; +} + +public OpenSshPublicKeyBuilder withServerHostPublicKey(PublicKey serverHostKey) { +this.serverHostKey = serverHostKey; +return this; +} + +public
[GitHub] [mina-sshd] lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates
lgoldstein commented on a change in pull request #119: Add support for openssh host key certificates URL: https://github.com/apache/mina-sshd/pull/119#discussion_r409317862 ## File path: sshd-common/src/main/java/org/apache/sshd/common/config/keys/OpenSshCertificate.java ## @@ -0,0 +1,262 @@ +/* + * 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; + +import java.security.PrivateKey; +import java.security.PublicKey; +import java.util.List; + +public final class OpenSshCertificate implements PublicKey, PrivateKey { Review comment: That's because the only way to build it is via the builder. If you replace the builder with getters and setters you will no longer have this problem. 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Comment Edited] (SSHD-660) Add support for authentication using signed client/server keys
[ https://issues.apache.org/jira/browse/SSHD-660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084583#comment-17084583 ] Lyor Goldstein edited comment on SSHD-660 at 4/16/20, 6:42 AM: --- {quote} For the unit tests I don't really know how and what is best to test.. Are there some integration tests which I overlook? Would be the easiest to spawn a server and let a client ping? {quote} For unit testing it is enough to spawn a server and let a client ping - there are plenty of examples in the current code how to do this. {quote} I tested it against `OpenSSH_7.9p1 Debian-10+deb10u2, OpenSSL 1.1.1d 10 Sep 2019` (default Debian Buster) and with `OpenSSH_8.1p1, LibreSSL 2.7.3` (default macOS) client. {quote} Can you provide some reference documentation how to set up these server to execute this type of authentication ? was (Author: lgoldstein): {quote} For the unit tests I don't really know how and what is best to test.. Are there some integration tests which I overlook? Would be the easiest to spawn a server and let a client ping? {quote} For unit testing it is enough to spawn a server and let a client ping - there are plenty of examples. {quote} I tested it against `OpenSSH_7.9p1 Debian-10+deb10u2, OpenSSL 1.1.1d 10 Sep 2019` (default Debian Buster) and with `OpenSSH_8.1p1, LibreSSL 2.7.3` (default macOS) client. {quote} Can you provide some reference documentation how to set up these server to execute this type of authentication ? > Add support for authentication using signed client/server keys > -- > > Key: SSHD-660 > URL: https://issues.apache.org/jira/browse/SSHD-660 > Project: MINA SSHD > Issue Type: Improvement >Reporter: Lyor Goldstein >Priority: Minor > > Similar to _HostCertificate_ and _TrustedUserCAKeys_ configuration values - > see https://ef.gy/hardening-ssh -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
[jira] [Commented] (SSHD-660) Add support for authentication using signed client/server keys
[ https://issues.apache.org/jira/browse/SSHD-660?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17084583#comment-17084583 ] Lyor Goldstein commented on SSHD-660: - {quote} For the unit tests I don't really know how and what is best to test.. Are there some integration tests which I overlook? Would be the easiest to spawn a server and let a client ping? {quote} For unit testing it is enough to spawn a server and let a client ping - there are plenty of examples. {quote} I tested it against `OpenSSH_7.9p1 Debian-10+deb10u2, OpenSSL 1.1.1d 10 Sep 2019` (default Debian Buster) and with `OpenSSH_8.1p1, LibreSSL 2.7.3` (default macOS) client. {quote} Can you provide some reference documentation how to set up these server to execute this type of authentication ? > Add support for authentication using signed client/server keys > -- > > Key: SSHD-660 > URL: https://issues.apache.org/jira/browse/SSHD-660 > Project: MINA SSHD > Issue Type: Improvement >Reporter: Lyor Goldstein >Priority: Minor > > Similar to _HostCertificate_ and _TrustedUserCAKeys_ configuration values - > see https://ef.gy/hardening-ssh -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org