Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encrypt balenaOS artifacts at rest in GitHub #410

Merged
merged 8 commits into from
Oct 1, 2024
Merged

Conversation

ab77
Copy link
Contributor

@ab77 ab77 commented Sep 12, 2024

Applies symmetric encryption (PBKDF2 hardened) to balenaOS build assets prior to uploading them to GitHub for temporary storage between builds. Decrypts assets after downloading.

Requires: https://github.com/balena-os/.github/pull/83
Depends-on: balena-os/balena-generic#525

https://balena.fibery.io/Work/Improvement/Encrypt-artifacts-used-between-jobs-2070

@ab77 ab77 requested a review from klutchell September 12, 2024 21:35
@ab77 ab77 marked this pull request as draft September 12, 2024 23:41
auto-merge was automatically disabled September 12, 2024 23:41

Pull request was converted to draft

@ab77 ab77 force-pushed the ab77/patch branch 2 times, most recently from de6b1a6 to 4988c33 Compare September 13, 2024 00:09
@ab77 ab77 marked this pull request as ready for review September 13, 2024 00:41
@ab77 ab77 temporarily deployed to balena-staging.com September 16, 2024 15:34 — with GitHub Actions Inactive
@klutchell
Copy link
Contributor

How can we make this conditional? Ideally we would only encrypt artifacts if certain conditions are met, like if the repository is public and sign image is enabled, for example.

Also please include a link to the Fibery improvement in the body of the PR.

@ab77 ab77 temporarily deployed to balena-staging.com September 16, 2024 17:56 — with GitHub Actions Inactive
ab77 and others added 7 commits October 1, 2024 10:35
Applies symmetric encryption (PBKDF2 hardened) to balenaOS
build assets prior to uploading them to GitHub for temporary
storage between builds. Decrypts assets after downloading.

Requires: balena-os/.github#83

change-type: patch
@rcooke-warwick
Copy link
Contributor

This is tested and working with both encrypt and decrypt here: https://github.com/balena-os/balena-raspberrypi/actions/runs/11126932142/job/30919311161?pr=1180

@@ -1138,6 +1138,9 @@ jobs:
path: ${{ env.WORKSPACE }}

- name: Decrypt artifacts
if: |
github.event.repository.private != true &&
(inputs.sign-image == true || needs.build.outputs.is_private == 'true')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rcooke-warwick I asked Anton to remove this so we had one fewer place where conditions were used to decide between encryption and decryption. Like if we changed the conditions for encryption and forgot to update the decryption conditions, it might take a while to notice we broke it.

Was it not correctly detecting the lack of *.enc files?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats right,

Run for artifact in *.enc **/*.enc; do
+ for artifact in *.enc **/*.enc
+ openssl enc -v -d -aes-256-cbc -k *** -pbkdf2 -iter 310000 -md sha256 -salt -in '*.enc' -out '*'
bufsize=8192
Can't open "*.enc" for reading, No such file or directory
40D756EFD17F0000:error:80000002:system library:BIO_new_file:No such file or directory:../crypto/bio/bss_file.c:67:calling fopen(*.enc, rb)
40D756EFD17F0000:error:10000080:BIO routines:BIO_new_file:no such file:../crypto/bio/bss_file.c:75:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're too lenient about the logic in this step passing, we risk scenarios like:

  • we break the encryption step
  • we break the decryption step
  • we change the logic to .enc files get written to a different place, or are called something else

And then trying to flash the DUT with whatever, or getting as far as the tests, which will then fail - but its harder to ID where the failure happened if it gets that far. Just to keep in mind

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, sounds good

@flowzone-app flowzone-app bot merged commit 97f7a27 into master Oct 1, 2024
58 checks passed
@flowzone-app flowzone-app bot deleted the ab77/patch branch October 1, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants