Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make KeyType compatible with Android Keystore #586

Merged
merged 5 commits into from
May 28, 2020
Merged

Make KeyType compatible with Android Keystore #586

merged 5 commits into from
May 28, 2020

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented May 26, 2020

Android Keystore private keys do not implement PrivateKey since the
raw key material is not available to applications.

With this commit, sshj's KeyType correctly detects the algorithm
associated with Android Keystore keys, which makes them usable for SSH
authentication.

Android Keystore private keys do not implement PrivateKey since the
raw key material is not available to applications.

With this commit, sshj's KeyType correctly detects the algorithm
associated with Android Keystore keys, which makes them usable for SSH
authentication.
@hierynomus
Copy link
Owner

I've merged master into this branch to fix the builds.

@fmeum fmeum closed this May 28, 2020
@fmeum fmeum reopened this May 28, 2020
@fmeum
Copy link
Contributor Author

fmeum commented May 28, 2020

I closed and re-opened to see whether this would trigger travis. Unfortunately, it didn't.

@hierynomus
Copy link
Owner

I closed and re-opened to see whether this would trigger travis. Unfortunately, it didn't.

I still need to remove travis :) Now with GH Actions travis is no longer needed...

@hierynomus hierynomus merged commit 4e802ce into hierynomus:master May 28, 2020
@fmeum fmeum deleted the fhenneke_aks branch May 28, 2020 14:10
@fmeum
Copy link
Contributor Author

fmeum commented May 28, 2020

Thanks! Are you able to provide an estimate fow when 0.30.0 will be released?

@oakkitten
Copy link

@FabianHenneke could you perhaps show how one could put ssh keys into Android Keystore please? that's something i could use in my app

btw 0.30.0 was released

@fmeum
Copy link
Contributor Author

fmeum commented Aug 21, 2020

@FabianHenneke could you perhaps show how one could put ssh keys into Android Keystore please? that's something i could use in my app

It's actually quite simple, you just need to implement a KeyProvider that returns the Android Keystore public/private key. I have implemented one here that you could base your implementation on: https://github.com/android-password-store/Android-Password-Store/pull/807/files#diff-aed5d545b55b63eecec85dead79797ffR103

@oakkitten
Copy link

thanks!

@oakkitten
Copy link

i can't get this to work.

my device is running Android 10. in gradle i'm using

api 'com.hierynomus:sshj:0.30.0'
implementation 'org.bouncycastle:bcpkix-jdk15on:1.65'

