[jira] [Resolved] (FTPSERVER-487) Timing Side Channel SaltedPasswordEncryptor.encrypt(String password, String salt)
[ https://issues.apache.org/jira/browse/FTPSERVER-487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jonathan Valliere resolved FTPSERVER-487. - Resolution: Fixed Resolved in FTPSERVER-485 > Timing Side Channel SaltedPasswordEncryptor.encrypt(String password, String > salt) > - > > Key: FTPSERVER-487 > URL: https://issues.apache.org/jira/browse/FTPSERVER-487 > Project: FtpServer > Issue Type: Bug > Components: Core >Affects Versions: 1.1.1 > Environment: tested on macOS High Sierra 10.13.4, but it is not > relevant >Reporter: Yannic Noller >Priority: Major > Labels: security > > Dear Apache FTPServer developers, > We have found a timing side-channel in class > org.apache.ftpserver.usermanager.SaltedPasswordEncryptor, method "private > String encrypt(String password, String salt)". This encryption method leaks > information about the salt. The processing time in this method differs for > different salt values. Therefore, a potential attacker could retrieve > information about the generated salt, which is imporant to guess the stored > password. > Do you agree with our findings? > We identified this side-channel after fixing the one mentioned in: > [FTPSERVER-485|https://issues.apache.org/jira/browse/FTPSERVER-485] > Please feel free to contact us for further clarification! You can reach us by > the following email address: yannic.nol...@informatik.hu-berlin.de > Best regards, > Yannic Noller -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FTPSERVER-487) Timing Side Channel SaltedPasswordEncryptor.encrypt(String password, String salt)
[ https://issues.apache.org/jira/browse/FTPSERVER-487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16684526#comment-16684526 ] Yannic Noller commented on FTPSERVER-487: - According to the comments in https://issues.apache.org/jira/browse/FTPSERVER-485 the issue here seems to be solved as well. Can you check and update this issue? -- Yannic > Timing Side Channel SaltedPasswordEncryptor.encrypt(String password, String > salt) > - > > Key: FTPSERVER-487 > URL: https://issues.apache.org/jira/browse/FTPSERVER-487 > Project: FtpServer > Issue Type: Bug > Components: Core >Affects Versions: 1.1.1 > Environment: tested on macOS High Sierra 10.13.4, but it is not > relevant >Reporter: Yannic Noller >Priority: Major > Labels: security > > Dear Apache FTPServer developers, > We have found a timing side-channel in class > org.apache.ftpserver.usermanager.SaltedPasswordEncryptor, method "private > String encrypt(String password, String salt)". This encryption method leaks > information about the salt. The processing time in this method differs for > different salt values. Therefore, a potential attacker could retrieve > information about the generated salt, which is imporant to guess the stored > password. > Do you agree with our findings? > We identified this side-channel after fixing the one mentioned in: > [FTPSERVER-485|https://issues.apache.org/jira/browse/FTPSERVER-485] > Please feel free to contact us for further clarification! You can reach us by > the following email address: yannic.nol...@informatik.hu-berlin.de > Best regards, > Yannic Noller -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (FTPSERVER-486) Timing Side Channel StringUtils
[
https://issues.apache.org/jira/browse/FTPSERVER-486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16684521#comment-16684521
]
Yannic Noller commented on FTPSERVER-486:
-
What is the current status of the issue?
-- Yannic
> Timing Side Channel StringUtils
> ---
>
> Key: FTPSERVER-486
> URL: https://issues.apache.org/jira/browse/FTPSERVER-486
> Project: FtpServer
> Issue Type: Bug
> Components: Core
>Affects Versions: 1.1.1
> Environment: test on macOS High Sierra 10.13.4, but not relevant
>Reporter: Yannic Noller
>Assignee: Emmanuel Lecharny
>Priority: Major
> Labels: easyfix, pull-request-available
> Fix For: 1.1.2
>
> Original Estimate: 24h
> Remaining Estimate: 24h
>
> Dear Apache FTPServer developers,
> We have found a timing side-channel in class
> org.apache.ftpserver.util.StringUtils, method "public final static String
> pad(String src, char padChar, boolean rightPad, int totalLength)". This
> method leaks the necessary padding in a timing side channel, from which a
> potential attacker could obtain the length of the src String. In your project
> this method is used to add padding to a username, hence, a potential attacker
> could obtain the length of a given username, which might be used for further
> attacks.
> Do you agree with our findings?
> We found this class in the latest version of your git repo:
> https://git-wip-us.apache.org/repos/asf?p=mina-ftpserver.git;a=summary
> As a secure fix we would recommend to use a variant of the equals method,
> which does iterate the complete strings in the case of the same string
> lengths, independent from whether they do match or not:
>public final static String pad_safe(String src, char padChar, boolean
> rightPad, int totalLength) {
>int srcLength = src.length();
>if (srcLength >= totalLength) {
>return src;
>}
>int padLength = totalLength - srcLength;
>StringBuilder sb = new StringBuilder(padLength);
>for (int i = 0; i < totalLength; ++i) {
>if (i < padLength) {
>sb.append(padChar);
>} else {
>sb.append("");
>}
>}
>if (rightPad) {
>return src + sb.toString();
>} else {
>return sb.toString() + src;
>}
>}
> Do you agree with our patch proposal?
> Please feel free to contact us for further clarification! You can reach us by
> the following email address:
> yannic.nol...@informatik.hu-berlin.de
> Best regards,
> Yannic Noller
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SSHD-860) org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client identities (private key files)
[
https://issues.apache.org/jira/browse/SSHD-860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16684259#comment-16684259
]
Goldstein Lyor commented on SSHD-860:
-
Great - the fix will be released as part of 2.2 (or higher) - I believe some
time in Q1 2019
> org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client
> identities (private key files)
>
>
> Key: SSHD-860
> URL: https://issues.apache.org/jira/browse/SSHD-860
> Project: MINA SSHD
> Issue Type: Bug
>Affects Versions: 2.0.0, 2.1.0, 2.1.1
>Reporter: Thomas Wolf
>Assignee: Goldstein Lyor
>Priority: Major
> Fix For: 2.1.1
>
>
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} loads and
> caches in memory private keys prematurely. Keys are loaded even if they are
> not used at all in the end. In other words, incremental loading of keys
> doesn't work.
> This is bad for two reasons:
> # Private keys should be kept in memory only if and when needed. Loading
> completely unused private keys must be avoided.
> # With encrypted private keys, the user may end up being asked for
> passphrases for keys that are not used at all in the end, which is highly
> disruptive.
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} does in its
> constructor:
> {code:java}
> Iterator current;
> Collection> identities = new
> LinkedList<>();
> ...
> identities.add(Stream.of(ClientSession.providerOf(session))
> .map(KeyIdentityProvider::loadKeys)
> .flatMap(GenericUtils::stream)
> .map(kp -> new KeyPairIdentity(signatureFactories, session, kp)));
> current = identities.stream().flatMap(r -> r).iterator();
> {code}
> The final {{current}} iterator uses some internal buffer (size unknown;
> didn't try to determine it) and will pre-fill this buffer completely. So with
> buffer size _N_ it'll pre-load the first _N_ keys from the combined identity
> stream. If the first key authenticates successfully, loading all the others
> must not be done.
> See my [test
> case|https://github.com/tomaswolf/mina-sshd/blob/SSHD-860/sshd-core/src/test/java/org/apache/sshd/client/ClientKeyLoadTest.java]
> showing this faulty behavior. It does exactly the same as the
> {{UserAuthPublicKeyIterator}} constructor, using two iterators with two
> identity files each, and then tests the resulting iterator. The first
> {{hasNext()/next()}} call on the {{current}} iterator _loads all four keys!_
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SSHD-860) org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client identities (private key files)
[
https://issues.apache.org/jira/browse/SSHD-860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16684218#comment-16684218
]
Thomas Wolf commented on SSHD-860:
--
Thank you; that did it. My test, adapted to current master, now passes, and
keys are indeed loaded one by one.
> org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client
> identities (private key files)
>
>
> Key: SSHD-860
> URL: https://issues.apache.org/jira/browse/SSHD-860
> Project: MINA SSHD
> Issue Type: Bug
>Affects Versions: 2.0.0, 2.1.0, 2.1.1
>Reporter: Thomas Wolf
>Assignee: Goldstein Lyor
>Priority: Major
> Fix For: 2.1.1
>
>
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} loads and
> caches in memory private keys prematurely. Keys are loaded even if they are
> not used at all in the end. In other words, incremental loading of keys
> doesn't work.
> This is bad for two reasons:
> # Private keys should be kept in memory only if and when needed. Loading
> completely unused private keys must be avoided.
> # With encrypted private keys, the user may end up being asked for
> passphrases for keys that are not used at all in the end, which is highly
> disruptive.
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} does in its
> constructor:
> {code:java}
> Iterator current;
> Collection> identities = new
> LinkedList<>();
> ...
> identities.add(Stream.of(ClientSession.providerOf(session))
> .map(KeyIdentityProvider::loadKeys)
> .flatMap(GenericUtils::stream)
> .map(kp -> new KeyPairIdentity(signatureFactories, session, kp)));
> current = identities.stream().flatMap(r -> r).iterator();
> {code}
> The final {{current}} iterator uses some internal buffer (size unknown;
> didn't try to determine it) and will pre-fill this buffer completely. So with
> buffer size _N_ it'll pre-load the first _N_ keys from the combined identity
> stream. If the first key authenticates successfully, loading all the others
> must not be done.
> See my [test
> case|https://github.com/tomaswolf/mina-sshd/blob/SSHD-860/sshd-core/src/test/java/org/apache/sshd/client/ClientKeyLoadTest.java]
> showing this faulty behavior. It does exactly the same as the
> {{UserAuthPublicKeyIterator}} constructor, using two iterators with two
> identity files each, and then tests the resulting iterator. The first
> {{hasNext()/next()}} call on the {{current}} iterator _loads all four keys!_
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Resolved] (SSHD-860) org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client identities (private key files)
[
https://issues.apache.org/jira/browse/SSHD-860?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Goldstein Lyor resolved SSHD-860.
-
Resolution: Fixed
> org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client
> identities (private key files)
>
>
> Key: SSHD-860
> URL: https://issues.apache.org/jira/browse/SSHD-860
> Project: MINA SSHD
> Issue Type: Bug
>Affects Versions: 2.0.0, 2.1.0, 2.1.1
>Reporter: Thomas Wolf
>Assignee: Goldstein Lyor
>Priority: Major
> Fix For: 2.1.1
>
>
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} loads and
> caches in memory private keys prematurely. Keys are loaded even if they are
> not used at all in the end. In other words, incremental loading of keys
> doesn't work.
> This is bad for two reasons:
> # Private keys should be kept in memory only if and when needed. Loading
> completely unused private keys must be avoided.
> # With encrypted private keys, the user may end up being asked for
> passphrases for keys that are not used at all in the end, which is highly
> disruptive.
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} does in its
> constructor:
> {code:java}
> Iterator current;
> Collection> identities = new
> LinkedList<>();
> ...
> identities.add(Stream.of(ClientSession.providerOf(session))
> .map(KeyIdentityProvider::loadKeys)
> .flatMap(GenericUtils::stream)
> .map(kp -> new KeyPairIdentity(signatureFactories, session, kp)));
> current = identities.stream().flatMap(r -> r).iterator();
> {code}
> The final {{current}} iterator uses some internal buffer (size unknown;
> didn't try to determine it) and will pre-fill this buffer completely. So with
> buffer size _N_ it'll pre-load the first _N_ keys from the combined identity
> stream. If the first key authenticates successfully, loading all the others
> must not be done.
> See my [test
> case|https://github.com/tomaswolf/mina-sshd/blob/SSHD-860/sshd-core/src/test/java/org/apache/sshd/client/ClientKeyLoadTest.java]
> showing this faulty behavior. It does exactly the same as the
> {{UserAuthPublicKeyIterator}} constructor, using two iterators with two
> identity files each, and then tests the resulting iterator. The first
> {{hasNext()/next()}} call on the {{current}} iterator _loads all four keys!_
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (SSHD-860) org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client identities (private key files)
[
https://issues.apache.org/jira/browse/SSHD-860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16684161#comment-16684161
]
Goldstein Lyor commented on SSHD-860:
-
Hi [~wolft], I have "un-fangled" the code you indicated and pushed the changes
to the _master_ branch. I would like to ask you to check that it indeed fixes
the issue (or at least the code you mentioned). Please do re-open the issue if
you find more "fangled" code...
> org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client
> identities (private key files)
>
>
> Key: SSHD-860
> URL: https://issues.apache.org/jira/browse/SSHD-860
> Project: MINA SSHD
> Issue Type: Bug
>Affects Versions: 2.0.0, 2.1.0, 2.1.1
>Reporter: Thomas Wolf
>Assignee: Goldstein Lyor
>Priority: Major
> Fix For: 2.1.1
>
>
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} loads and
> caches in memory private keys prematurely. Keys are loaded even if they are
> not used at all in the end. In other words, incremental loading of keys
> doesn't work.
> This is bad for two reasons:
> # Private keys should be kept in memory only if and when needed. Loading
> completely unused private keys must be avoided.
> # With encrypted private keys, the user may end up being asked for
> passphrases for keys that are not used at all in the end, which is highly
> disruptive.
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} does in its
> constructor:
> {code:java}
> Iterator current;
> Collection> identities = new
> LinkedList<>();
> ...
> identities.add(Stream.of(ClientSession.providerOf(session))
> .map(KeyIdentityProvider::loadKeys)
> .flatMap(GenericUtils::stream)
> .map(kp -> new KeyPairIdentity(signatureFactories, session, kp)));
> current = identities.stream().flatMap(r -> r).iterator();
> {code}
> The final {{current}} iterator uses some internal buffer (size unknown;
> didn't try to determine it) and will pre-fill this buffer completely. So with
> buffer size _N_ it'll pre-load the first _N_ keys from the combined identity
> stream. If the first key authenticates successfully, loading all the others
> must not be done.
> See my [test
> case|https://github.com/tomaswolf/mina-sshd/blob/SSHD-860/sshd-core/src/test/java/org/apache/sshd/client/ClientKeyLoadTest.java]
> showing this faulty behavior. It does exactly the same as the
> {{UserAuthPublicKeyIterator}} constructor, using two iterators with two
> identity files each, and then tests the resulting iterator. The first
> {{hasNext()/next()}} call on the {{current}} iterator _loads all four keys!_
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
