Support encrypted and signed user data #5599

TheRealFalcon · 2024-08-08T20:33:35Z

Proposed Commit Message

feat: Support encrypted and signed user data

Cloud-init user data often contains user secrets including passwords
and private keys. This data has always been submitted in plain text.
To protect this data's confidentiality and guarantee its authenticity,
add the ability to have this data encrypted and signed.

A new user data format is added allowing for an ASCII armored PGP
MESSAGE. If detected, cloud-init will import into a temporary keyring
any keys provided in /etc/cloud/keys and use these keys to decrypt
and/or verify the provided data.

After decryption, the resulting message will be treated as user data
as before.

Additional Context

The squash: commits are just to make reviewing easier. I plan to squash them before merging.

Test Steps

Merge type

Squash merge using "Proposed Commit Message"
Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

TheRealFalcon · 2024-08-08T20:34:08Z

tests/unittests/cloudinit/test_user_data.py

@@ -0,0 +1,129 @@
+"""These tests are for code in the cloudinit.user_data module.


Note that I moved non-PGP related tests here as they fit better here.

TheRealFalcon · 2024-08-08T20:36:09Z

cloudinit/user_data.py

+            # that we attempt to decode said payload so that the transformed
+            # data can be examined.
+            parent_ctype = None
+            if ctype in TRANSFORM_TYPES:


I made a necessary refactor to this section involving setting the mime type of transformed messages. Please double check my logic 🙂 .

I did ensure that gzipped user data still works properly with this code.

cloudinit/user_data.py

TheRealFalcon · 2024-08-08T20:41:35Z

doc/rtd/howto/pgp.rst

@@ -0,0 +1,201 @@
+.. _pgp:


If running this guide, don't be like me and forget to install this branch into the container and then be mad it doesn't work at the end.

holmanb

First pass review

holmanb · 2024-08-13T21:20:29Z

doc/rtd/howto/pgp.rst

+
+Save this file to your working directory as `cloud-config.yaml`.
+
+Encrypt the user data using the public key of the encrypting user and the


Encrypt the user data using the public key of the encrypting user and the private key of the signing user:

The signing key doesn't do encryption. How about:

Encrypt the user data using the public key of the encrypting user and sign it using the private key of the signing user:

holmanb · 2024-08-13T21:21:59Z

doc/rtd/howto/pgp.rst

+using the existing key ring in the snapshot, we do this for a few reasons:
+
+* Users may not want these keys in any key ring by default on a new instance
+* Keys may be exported from any system without needing to be concerned with


I don't understand what this is trying to say.

I can export keys from a system I'm not making an instance from without trying to move gpg keyrings around that may not even be compatible with the target system.

I can wordsmith it.

holmanb · 2024-08-14T21:56:27Z

cloudinit/gpg.py

+    def decrypt(self, data: str) -> str:
+        """Process data using gpg.
+
+        This can be used to decrypt encrypted data, verify signed data,


This only verifies signed data if the data is signed, correct?

IOW, if a user wants their user data to be signed and encrypted, an attacker would need only to acquire the public key of the encryption key and then provide their own unsigned encrypted payload to cloud-init, and cloud-init will not verify it.

Thank you. I pushed a new version that checks that gpg verified the signature if we require the signature.

github-actions · 2024-08-30T00:24:15Z

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

TheRealFalcon · 2024-09-12T19:37:02Z

@holmanb , I have rebased the branch (only minor format.rst conflicts). It should be ready for review again.

holmanb · 2024-09-18T15:58:02Z

cloudinit/gpg.py

+            data=data,
+            update_env=self.env,
+        )
+        if require_signature and "gpg: Good signature" not in result.stderr:


I don't love depending on stderr for this behavior. Are there alternatives that might be better?

Updated. I thought the same thing as I wrote it but also couldn't think of a better idea. Not sure why I didn't think of the verify command sooner.

Rather than an top-level `allow_userdata` key, instead use a `user_data` dict. This is to better align with the top-level `vendor_data` keys.

Cloud-init user data often contains user secrets including passwords and private keys. This data has always been submitted in plain text. To protect this data's confidentiality and guarantee its authenticity, this commit add the ability to have this data encrypted and signed. A new user data format is added allowing for an ASCII armored PGP MESSAGE. If detected, cloud-init will import into a temporary keyring any keys provided in /etc/cloud/keys and use these keys to decrypt and/or verify the provided data. After decryption, the resulting message will be treated as user data as before.

TheRealFalcon · 2024-09-26T20:53:21Z

Rebased away the conflict.

@holmanb , can I get priority on this review? We're getting close to the end of the cycle and I'd like to get this one done so I can focus on my other items with less context switching.

TheRealFalcon commented Aug 8, 2024

View reviewed changes

cloudinit/user_data.py Show resolved Hide resolved

TheRealFalcon commented Aug 8, 2024

View reviewed changes

holmanb reviewed Aug 14, 2024

View reviewed changes

github-actions bot added the documentation This Pull Request changes documentation label Aug 15, 2024

TheRealFalcon force-pushed the encrypted branch 2 times, most recently from 884b803 to 344c3bc Compare August 15, 2024 04:28

github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Aug 30, 2024

github-actions bot closed this Sep 6, 2024

TheRealFalcon reopened this Sep 6, 2024

TheRealFalcon removed the stale-pr Pull request is stale; will be auto-closed soon label Sep 6, 2024

TheRealFalcon force-pushed the encrypted branch from 344c3bc to 36719d8 Compare September 12, 2024 19:36

holmanb self-assigned this Sep 16, 2024

holmanb reviewed Sep 18, 2024

View reviewed changes

TheRealFalcon added 8 commits September 26, 2024 15:47

chore: deprecate 'allow_userdata', add 'user_data'

9f9bcc2

Rather than an top-level `allow_userdata` key, instead use a `user_data` dict. This is to better align with the top-level `vendor_data` keys.

squash: Add require_gpg flag

421fd64

squash: tests and docs

fcf8602

spelling lint

30423a3

comments

7dd84e2

unit test failure and lint

050ba0e

stderr bad

a1186a8

TheRealFalcon force-pushed the encrypted branch from f3be313 to a1186a8 Compare September 26, 2024 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support encrypted and signed user data #5599

Support encrypted and signed user data #5599

TheRealFalcon commented Aug 8, 2024

TheRealFalcon Aug 8, 2024

TheRealFalcon Aug 8, 2024

TheRealFalcon Aug 8, 2024

holmanb left a comment

holmanb Aug 13, 2024

holmanb Aug 13, 2024

TheRealFalcon Aug 15, 2024

holmanb Aug 14, 2024 •

edited

Loading

TheRealFalcon Aug 15, 2024

github-actions bot commented Aug 30, 2024

TheRealFalcon commented Sep 12, 2024

holmanb Sep 18, 2024

TheRealFalcon Sep 18, 2024 •

edited

Loading

TheRealFalcon commented Sep 26, 2024

		@@ -0,0 +1,129 @@
		"""These tests are for code in the cloudinit.user_data module.


		Save this file to your working directory as `cloud-config.yaml`.

		Encrypt the user data using the public key of the encrypting user and the

Support encrypted and signed user data #5599

Are you sure you want to change the base?

Support encrypted and signed user data #5599

Conversation

TheRealFalcon commented Aug 8, 2024

Proposed Commit Message

Additional Context

Test Steps

Merge type

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

holmanb 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

holmanb Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2024

TheRealFalcon commented Sep 12, 2024

Choose a reason for hiding this comment

TheRealFalcon Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

TheRealFalcon commented Sep 26, 2024

holmanb Aug 14, 2024 •

edited

Loading

TheRealFalcon Sep 18, 2024 •

edited

Loading