ubiquity / ubiquibot-kernel

0 stars 11 forks source link

Decrypt sensitive config parameters #90

Open rndquu opened 1 month ago

rndquu commented 1 month ago

Right now there are 2 ways to hide sensitive bot's config parameters:

  1. Make a config repository private (organization collaborators can still see the values though)
  2. Fork plugin, self host and set sensitive parameters in env variables (bad UX for partners)

Sensitive config parameters could be encrypted via our own x25519_PUBLIC_KEY (the same one we use for encrypting partners' private keys) and the kernel could then decrypt them.

So as a part of this issue the kernel should be able to decrypt config parameters on initial config parsing.

The expected flow for parsing a single config param:

  1. Try to decrypt the parameter.
  2. If it is decrypted then use the decrypted parameter.
  3. If there's a decryption error then assume the parameter is not encrypted and use a raw value.

Notice that there is a difference between decrypting:

  1. Unencrypted param
  2. Encrypted param with another PK

I hope https://doc.libsodium.org/ distinguishes those 2 errors because in the 2nd case it will be a very subtle bug and the kernel should ideally throw an error like The config parameter encryption is invalid so that partner knew he did something wrong on encrypting a param.

P.S. Handy tool: https://keygen.ubq.fi/

rndquu commented 1 month ago

@gentlementlegen Pls check the description

0x4007 commented 1 month ago
Error: incorrect key pair for the given ciphertext

I believe this is the error thrown for

  1. Encrypted param with another PK

Another idea for a future vision of mine (perhaps out of scope for this task) is to abstract the GitHub UI with a Ubiquity app that looks like the following:

  1. Login with GitHub
  2. Setup wizard with some text input fields for the secrets
  3. Save button

And behind the scenes we handle the forking on behalf of the user and populate the secrets.

This approach can generally allow us to build on top of GitHub as our backend while still being able to control the end user experience.

rndquu commented 1 month ago

Another idea for a future vision of mine (perhaps out of scope for this task) is to abstract the GitHub UI with a Ubiquity app > that looks like the following:

  1. Login with GitHub
  2. Setup wizard with some text input fields for the secrets
  3. Save button

This is basically https://github.com/ubiquity/onboard.ubq.fi v2 where users should be able to edit their configs via UI.

https://github.com/ubiquity/onboard.ubq.fi/issues/22

whilefoo commented 1 week ago

So you propose to try to decrypt every parameter in the config instead of creating a special prefix like _UBIQUITY_ENCRYPTED_PARAM=?

rndquu commented 1 week ago

So you propose to try to decrypt every parameter in the config instead of creating a special prefix like _UBIQUITY_ENCRYPTED_PARAM=?

Yes

I don't fully understand how the _UBIQUITY_ENCRYPTED_PARAM prefix may be used. For example, if partner wants to encrypt the evmPrivateEncrypted param then how to tell the kernel that param is encrypted? Either via some custom prefix, like evmPrivateEncrypted_UBIQUITY_ENCRYPTED_PARAM or via introducing a new param option so the config could look like:

plugins:
  issues.closed:
    - uses:
      - plugin: rndquu/conversation-rewards@fix/nonce-generation
        with:
          evmPrivateEncrypted: 
            value: "wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd"
            encrypted: true

I wanted things to be backwards compatible. But if decrypting every param takes time (or there are other issues with this approach) then we could stick to the encrypted option param or suffixes.

whilefoo commented 1 week ago

like this

plugins:
  issues.closed:
    - uses:
      - plugin: rndquu/conversation-rewards@fix/nonce-generation
        with:
          evmPrivateKey: "_UBIQUITY_ENCRYPTED_PARAM=wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd"
rndquu commented 5 days ago

like this

plugins:
  issues.closed:
    - uses:
      - plugin: rndquu/conversation-rewards@fix/nonce-generation
        with:
          evmPrivateKey: "_UBIQUITY_ENCRYPTED_PARAM=wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd"

For me this way of telling the kernel that param is encrypted is more straightforward:

plugins:
  issues.closed:
    - uses:
      - plugin: rndquu/conversation-rewards@fix/nonce-generation
        with:
          evmPrivateEncrypted: 
            value: "wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd"
            encrypted: true
whilefoo commented 5 days ago

I agree your approach looks good but what if someone creates a plugin with the same structure. Maybe we can change encrypted to ubiquityEncrypted?

0x4007 commented 5 days ago

I wanted things to be backwards compatible.

We shouldn't put effort into this until its necessary. We don't have active partners, however now at conferences I've been more focused on meeting prospective partners and investors.

Even if we get active partners (theres a couple interested in the pipeline) we can version lock plugins and things, right? Not sure if we can do versioned deployments of the kernel but that would be the final step to making things totally version-able and we no longer have to worry at all about backwards compatibility.

For us internally I think we should always be running the latest to catch problems faster.

rndquu commented 19 hours ago

I agree your approach looks good but what if someone creates a plugin with the same structure. Maybe we can change encrypted to ubiquityEncrypted?

Yes, plugin developer may create a param with the encrypted name.

To sum up there are at least 3 options:

  1. Try to decrypt everything (as described in the issue description)
  2. Use prefix, example:
    plugins:
    issues.closed:
    - uses:
      - plugin: rndquu/conversation-rewards@fix/nonce-generation
        with:
          evmPrivateKey: "_UBIQUITY_ENCRYPTED_PARAM=wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd"
  3. Use a separate param with "unique" name, example:
    plugins:
    issues.closed:
    - uses:
      - plugin: rndquu/conversation-rewards@fix/nonce-generation
        with:
          evmPrivateEncrypted: 
            value: "wOzNgt-yKT6oFlOVz5wrBLUSYxAbKGE9Co-yvT8f9lePsx7wJwPVugS9186zdhr1T4UpkpXvq9ii5M-nWfrydMnllSkowH4LirRZsHbvRVSvDoH_uh80p6HpwqDSG3g4Nwx5q0GD3H-ne4vwXMuwWAHd"
            ubiquityEncrypted: true