rdiankov / openrave

Open Robotics Automation Virtual Environment: An environment for testing, developing, and deploying robotics motion planning algorithms.
http://www.openrave.org
Other
693 stars 343 forks source link

GPG-based encryption #1262

Closed undisputed-seraphim closed 10 months ago

undisputed-seraphim commented 1 year ago

This pull request

This pull request seeks to merge the ability to read GPG-encrypted files.

There are some preamble commits. First, the functions used to query file extensions are simplified. _Is***URI() functions are decoupled from also querying file extensions, so that fewer slow regex operations are used in the initial file-or-URI loading process. We test if a string is a URI only once; subsequently we only need to check the extensions. I have also streamlined some of the logic used in ReadInterfaceURI(). I have also added a stringutils.h to deduplicate the multiple suffix matching functions, then added prefix matching for symmetry.

The new feature incurs a dependency on libgpgme and libgpgmepp. These are the official C and C++ interfaces to GPG. The implementation simply adds an additional matching test for .gpg and .pgp file extensions and calls _ParseEncryptedDocument() in jsondownloader.cpp, which is where the meat of the implementation is.

To leverage this feature, simply install the correct secret key into the OS. If a matching key to decrypt a document exists, then it is automatically used; notice that the code does not need to do any key-finding, it simply loads the data into a structure and then calls gpgCtx->decrypt(). The resulting plaintext document is then stored back into the same context buffer, and then recurses into _ParseDocument().

Further work

This implementation only supports reading. Write-and-encrypting documents will be a subsequent pull request.

cielavenir commented 1 year ago

@undisputed-seraphim May I have a very simple test case? Also, I remember that gpgme requires gpg-agent running in background; Is gpg-agent started automatically?

[edit] it is done :tada:

cielavenir commented 1 year ago

@undisputed-seraphim Seems I need the private key for dev/webstack/-/blob/encryptEnvironmentForTesting20230620/gpgpubkey.asc (if I need to test this)

[edit] the key matter was resolved, thank you!

cielavenir commented 1 year ago

Test procedure:

  1. gpg --import < private.key
  2. gpg --encrypt --recipient 0123456789ABCDEF < motoman-gp7.mujin.json > motoman-gp7.mujin.json.gpg
  3. copy motoman-gp7.mujin.json.gpg and private.key
  1. gpg --import < private.key
  2. killall gpg-agent # to ensure that gpgme wakes gpg-agent up automatically
  3. python -m openravepy -i --viewer= -p "RaveSetDebugLevel(100); env.LoadURI('mujin:/motoman-gp7.mujin.json.gpg',{'openravescheme':'mujin','remoteurl':'file:///home'})"

It was loaded correctly :tada:

Issues:

  1. RaveSetDebugLevel needs to be set, cf https://github.com/rdiankov/openrave/pull/1262#discussion_r1238116412
  2. Without tini, gpg zombie processes are created.
  3. env.Load() does not work. For testing I used the fact that curl can download file:/// URI, but then it is incompatible with how the scene is loaded in the controller. Actually what is the intended use? If the gpg file is used inside the controller, we would need two key pairs, one for env and one for downloaddebug, because env one's private key needs to be installed to the controller. Also, we need some protection mechanism or unencrypted files can be downloaded easily.
undisputed-seraphim commented 1 year ago

In the new updates, I have moved the code handling encryption and decryption into jsoncommon.cpp.

In jsoncommon.h, most of the RaveParse{JSON|MsgPack}{URI|File|Data} and RaveWrite{JSON|MsgPack}{File|Stream|Memory} are highly redundant, and I think we should see if those code can be deduplicated.

cielavenir commented 1 year ago

Also, we need some protection mechanism or unencrypted files can be downloaded easily.

I will check this part with @woswos

cielavenir commented 1 year ago

@undisputed-seraphim is it possible to set the homedir? It seems possible by set_engine_info (or Context::setEngineHomeDirectory).

edit: nvm environ['GNUPGHOME'] worked

rdiankov commented 1 year ago

@ziyan is this ready?

superfashi commented 1 year ago

@ziyan is this ready?

I will look at the code and have this ready.

superfashi commented 1 year ago

Should be cleaned up @cielavenir do you want to test again?

ziyan commented 1 year ago

@superfashi Please add unit test to testopenrave

rdiankov commented 1 year ago

is this ready?

rdiankov commented 1 year ago

Anyone actively working on this?

ziyan commented 1 year ago

Anyone actively working on this?

@superfashi Please resolve conflicts and I will take another pass.

superfashi commented 1 year ago

Anyone actively working on this?

@superfashi Please resolve conflicts and I will take another pass.

I will merge master by the end of day. Merged. Very sorry for the delayed response as I'm currently out of office.

superfashi commented 1 year ago

is this ready?

@rdiankov It is now ready as reviewed by Ziyan, tested by me.

rdiankov commented 10 months ago

thanks~