sailfishos / sailfish-secrets

BSD 3-Clause "New" or "Revised" License
24 stars 15 forks source link

Feature: QML plugin support for RSA #174

Open szopin opened 3 years ago

szopin commented 3 years ago

I'm hoping to add login support for sfos forum viewer, but would prefer to keep it QML only so it stays always tinkerable on device and easily auditable. Will try to add some kind of a weird workflow for now so people can generate a keypair, probably using some online tool that also will decrypt the token from the forum, but if it is not too much of a hassle ideally in sfos 4.1 it could be done properly (even if with still experimental and subject to change API). I believe the whole workflow is:

  1. generate a key pair
  2. pass public key to forum
  3. returns a payload (token) encrypted with the public key
  4. decrypt token using private key from step 1
  5. store token Steps 1, 4 and 5 are I believe currently not doable from QML, so this issue/feature request is for that. Thanks in advance
Venemo commented 3 years ago

Why do you think they are not doable in QML?

Sorry, I misremembered

szopin commented 3 years ago

If any of them is even partially doable that would be a huge help (browser doesn't like text input fields for copy-pasting which makes one idea not doable on-device)

chriadam commented 3 years ago

The QML API already has support for some RSA stuff (see e.g. https://github.com/sailfishos/sailfish-secrets/blob/master/qml/Crypto/main.cpp#L95) so it may very well be the case that much of this can be done already.

Basically, it would be similar to https://github.com/sailfishos/sailfish-secrets/pull/173 - i.e. just use a GenerateKeyRequest (or GenerateStoredKeyRequest) to generate the key (but instead of using AlgorithmAes, use AlgorithmRsa, and instead of using keyDerivationParameters, use keyPairGenerationParameters), then normal EncryptRequest and DecryptRequest to do the cipher stuff.

e.g. keyPairGenerationParameters: cryptoManager.constructRsaKeygenParams({"modulusLength": 2048, "numberPrimes": 2, "publicExponent": 65537}); should work, I think.

szopin commented 3 years ago

Thanks so much, I've been trying to get the modified example running in SDK, but it seems to complain (added CONFIG += link_pkgconfig and PKGCONFIG += sailfishsecrets sailfishcrypto in .pro as in the help docs for crypto and secrets libraries):

module "Sailfish.Crypto" is not installed 
module "Sailfish.Secrets" is not installed 

Same happens when trying to qmlscene any qml file with the "import Sailfish.Crypto/Secrets as..." line on device. I'm missing something very obvious I'm sure.

szopin commented 3 years ago

Thanks to dcaliste managed to get it kinda working ('Requires: libsailfishsecretsplugin' and libsailfishcryptoplugin did the trick). Now only complains about 'No such cryptographic service provider plugin exists' (on phone, emulator seems to be missing the daemon) and seems the constructKeyTemplate is needed in qml/Crypto/main.cpp after all. Any chance constructKeyTemplate gets in for SFOS 4.0.2 (or 4.1 whichever is next)? Or is it possible to do that in QML without the additions from this example?

dcaliste commented 3 years ago

'No such cryptographic service provider plugin exists'

Sorry, I didn't pay enough attention, can you point me to your code, so I can give a look ?

Here is an example of how I generate a pair of GnuPG keys (not exactly your usecase, but it may help) (notice this code is proprietary Jolla, don't copy paste it, you can find it on device if you have installed the GnuPG plugin for emails):

            Crypto.GenerateStoredKeyRequest {
                id: keyGenerator
                manager: cryptoManager
                cryptoPluginName: "org.sailfishos.crypto.plugin.gnupg.openpgp"
                keyPairGenerationParameters: cryptoManager.constructRsaKeygenParams(
                    {"name": identity,
                     "email": emailAddress,
                     "expire": "2y"})
                keyTemplate: cryptoManager.constructKey("name",
                    "import", "org.sailfishos.crypto.plugin.gnupg.openpgp")
                onStatusChanged: {
                    if (status === Crypto.Request.Finished) {
                        if (result.code === Crypto.Result.Succeeded) {
                            keyColumn.keyRingChanged()
                            deleteKeyHelper.startRequest()
                            errorLabel.text = ""
                        } else {
                            console.log(result.code)
                            //% "Cannot generate key: %1"
                            errorLabel.text = qsTrId("settings-accounts-la-key_generation_error").arg(result.errorMessage)
                        }
                    }
                }
            }
szopin commented 3 years ago

Thanks Damien, I'm trying to run a code that's basically example from #173 with the changes from Chris from this discussion (https://github.com/sailfishos/sailfish-secrets/pull/173/files#diff-67dbe4d4a6e70353df8f64b0e0ab46cac90e122e2e9530b6db64a542502b769d with https://github.com/sailfishos/sailfish-secrets/issues/174#issuecomment-811640857) and trying to pass AlgorithmRsa, but this needs constructKeyTemplate (or at least that's how it works in the example, maybe RSA keypair can be created without a template?). The plugin part is probably caused by: cryptoPluginName: crypto.defaultCryptoPluginName Weird as your code seems to use constructKey for the keyTemplate. Can it be used with AlgorithmRsa?

dcaliste commented 3 years ago

I'll try the code next week, I think. I don't remember all the bit honestly, I worked on this some years ago already. But I remember that I definetely has to arrange one or two things to make the key generation works from QML only. Just that I cannot say at the moment if it was GnuPG specific or if it should work for RSA also.

If you copy-paste the whole QML code somewhere, I'll give a look and see how to make it works.

Besides, the daemon is in package sailfishsecretsdaemon. I've no idea if it can work in the emulator, I've never tried.

szopin commented 3 years ago

Thanks so much! I've pasted the code I used to generate a package so I can tinker on device here: https://pastebin.com/hY4RSNKJ (changed constructKeyTemplate to constructKey as the constructKeyTemplate is only added to the qml/Crypto/main.cpp in the PR, but of course that's wrong) edit: the output of that code:

[D] unknown:0 - Using Wayland-EGL
[D] onCompleted:580 - Warning: specifying an object instance for initialPage is sub-optimal - prefer to use a Component
[D] onResultChanged:59 - GKR: error: No such cryptographic service provider plugin exists
dcaliste commented 3 years ago

Just a remark, do you have this package installed : sailfishsecretsdaemon-cryptoplugins-default ? It contains the OpenSSL plugin to do crypto operations.

szopin commented 3 years ago

Didn't have it, installed it but the output didn't change

szopin commented 3 years ago

Actually just needed a reboot it seems as now it throws: [D] onResultChanged:59 - GKR: error: This method can only generate RSA keys.

dcaliste commented 3 years ago

Yeh, sorry, I forgot to tell you to restart the daemon after adding the new plugins. Now, I'll try later today or tomorrow to see what's wrong with this RSA key generation.

dcaliste commented 3 years ago

I didn't try, but I would replace the line

keyTemplate: crypto.constructKey(Crypto.CryptoManager.AlgorithmRsa,
                                                     Crypto.CryptoManager.OperationEncrypt | Crypto.CryptoManager.OperationDecrypt)

with:

keyTemplate: {
   var key = crypto.constructKey("myKey", "myGroup", crypto.defaultCryptoPluginName)
   key.algorithm = Crypto.CryptoManager.AlgorithmRsa
   key.operations = Crypto.CryptoManager.OperationEncrypt | Crypto.CryptoManager.OperationDecrypt
    return key
}
szopin commented 3 years ago

Thanks so much, it works! gkr.generatedKey has both the private and public key, awesome! Even better generatedKey.publicKey and .privateKey have the respective parts so no need to do any string operations (encryption request still complains about 'TODO: initializationvector not yet supported for asymmetric encryption' and 'TODO: block mode other than Unknown' after commenting initializationvector out, but that's probably because of AES->RSA and I just need decryption anyway, should be fine) https://i.ibb.co/S0Lst4S/Screenshot-21-05-03-13-51-20.png (added generatedKey to Cipher just to see it in action)

dcaliste commented 3 years ago

encryption request still complains about 'TODO: initializationvector not yet supported for asymmetric encryption'

Yeh, RSA encryption or decryption will not work with any non null initialisationVector. It's not implemented. Normally your EncryptionRequest should return a NotSupported error. AES doesn't have the same restriction.

szopin commented 3 years ago

Uh-oh, decryption will be needed, wonder if nonce that's being passed to the forum is the same as initialization vector (both 16 bytes, hopefully jist coincidence), it does seem to be required on discourse side. Maybe '0' will work. Still even with 16bytes nonce have only 10% success rate of reaching the authorize/payload page (same code, just executed over and over, wonder if some of the keys are generated incorrectly or just Discourse throwing a tantrum, doesn't seem to be time-limited as getting same failure rate even after waiting 15 minutes or so)

On Tuesday, 4 May 2021, Damien Caliste wrote:

encryption request still complains about 'TODO: initializationvector not yet supported for asymmetric encryption'

Yeh, RSA encryption or decryption will not work with any non null initialisationVector. It's not implemented. Normally your EncryptionRequest should return a NotSupported error.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/sailfishos/sailfish-secrets/issues/174#issuecomment-83194945

-- Sent from my Jolla

szopin commented 3 years ago

Sorry to resurrect that, but I did manage with help from vige to get my 15% success rate of getting a key from sfos forum up to 100% (encodeURI vs component), could anyone please give an example of how QML code would look to decode RSA having all required fields? The example I'm working with seems to lack something curcial. I tried to comment out initializationVector, padding and blockMode, or any combination of these, but it still errors out (key generation works great), again sorry but this is way too deep for me to figure out it seems

chriadam commented 3 years ago

You shouldn't need any IV or block mode at least. What is the question, precisely? Is it: you have a chunk of data which has been RSA encrypted with your public key, and you need to use your private key to decrypt the chunk?

I would have suspected it to work similarly to https://github.com/sailfishos/sailfish-secrets/blob/master/tests/qml/tst_qml_signing/tst_qml_signing.qml but using EncryptRequest instead of SignRequest, and DecryptRequest instead of VerifyRequest.

/edit: I was wrong, you need to specify BlockModeUnknown specifically apparently. Also, you need to use the appropriate EncryptionPadding mode.

chriadam commented 3 years ago

I've added a simple round-trip encrypt+decrypt unit test to the qmlexample branch: https://github.com/sailfishos/sailfish-secrets/pull/173/files#diff-16f96021a288d7f27b5fa767c7a584b17b5693f06a96f42b670b0e78f084f3b5R99

This one passes for me at least (when I have a clean test database, and have the daemon running with devel-su -p sailfishsecretsd --test etc).