Crypto tutorial #1955

petermax2 · 2018-05-06T16:04:00Z

Purpose

Add crypto tutorial, as requested in #1948 .

Please feel free to critisize, add ideas, give feedback, improve, etc. etc.

TODO

use Shell Recorder syntax in examples

Checklist

commit messages are fine ("module: short statement" syntax and refer to issues)
I added unit tests
I ran all tests locally and everything went fine
affected documentation is fixed
I added code comments, logging, and assertions (see doc/CODING.md)
meta data is updated (e.g. README.md of plugins)
release notes are updated (doc/news/_preparation_next_release.md)

@markus2330 please review my pull request

markus2330

Thank you for the great tutorial!

markus2330 · 2018-05-06T18:36:39Z

doc/tutorials/crypto.md

+
+## Configuration File Encryption/Decryption
+
+The `fcrypt` plugin enables the encryption and decryption of entire configuration files, thus protecting the confidentiality of the configuration values.


suggestions:

of all containing configuration settings.

of the configuration keys and values.

markus2330 · 2018-05-06T18:36:52Z

doc/tutorials/crypto.md

+Elektra can protect the following aspects of your configuration:
+
+1. confidentiality, and
+2. integrity.


write very shortly what this is.

markus2330 · 2018-05-06T18:37:24Z

doc/tutorials/crypto.md

+The `fcrypt` plugin enables the encryption and decryption of entire configuration files, thus protecting the confidentiality of the configuration values.
+`fcrypt` utilizes GnuPG (GPG) for all cryptographic operations.
+The GPG key, which is used for encryption and decryption, is specified in the backend configuration under `encrypt/key`.
+You MUST be in possesion of the private key, otherwise `fcrypt` will deny all operations.


typo possesion

markus2330 · 2018-05-06T18:37:54Z

doc/tutorials/crypto.md

+The GPG key, which is used for encryption and decryption, is specified in the backend configuration under `encrypt/key`.
+You MUST be in possesion of the private key, otherwise `fcrypt` will deny all operations.
+
+Let's assume your GPG private key has the ID `DDEBEF9EE2DC931701338212DAF635B17F230E8D`.


Please explain how to find out.

markus2330 · 2018-05-06T18:38:37Z

doc/tutorials/crypto.md

+
+The `fcrypt` plugin enables the encryption and decryption of entire configuration files, thus protecting the confidentiality of the configuration values.
+`fcrypt` utilizes GnuPG (GPG) for all cryptographic operations.
+The GPG key, which is used for encryption and decryption, is specified in the backend configuration under `encrypt/key`.


That is too fast. It is not clear what the tutorial is about. What is a backend configuration? (maybe avoid the term altogether)

markus2330 · 2018-05-06T18:48:05Z

doc/tutorials/crypto.md

+
+	kdb mount test.ini user/test crypto_gcrypt "crypto/key=DDEBEF9EE2DC931701338212DAF635B17F230E8D" base64 ini
+
+We recommend adding the `base64` plugin to the backend, because `crypto` will output binary data.


Why not recommend it in the contract? Then simply pass --with-recommends in the command above.

markus2330 · 2018-05-06T18:48:42Z

doc/tutorials/crypto.md

+
+We recommend adding the `base64` plugin to the backend, because `crypto` will output binary data.
+Having binary data in configuration files is hardly ever feasible.
+`base64` encodes all binary values within a configuration file and transforms them into Base64 strings.


Small example: what are base64 strings

markus2330 · 2018-05-06T18:49:38Z

doc/tutorials/crypto.md

+If the value is equal to `1`, the value of the Key will be encrypted.
+
+Let's demonstrate this using an example.
+We want to protect the password, that is stored under `user/test/password`.


Again: a more concrete example would be more interesting.

markus2330 · 2018-05-06T18:50:52Z

doc/tutorials/crypto.md

+We want to protect the password, that is stored under `user/test/password`.
+So we set the meta-key as follows:
+
+	kdb setmeta user/test/password crypto/encrypt 1


Does it also work if you do everything with cascading keys (without user). (And thus writing to spec)

markus2330 · 2018-05-06T18:54:54Z

doc/tutorials/crypto.md

+You can disable the encryption by setting `crypto/encrypt` to a value other than `1`, for example:
+
+	kdb setmeta user/test/password crypto/encrypt 0
+


Always demonstrate that what you said actually happened (cat the file or grep for the value)

markus2330

Much better now! A small remark.

markus2330 · 2018-05-06T19:55:30Z

doc/tutorials/crypto.md

+
+The GPG key we use in this tutorial has the ID `DDEBEF9EE2DC931701338212DAF635B17F230E8D`.
+
+A GPG private key is mandatory for the plugins to work.


Also for checking signatures?

markus2330 · 2018-05-06T19:55:51Z

doc/tutorials/crypto.md

+
+	gpg2 --generate-key
+
+The `fcrypt` plugin and the `crypto` plugin support both versions (version 1 and version 2) of GPG.


Is it possible to intermix them? Which will be preferred?

sanssecours

Thank you for writing the tutorial. The text looks good to me. I only noticed some minor things.

Overall I think it would make sense to use Markdown Shell Recorder syntax in the tutorial. Two benefits of this approach are:

The reader of the tutorial sees all commands and the expected behavior of the commands.
The build servers check that the commands work correctly.

If we use this approach, then the tutorial requires a (predetermined) cryptographic key pair. I suggest we add a randomly generated pair of public and private key to the repo. In the tutorial we can then add shell commands to import the keys via gpg at the start and commands to remove the two keys at the end. I do not know how much work that is though, since I do not use gpg that often.

sanssecours · 2018-05-07T06:06:43Z

doc/tutorials/crypto.md

+In this tutorial we explain the use of the `crypto` plugin and the `fcrypt` plugin by a simple example:
+We want to protect a password that is contained in an INI-file.
+
+Without encryption, the file could be mounted like:


I am not sure if this is actually a mistake, but I would add the word “this” here:

…could be mounted like this:

.

sanssecours · 2018-05-07T06:31:58Z

doc/tutorials/crypto.md

+`fcrypt` utilizes GPG for all cryptographic operations.
+The GPG key, which is used for encryption and decryption, is specified in the backend configuration under `encrypt/key`.
+
+	sudo kdb mount test.ini user/test fcrypt "encrypt/key=DDEBEF9EE2DC931701338212DAF635B17F230E8D" ini


This command fails if we start with an empty configuration:

The command kdb mount terminated unsuccessfully with the info: Too many plugins! The plugin sync can't be positioned at position precommit anymore. Try to reduce the number of plugins! Failed because precommit with 7 is larger than 6 Please report the issue at https://issues.libelektra.org/

. I would add something like this:

If the above command fails, please take a look at the [ReadMe of the `fcrypt` plugin](https://master.libelektra.org/src/plugins/fcrypt/README.md#known-issues).

.

sanssecours · 2018-05-07T06:32:12Z

doc/tutorials/crypto.md

+
+The command above results in the following content of `test.ini`:
+
+	password = 1234


I think it might also make sense to add a shell command that prints the content of test.ini here:

kdb file user/test/password | xargs cat #> password = 1234

.

sanssecours · 2018-05-07T06:44:54Z

doc/tutorials/crypto.md

+## Configuration File Signatures
+
+`fcrypt` also offers the option to sign and verify configuration files, thus protecting the integrity of the configuration values.
+If `sign/key` is specified in the backend configuration, `fcrypt` will forward the key ID to be used for signing the configuration file.


I think you can remove ”to be used” here:

…fcrypt will forward the key ID to sign the configuration file.

.

sanssecours · 2018-05-07T07:38:26Z

doc/tutorials/crypto.md

+An example backend configuration is given as follows:
+
+	sudo kdb mount test.ini user/test crypto_gcrypt "crypto/key=DDEBEF9EE2DC931701338212DAF635B17F230E8D" base64 ini
+


This line contains 4 trailing spaces (as do lines 86, 130, 134 and 143). Do your really want to add an empty line of code after the command?

sanssecours · 2018-05-07T08:07:14Z

doc/tutorials/crypto.md

+3. `crypto_botan`
+
+provide the option to encrypt and decrypt single configuration values (Keys) in a Keyset.
+`crypto` is using GPG for key-handling.


While not wrong in any shape or form, I think we should prefer active over passive voice whenever possible:

crypto uses GPG for key-handling.

.

And to not start with a mono spaced word, you could write:

The plugin crypto uses GPG for keys.

petermax2 · 2018-05-07T08:43:26Z

Thank you both for the feedback. I will try to further improve the tutorial.

How hard is it to learn the Shell-Recorder stuff? I've never used it before (but would like to try) 😄

sanssecours · 2018-05-07T10:54:34Z

How hard is it to learn the Shell-Recorder stuff?

I think using the Markdown Shell Recorder should be relatively easy, if you know a little bit about shell commands. While the (Markdown) Shell Recorder certainly has a lot of problems, using it for simple commands should work just fine. You can add a Markdown Shell Recorder test for the tutorial by adding the line

add_msr_test (tutorial_crypto "${CMAKE_SOURCE_DIR}/doc/tutorials/crypto.md" 
              REQUIRED_PLUGINS crypto fcrypt)

to the file tests/shell/shell_recorder/tutorial_wrapper/CMakeLists.txt. After that just add shell commands and tests for the output/return values using the Markdown Shell Recorder syntax inside crypto.md. For example, after you copy and paste the following code into crypto.md:

```sh
kdb set user/test/password 1234
#> Create a new key user/test/password with string "1234"
kdb get user/test/password
#> 1234
kdb rm user/test/password
```

, the Shell Recorder will test that

the command kdb set user/test/password 1234 prints the text Create a new key user/test/password with string "1234" to stdout,
the command kdb get user/test/password prints the text 1234 to stdout, and
all three commands return the exit code 0

. To execute the test you can use the following command inside the build directory:

ctest --output-on-failure -V -R testshell_markdown_tutorial_crypto

.

petermax2 · 2018-05-08T19:37:56Z

I would like to use the shell recorder but I see two problems here:

We need to modify the system configuration in order to mount fcrypt. Shell recorder does not like that at all. Try:

	kdb set /sw/elektra/kdb/#0/current/plugins ""
	sudo kdb set system/sw/elektra/kdb/#0/current/plugins ""
	sudo kdb mount test.ini user/test fcrypt "encrypt/key=DDEBEF9EE2DC931701338212DAF635B17F230E8D" ini
	kdb set user/test/password 1234
	#> Create a new key user/test/password with string "1234"
	kdb file user/test/password | xargs cat
	kdb file user/test/password | xargs rm -f
	sudo kdb umount user/test

Having the GPG key available is no problem locally, but it will lead to errors on the build server. Meaning we would have to install the GPG key inside the docker containers and mark them as trusted.

ingwinlu · 2018-05-08T19:51:34Z

sry I accidentally stopped your jenkinsfile build job

ingwinlu · 2018-05-08T19:53:55Z

jenkins build jenkinsfile please

petermax2 · 2018-05-08T19:55:53Z

sry I accidentally stopped your jenkinsfile build job

no problem

sanssecours · 2018-05-09T06:28:35Z

We need to modify the system configuration in order to mount fcrypt. Shell recorder does not like that at all. Try:…

I think we should try to not use rm in Shell Recorder tests whenever possible, especially if we use rm … | xarg, which does not handle spaces in filenames properly, unless we escape them beforehand.

Since Elektra already removes empty configuration files we can use kdb rm instead of rm. The code below shows a slightly modified version of your Markdown Shell Recorder test that works (at least on my machine):

kdb set /sw/elektra/kdb/#0/current/plugins ""
sudo kdb set system/sw/elektra/kdb/#0/current/plugins ""
sudo kdb mount test.ini user/test fcrypt "encrypt/key=14E8E68A868B858E6DE53044630BDC935E2CC0C7" ini
kdb set user/test/password 1234
#> Create a new key user/test/password with string "1234"

# Cleanup
kdb rm user/test/password
kdb rm /sw/elektra/kdb/#0/current/plugins
sudo kdb rm system/sw/elektra/kdb/#0/current/plugins
sudo kdb umount user/test

. Please note that for the test to also work on your computer, you have to replace the key fingerprint above with the fingerprint of one of your keys.

Another problem I noticed was that the tutorial contains indented code blocks. The Markdown Shell Recorder does not handle such code blocks properly! You need to move the block and the code to the left. For an example, please take a look at src/plugins/base64/README.md.

Having the GPG key available is no problem locally, but it will lead to errors on the build server. Meaning we would have to install the GPG key inside the docker containers and mark them as trusted.

Unfortunately my knowledge of GPG and especially Docker is quite limited. Maybe some of the other Elektra developers knows how to handle this situation properly.

ingwinlu · 2018-05-09T06:41:09Z

Unfortunately my knowledge of GPG and especially Docker is quite limited. Maybe some of the other Elektra developers knows how to handle this situation properly.

since the 'private' key does not have to be really private it is trivial to add it to the container.

markus2330 · 2018-05-09T06:48:56Z

Btw. if the docu of the shell recorder has problems it would be great to get fixes there, too.

petermax2 · 2018-05-09T19:27:20Z

Is doc/docker/Dockerfile the correct location for installing the GPG test-key of the crypto plugin?

See ElektraInitiative#1948

ingwinlu · 2018-05-11T19:21:15Z

No it isn't.

Do you really need to bake it into the docker file? Can you not run gpg --import path-to-key-in-repo somewhere in the tutorial?

This reverts commit 4bb95aa.

markus2330 · 2018-05-12T05:40:40Z

No, they were broken since a long time but nobody fixed or disabled them. I disabled them now.

#1963 and #1964 will reintroduce them (as Jenkinsfile).

@petermax2 There are open issues about fcrypt/crypto described in #1973

markus2330 · 2018-05-12T06:24:48Z

I am afraid that the build server is not able to successfully run the tutorial with shell recorder, curl was not available. I installed curl on some of the machines but I am not sure if I got all.

Maybe using wget is more easily available. We already use it in the link checker. (the curlget plugin only uses libcurl but does not need the tool curl)

But why not directly include the private key in the repo and avoid fetching remote content at all?

markus2330 · 2018-05-12T06:26:21Z

Travis fails too but for a different reason: password = 1234
=== FAILED stdout does not match expected pattern password = 1234

https://travis-ci.org/ElektraInitiative/libelektra/jobs/377887442

markus2330 · 2018-05-12T06:26:37Z

jenkins build all please

markus2330 · 2018-05-12T06:29:11Z

Btw. if all of the failing jobs are problems of the build servers (and not bugs in the tutorial), we can include the tutorial without executing shell recorder for this release. But please have a look what the build servers say.

markus2330 · 2018-05-12T06:37:34Z

Thank you! Looks much more robust now!

markus2330 · 2018-05-12T06:37:42Z

jenkins build all please

petermax2 · 2018-05-12T06:39:24Z

Thank you! Looks much more robust now!

I hope the relative path works for all build environments.

sanssecours · 2018-05-12T07:24:23Z

doc/tutorials/crypto.md

+We **DO NOT RECOMMEND** to use our key on your local machine, as it is available to the public!
+
+```sh
+gpg --import ../../src/plugins/crypto/test_key.asc


I think the correct path should be src/plugins/crypto/test_key.asc. The Shell Recorder uses the root of the repo as working directory.

petermax2 · 2018-05-12T09:19:19Z

jenkins build all please

markus2330 · 2018-05-12T11:08:14Z

I documented the shell recorder a bit more in #1980, I hope it is useful. Please review the docu.

@ingwinlu is there something wrong with the Jenkinsfile build job? In GitHub it says it fails but when clicking on "Details" it looks like it did not finish?

If gpg2 is available, fcrypt and crypto will prefer v2. On build servers where v2 is available, the shell recorder will most likely fail, if we import the test key solely for v1.

petermax2 · 2018-05-12T13:15:07Z

jenkins build all please

petermax2 · 2018-05-12T13:53:30Z

@markus2330 it seems like neither gpg nor gpg2 is available in elektra-ini-mergerequest.

gpg2 --import src/plugins/crypto/test_key.asc || gpg --import src/plugins/crypto/test_key.asc

returned with exit code 2.

EDIT: Exit code 2 means that the key file can not be found. Maybe the MSR does not work equally among all builds?

sanssecours · 2018-05-12T14:27:28Z

Maybe the MSR does not work equally among all builds?

I do not think that is the problem. I can reproduce the failure locally, if I use ini as default storage:

cmake ..
        -DKDB_DB_FILE='default.ini' \
	-DKDB_DB_INIT='elektra.ini' \
	-DKDB_DEFAULT_STORAGE=ini

. Everything works fine if I use dump as default storage instead.

Edit: Corrected value for KDB_DEFAULT_STORAGE

petermax2 · 2018-05-12T14:30:40Z

@sanssecours thank you for your help! we should definetly investigate this issue further, but not within the scope of this PR.

Does everyone agree?

sanssecours · 2018-05-12T14:39:41Z

doc/tutorials/crypto.md

+
+```sh
+gpg2 --import src/plugins/crypto/test_key.asc || gpg --import src/plugins/crypto/test_key.asc
+echo "trust-model always" > ~/.gnupg/gpg.conf


I think this line is quite dangerous. This command just overwrote my local GPG configuration.

petermax2 · 2018-05-12T14:41:35Z

@sanssecours I agree. It's fine on the build server but nothing one should execute locally. I will disable the MSR check until the existing issues are resolved.

ingwinlu · 2018-05-12T14:55:47Z

hmm the msr test for crypto should probably set HOME to a tmpdir so no overwrite of config can happen on dev machines

petermax2 · 2018-05-12T15:00:17Z

I opened #1981 for further investigation.

markus2330 · 2018-05-12T15:04:43Z

Thank you for creating the tutorial! Please feel free to create a new PR if you managed to successfully run it with the shell recorder.

petermax2 requested review from sanssecours and markus2330 May 6, 2018 16:04

petermax2 added the work in progress label May 6, 2018

markus2330 reviewed May 6, 2018

View reviewed changes

sanssecours approved these changes May 7, 2018

View reviewed changes

petermax2 added 9 commits May 11, 2018 21:12

crypto: add tutorial

db0a8e3

See ElektraInitiative#1948

crypto: tutorial correction

ef60fd5

crypto: improve tutorial (still needs some love)

cd5e48e

crypto: tutorial corrections

a484bdc

crypto: tutorial corrections

6b846a3

crypto: tutorial - try shell recorder

42e2e33

crypto: tutorial - add shell recorder examples

1e0c5e2

crypto: tutorial - minor improvement

fab1b5a

Docker: add GPG key for crypto/fcrypt unit tests

4bb95aa

petermax2 force-pushed the crypto-tutorial branch from 2e8d8e5 to 4bb95aa Compare May 11, 2018 19:13

petermax2 added 3 commits May 11, 2018 21:23

Revert "Docker: add GPG key for crypto/fcrypt unit tests"

6cecc56

This reverts commit 4bb95aa.

crypto: tutorial - download and install the Elektra key

00481d3

crypto: tutorial - use gpg v1

9019088

markus2330 removed the ready to merge label May 12, 2018

crypto: fix tutorial - try importing the GPG key from the repo

f53cbe9

sanssecours reviewed May 12, 2018

View reviewed changes

crypto: tutorial - fix path of GPG test key

bdbefda

crypto: tutorial - consider gpg2 before gpg while importing the test key

95dc360

If gpg2 is available, fcrypt and crypto will prefer v2. On build servers where v2 is available, the shell recorder will most likely fail, if we import the test key solely for v1.

sanssecours requested changes May 12, 2018

View reviewed changes

crypto: disable MSR check for tutorial

3bae912

sanssecours approved these changes May 12, 2018

View reviewed changes

markus2330 merged commit e26905c into ElektraInitiative:master May 12, 2018

petermax2 deleted the crypto-tutorial branch May 12, 2018 15:05

markus2330 mentioned this pull request May 12, 2018

shell recorder: safe working directory #1982

Closed

markus2330 pushed a commit that referenced this pull request May 12, 2018

doc: add release notes as discussed in #1955

0f05a27


		## Configuration File Encryption/Decryption

		The `fcrypt` plugin enables the encryption and decryption of entire configuration files, thus protecting the confidentiality of the configuration values.


		kdb mount test.ini user/test crypto_gcrypt "crypto/key=DDEBEF9EE2DC931701338212DAF635B17F230E8D" base64 ini

		We recommend adding the `base64` plugin to the backend, because `crypto` will output binary data.

		You can disable the encryption by setting `crypto/encrypt` to a value other than `1`, for example:

		kdb setmeta user/test/password crypto/encrypt 0


		The GPG key we use in this tutorial has the ID `DDEBEF9EE2DC931701338212DAF635B17F230E8D`.

		A GPG private key is mandatory for the plugins to work.


		gpg2 --generate-key

		The `fcrypt` plugin and the `crypto` plugin support both versions (version 1 and version 2) of GPG.


		The command above results in the following content of `test.ini`:

		password = 1234

		An example backend configuration is given as follows:

		sudo kdb mount test.ini user/test crypto_gcrypt "crypto/key=DDEBEF9EE2DC931701338212DAF635B17F230E8D" base64 ini

Crypto tutorial #1955

Crypto tutorial #1955

Conversation

petermax2 commented May 6, 2018 • edited

Purpose

TODO

Checklist

markus2330 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markus2330 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanssecours left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petermax2 commented May 7, 2018

sanssecours commented May 7, 2018

petermax2 commented May 8, 2018

ingwinlu commented May 8, 2018

ingwinlu commented May 8, 2018

petermax2 commented May 8, 2018

sanssecours commented May 9, 2018

ingwinlu commented May 9, 2018

markus2330 commented May 9, 2018

petermax2 commented May 9, 2018

ingwinlu commented May 11, 2018

markus2330 commented May 12, 2018

markus2330 commented May 12, 2018

markus2330 commented May 12, 2018

markus2330 commented May 12, 2018

markus2330 commented May 12, 2018

markus2330 commented May 12, 2018

markus2330 commented May 12, 2018

petermax2 commented May 12, 2018

Choose a reason for hiding this comment

petermax2 commented May 12, 2018

markus2330 commented May 12, 2018

petermax2 commented May 12, 2018

petermax2 commented May 12, 2018 • edited

sanssecours commented May 12, 2018 • edited

petermax2 commented May 12, 2018

Choose a reason for hiding this comment

petermax2 commented May 12, 2018

ingwinlu commented May 12, 2018

petermax2 commented May 12, 2018

markus2330 commented May 12, 2018

petermax2 commented May 6, 2018 •

edited

petermax2 commented May 12, 2018 •

edited

sanssecours commented May 12, 2018 •

edited