i tried "importing" BC in a variety of ways, and stuff below are best case scenarios

  • first, i can't import any keys into Android Key Store (AKS).

    this KeyPair created by sshlib is importable into AKS:

    KeyPair kp = PEMDecoder.decode(charKey, sshPassword);

    this KeyPair created by sshj, while usable in authPublickey(), is not:

    KeyFormat keyFormat = KeyProviderUtil.detectKeyFileFormat(
            new InputStreamReader(new ByteArrayInputStream(sshKey)), false);
    final FileKeyProvider fileKeyProvider = Factory.Named.Util.create(
            new DefaultConfig().getFileKeyProviderFactories(), keyFormat.toString());
    if (fileKeyProvider == null)
        throw new SSHException("No provider available for " + keyFormat + " key file");
    fileKeyProvider.init(new InputStreamReader(new ByteArrayInputStream(sshKey)),
            PasswordUtils.createOneOff(sshPassword.toCharArray()));
    KeyPair kp = new KeyPair(fileKeyProvider.getPublic(), fileKeyProvider.getPrivate());

    public keys are the same in both cases, but sshlib creates RSA Private CRT Key instead of RSA Private Key. when creating a certificate to import this into AKS, i'm getting:

    PrivateKeyPickerPreference: Error while putting "RSA" key into AndroidKeyStore
        java.lang.ArithmeticException: BigInteger division by zero
            at java.math.NativeBN.BN_div(Native Method)
            at java.math.BigInt.division(BigInt.java:312)
            at java.math.BigInteger.remainder(BigInteger.java:955)
            at org.bouncycastle.crypto.engines.RSACoreEngine.processBlock(Unknown Source:28)
            at org.bouncycastle.crypto.engines.RSABlindedEngine.processBlock(Unknown Source:54)
            at org.bouncycastle.crypto.encodings.PKCS1Encoding.encodeBlock(Unknown Source:76)
            at org.bouncycastle.crypto.encodings.PKCS1Encoding.processBlock(Unknown Source:4)
            at org.bouncycastle.crypto.signers.RSADigestSigner.generateSignature(Unknown Source:25)
            at org.bouncycastle.operator.bc.BcSignerOutputStream.getSignature(Unknown Source:2)
            at org.bouncycastle.operator.bc.BcContentSignerBuilder$1.getSignature(Unknown Source:2)
            at org.bouncycastle.cert.X509v3CertificateBuilder.generateSig(Unknown Source:12)
            at org.bouncycastle.cert.X509v3CertificateBuilder.build(Unknown Source:40)
            at com.ubergeek42.WeechatAndroid.utils.AndroidKeyStoreUtils.generateCertificate(AndroidKeyStoreUtils.java:94)
            ...
    

    certificates for EC keys can be generated (i added case "ECDSA:), but can't be imported into AKS. I assume that it's because the key algorithm is "ECDSA" instead of "EC":

    java.security.KeyStoreException: Unsupported key algorithm: ECDSA
        at android.security.keystore.AndroidKeyStoreSpi.getLegacyKeyProtectionParameter(AndroidKeyStoreSpi.java:348)
        at android.security.keystore.AndroidKeyStoreSpi.setPrivateKeyEntry(AndroidKeyStoreSpi.java:360)
        at android.security.keystore.AndroidKeyStoreSpi.engineSetKeyEntry(AndroidKeyStoreSpi.java:294)
        at java.security.KeyStore.setKeyEntry(KeyStore.java:1179)
        at com.ubergeek42.WeechatAndroid.utils.AndroidKeyStoreUtils.putKeyPairIntoAndroidKeyStore(AndroidKeyStoreUtils.java:153)
    

    i didn't try DSA.

  • second, i cannot connect (using keys from AKS)

    using RSA key, i get the AndroidKeyStoreRSAPrivateKey is not a RSAPrivateKey (which it isn't):

    net.schmizz.sshj.userauth.UserAuthException: Exhausted available authentication methods
        at net.schmizz.sshj.SSHClient.auth(SSHClient.java:227)
        at net.schmizz.sshj.SSHClient.authPublickey(SSHClient.java:342)
        at net.schmizz.sshj.SSHClient.authPublickey(SSHClient.java:360)
        at com.ubergeek42.weechat.relay.connection.SSHConnection.connect(SSHConnection.java:134)
        at com.ubergeek42.weechat.relay.connection.RelayConnection.lambda$connect$1$RelayConnection(RelayConnection.java:90)
        at com.ubergeek42.weechat.relay.connection.-$$Lambda$RelayConnection$CgdZlWpK5gNUVzAJypQrciXI8lg.run(Unknown Source:2)
        at com.ubergeek42.weechat.relay.connection.RelayConnection$Protected.run(RelayConnection.java:180)
        at com.ubergeek42.weechat.relay.connection.Events$EventStream.lambda$new$1$Events$EventStream(Events.java:60)
        at com.ubergeek42.weechat.relay.connection.-$$Lambda$Events$EventStream$GPjMZUunGsQtYUQNi0r5b_jZ9qc.run(Unknown Source:2)
        at com.ubergeek42.weechat.relay.connection.Utils$FriendlyThread.run(Utils.java:108)
     Caused by: net.schmizz.sshj.userauth.UserAuthException: Supplied key (android.security.keystore.AndroidKeyStoreRSAPrivateKey) is not a RSAPrivateKey instance
        at net.schmizz.sshj.userauth.UserAuthException$1.chain(UserAuthException.java:33)
        at net.schmizz.sshj.userauth.UserAuthException$1.chain(UserAuthException.java:26)
        at net.schmizz.concurrent.Promise.deliverError(Promise.java:95)
        at net.schmizz.sshj.userauth.UserAuthImpl.notifyError(UserAuthImpl.java:157)
        at net.schmizz.sshj.transport.TransportImpl.die(TransportImpl.java:620)
        at net.schmizz.sshj.transport.Reader.run(Reader.java:66)
     Caused by: net.schmizz.sshj.common.SSHException: Supplied key (android.security.keystore.AndroidKeyStoreRSAPrivateKey) is not a RSAPrivateKey instance
        at net.schmizz.sshj.common.SSHException$1.chain(SSHException.java:36)
        at net.schmizz.sshj.common.SSHException$1.chain(SSHException.java:29)
        at net.schmizz.sshj.transport.TransportImpl.die(TransportImpl.java:614)
        at net.schmizz.sshj.transport.Reader.run(Reader.java:66) 
     Caused by: net.schmizz.sshj.common.SSHRuntimeException: Supplied key (android.security.keystore.AndroidKeyStoreRSAPrivateKey) is not a RSAPrivateKey instance
        at net.schmizz.sshj.signature.AbstractSignature.initSign(AbstractSignature.java:68)
        at net.schmizz.sshj.userauth.method.KeyedAuthMethod.putSig(KeyedAuthMethod.java:79)
        at net.schmizz.sshj.userauth.method.AuthPublickey.sendSignedReq(AuthPublickey.java:74)
        at net.schmizz.sshj.userauth.method.AuthPublickey.handle(AuthPublickey.java:45)
        at net.schmizz.sshj.userauth.UserAuthImpl.handle(UserAuthImpl.java:143)
        at net.schmizz.sshj.transport.TransportImpl.handle(TransportImpl.java:515)
        at net.schmizz.sshj.transport.Decoder.decodeMte(Decoder.java:159)
        at net.schmizz.sshj.transport.Decoder.decode(Decoder.java:79)
        at net.schmizz.sshj.transport.Decoder.received(Decoder.java:231)
        at net.schmizz.sshj.transport.Reader.run(Reader.java:60)
     Caused by: java.security.InvalidKeyException: Supplied key (android.security.keystore.AndroidKeyStoreRSAPrivateKey) is not a RSAPrivateKey instance
        at org.bouncycastle.jcajce.provider.asymmetric.rsa.DigestSignatureSpi.engineInitSign(Unknown Source:50)
        at java.security.Signature$Delegate.engineInitSign(Signature.java:1383)
        at java.security.Signature.initSign(Signature.java:679)
        at net.schmizz.sshj.signature.AbstractSignature.initSign(AbstractSignature.java:66)
        at net.schmizz.sshj.userauth.method.KeyedAuthMethod.putSig(KeyedAuthMethod.java:79) 
        at net.schmizz.sshj.userauth.method.AuthPublickey.sendSignedReq(AuthPublickey.java:74) 
        at net.schmizz.sshj.userauth.method.AuthPublickey.handle(AuthPublickey.java:45) 
        at net.schmizz.sshj.userauth.UserAuthImpl.handle(UserAuthImpl.java:143) 
        at net.schmizz.sshj.transport.TransportImpl.handle(TransportImpl.java:515) 
        at net.schmizz.sshj.transport.Decoder.decodeMte(Decoder.java:159) 
        at net.schmizz.sshj.transport.Decoder.decode(Decoder.java:79) 
        at net.schmizz.sshj.transport.Decoder.received(Decoder.java:231) 
        at net.schmizz.sshj.transport.Reader.run(Reader.java:60) 
    

    using EC key, i get

    ...
    Caused by: java.security.InvalidKeyException: cannot identify EC private key: java.security.InvalidKeyException: no encoding for EC private key
        at org.bouncycastle.jcajce.provider.asymmetric.util.ECUtil.generatePrivateKeyParameter(Unknown Source:225)
        at org.bouncycastle.jcajce.provider.asymmetric.ec.SignatureSpi.engineInitSign(Unknown Source:0)
        at java.security.Signature$Delegate.engineInitSign(Signature.java:1383)
        at java.security.Signature.initSign(Signature.java:679)
        at net.schmizz.sshj.signature.AbstractSignature.initSign(AbstractSignature.java:66)
    ...
    

    this also checks if it's ECPrivateKey and fails as the encoded key is null

any ideas as to what might be wrong?

perhaps we can have a tiny example of Android + sshj + AKS?

@fmeum
Copy link
Contributor Author

fmeum commented Oct 4, 2020

  • first, i can't import any keys into Android Key Store (AKS).

I would strongly recommend to generate keys within the Android Keystore, not import keys from the outside. If you want to protect an existing SSH key, consider wrapping it using the AndroidX Security library's EncryptedFile. For an example of how this is done with an ed25519 key in the context of SSHJ, see https://github.com/android-password-store/Android-Password-Store/blob/8a4316067396e6a0a5490e727197cf843d3f4ec6/app/src/main/java/com/zeapo/pwdstore/git/sshj/SshKey.kt#L217-L253.

Because I have never tried this myself, I unfortunately can't help you with the issues you are facing when importing keys.

  • second, i cannot connect (using keys from AKS)

This I might be able to help you with. The stack traces you provided seem to indicate that SSHJ is specifying the engine for its signature operations to be specifically BC, whereas the Keystore-backed keys require a different engine that should not be specified explicitly. With sshj 0.30.0, you should be able to work around this in the following way:

  1. Loop over all security providers looking for Android's own "BC" implementation and remove it.
  2. Insert the Java BC provider at that position.
  3. Instruct SSHJ to not register BC itself and specify a null SecurityProvider.

I implemented these steps here: https://github.com/android-password-store/Android-Password-Store/blob/8a4316067396e6a0a5490e727197cf843d3f4ec6/app/src/main/java/com/zeapo/pwdstore/git/sshj/SshjConfig.kt#L36-L57

@oakkitten
Copy link

thanks for trying to help!

strongly recommend to generate keys within the Android Keystore

it's a good idea, and we might implement it some day (low priority), but the ability to import existing keys is something our users expect so it better work

wrapping it using the AndroidX Security library

also a good idea, thanks. as i understand, the key material must be decrypted into memory in order for the key to perform actual crypto? but yeah, we should do it

can't help you with the issues you are facing when importing keys

i think i solved this somewhat, this code loads pkcs1 and pksc8 files only relying on BC. the trick is using JcaPEMKeyConverter() instead of JcaPEMKeyConverter().setProvider(bouncyCastleProvider)
exclusively for pkcs1 operations (and maybe only for ECDSA operations), that gives us EC keys that are importable into AKS

This I might be able to help you with

i tried your code and i still can't connect, except if i use a very nasty trick

i tried running setUpBouncyCastleForSshj() verbatim (except different logging statement) from Application.onCreate and i'm getting

JCE providers: AndroidNSSP (1.0), AndroidOpenSSL (1.0), CertPathProvider (1.0), AndroidKeyStoreBCWorkaround (1.0), BC (1.66), HarmonyJSSE (1.0), AndroidKeyStore (1.0)

and this error when connecting:

net.schmizz.sshj.transport.TransportException: Unable to reach a settlement: [diffie-hellman-group1-sha1, diffie-hellman-group-exchange-sha1] and [curve25519-sha256, [email protected], ecdh-sha2-nistp256, ecdh-sha2-nistp384, ecdh-sha2-nistp521, diffie-hellman-group-exchange-sha256, diffie-hellman-group16-sha512, diffie-hellman-group18-sha512, diffie-hellman-group14-sha256, diffie-hellman-group14-sha1]
    at net.schmizz.sshj.transport.Proposal.firstMatch(Proposal.java:149)
    at net.schmizz.sshj.transport.Proposal.negotiate(Proposal.java:130)
    at net.schmizz.sshj.transport.KeyExchanger.gotKexInit(KeyExchanger.java:223)
    at net.schmizz.sshj.transport.KeyExchanger.handle(KeyExchanger.java:359)
    at net.schmizz.sshj.transport.TransportImpl.handle(TransportImpl.java:517)
    at net.schmizz.sshj.transport.Decoder.decodeMte(Decoder.java:159)
    at net.schmizz.sshj.transport.Decoder.decode(Decoder.java:79)
    at net.schmizz.sshj.transport.Decoder.received(Decoder.java:231)
    at net.schmizz.sshj.transport.Reader.run(Reader.java:60)

(on the sshd part, i get this:)

Unable to negotiate with 192.168.1.128 port 41145: no matching key exchange method found. Their offer: diffie-hellman-group1-sha1,diffie-hellman-group-exchange-sha1 [preauth]

if i run my sshd as follows: /usr/sbin/sshd -ddd -p 23 -oKexAlgorithms=+diffie-hellman-group1-sha1 then i can connect fine, but allowing something like diffie-hellman-group1-sha1 sounds like a nasty nasty idea...

@fmeum
Copy link
Contributor Author

fmeum commented Oct 5, 2020

wrapping it using the AndroidX Security library

also a good idea, thanks. as i understand, the key material must be decrypted into memory in order for the key to perform actual crypto? but yeah, we should do it

Yes, the contents of the decrypted file would be used to initialize a PrivateKey, which could then be passed into SSHJ. This was the only option for us as we wanted to support ed25519 keys which are not supported by the Android Keystore directly.

can't help you with the issues you are facing when importing keys

i think i solved this somewhat, this code loads pkcs1 and pksc8 files only relying on BC. the trick is using JcaPEMKeyConverter() instead of JcaPEMKeyConverter().setProvider(bouncyCastleProvider)

This looks pretty good. The rule of thumb regarding the Android Keystore is that everything should work as long as no explicit provider is ever set anywhere.

and this error when connecting:

net.schmizz.sshj.transport.TransportException: Unable to reach a settlement: [diffie-hellman-group1-sha1, diffie-hellman-group-exchange-sha1] and [curve25519-sha256, [email protected], ecdh-sha2-nistp256, ecdh-sha2-nistp384, ecdh-sha2-nistp521, diffie-hellman-group-exchange-sha256, diffie-hellman-group16-sha512, diffie-hellman-group18-sha512, diffie-hellman-group14-sha256, diffie-hellman-group14-sha1]
    at net.schmizz.sshj.transport.Proposal.firstMatch(Proposal.java:149)
    at net.schmizz.sshj.transport.Proposal.negotiate(Proposal.java:130)
    at net.schmizz.sshj.transport.KeyExchanger.gotKexInit(KeyExchanger.java:223)
    at net.schmizz.sshj.transport.KeyExchanger.handle(KeyExchanger.java:359)
    at net.schmizz.sshj.transport.TransportImpl.handle(TransportImpl.java:517)
    at net.schmizz.sshj.transport.Decoder.decodeMte(Decoder.java:159)
    at net.schmizz.sshj.transport.Decoder.decode(Decoder.java:79)
    at net.schmizz.sshj.transport.Decoder.received(Decoder.java:231)
    at net.schmizz.sshj.transport.Reader.run(Reader.java:60)

You get this error because the SSHJ default config is not really usable without automatically registered BC. You will have to roll your own config. Ours can be found here for reference, just ignore the OpenKeychainWrappedKeyAlgorithmFactory.

@oakkitten
Copy link

aha, i think i figured this part out! for the sake of testing, this works:

SecurityUtils.setSecurityProvider("BC");
ssh = new SSHClient();
SecurityUtils.setSecurityProvider(null);

this will require a lot of work, still.

perhaps this would be best to put into AndroidConfig? that file seems outdated?

another small thing. how do i specify the types of host keys that i have for the server? looking at Proposal.java there's

firstMatch(this.getHostKeyAlgorithms(), other.getHostKeyAlgorithms()),

and host keys come from

public List<String> getHostKeyAlgorithms() {
    return sig;
}

and sig comes from

sig = Factory.Named.Util.getNames(config.getKeyAlgorithms());

but as key algorithms come from config, they are the same for every connection, aren't they? so if the server had only key of type RSA, and we remember it, and the server adds Ed25519 key, which has a higher priority in sshj, and we try to connect, we'll be getting that huge someone is doing something evil warning, won't we? instead of verifying using RSA

while it's possible to specify key algorithms in the config, it feels like a wrong place to do it?

also, i don't see how OpenSSHKnownHosts is doing this...

@oakkitten
Copy link

also, ECDSA host keys will not work, as in ECDSAVariationsAdapter.java there's this piece of code:

    static PublicKey readPubKeyFromBuffer(Buffer<?> buf, String variation) throws GeneralSecurityException {
        String algorithm = BASE_ALGORITHM_NAME + variation;
        if (!SecurityUtils.isBouncyCastleRegistered()) {
            throw new GeneralSecurityException("BouncyCastle is required to read a key of type " + algorithm);
        }
    ...

@fmeum
Copy link
Contributor Author

fmeum commented Oct 6, 2020

aha, i think i figured this part out! for the sake of testing, this works:

SecurityUtils.setSecurityProvider("BC");
ssh = new SSHClient();
SecurityUtils.setSecurityProvider(null);

this will require a lot of work, still.

perhaps this would be best to put into AndroidConfig? that file seems outdated?

another small thing. how do i specify the types of host keys that i have for the server? looking at Proposal.java there's

firstMatch(this.getHostKeyAlgorithms(), other.getHostKeyAlgorithms()),

and host keys come from

public List<String> getHostKeyAlgorithms() {
    return sig;
}

and sig comes from

sig = Factory.Named.Util.getNames(config.getKeyAlgorithms());

but as key algorithms come from config, they are the same for every connection, aren't they? so if the server had only key of type RSA, and we remember it, and the server adds Ed25519 key, which has a higher priority in sshj, and we try to connect, we'll be getting that huge someone is doing something evil warning, won't we? instead of verifying using RSA

The negotiation algorithm is correct according to the relevant SSH RFC, but perhaps the client proposal, which you can tweak via the per-connection SshjConfig, should only consist of the subset of algorithms it already has a host key for (if not empty)? I don't even know how OpenSSH does this though. @hierynomus Do you have a recommendation on how to fix a host key type per connection?

also, ECDSA host keys will not work, as in ECDSAVariationsAdapter.java there's this piece of code:

Good catch, that seems to be an Android compatibility issue. I will look into whether I can fix it.

@oakkitten
Copy link

OpenSSH sends all keys, but it reorders them unless you overwrite that in a config (see the penultimate paragraph):

HostKeyAlgorithms
        Specifies the host key algorithms that the client wants to use in or‐
        der of preference.  Alternately if the specified value begins with a
        ‘+’ character, then the specified key types will be appended to the
        default set instead of replacing them.  If the specified value begins
        with a ‘-’ character, then the specified key types (including wild‐
        cards) will be removed from the default set instead of replacing them.
        The default for this option is:

           [email protected],
           [email protected],
           [email protected],
           [email protected],
           [email protected],[email protected],
           [email protected],
           ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,
           ssh-ed25519,rsa-sha2-512,rsa-sha2-256,ssh-rsa

        If hostkeys are known for the destination host then this default is
        modified to prefer their algorithms.

        The list of available key types may also be obtained using "ssh -Q
        key".

e.g. for a single RSA key,

debug3: order_hostkeyalgs: prefer hostkeyalgs: [email protected],[email protected],[email protected],rsa-sha2-512,rsa-sha2-256,ssh-rsa
...
debug2: local client KEXINIT proposal
...
debug2: host key algorithms: [email protected],[email protected],[email protected],rsa-sha2-512,rsa-sha2-256,ssh-rsa,[email protected],[email protected],[email protected],[email protected],ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,ssh-ed25519

i'm assuming it sends all algos in order to in fact receive some host key so that it can print its hash to the user along with the warning

@fmeum
Copy link
Contributor Author

fmeum commented Oct 6, 2020

I didn't know this is what OpenSSH is doing, but it should be easy to recreate in SSHJ: You can permute the host key algorithms in your instance of SshjConfig according to the memorized host key.

@oakkitten
Copy link

this is doable, but feels hackish. requires me to recreate config which is supposed to be global, and to have an opinion on which host key algorithms are better. i'd expect something like ssh.setAvailableHostKeyTypes(hostKeys.getAvailableHostKeyTypes(host, port))

i'm not quite sure that you can get available host keys from OpenSSHKnownHosts without subclassing it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants