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

s3: add server side encryption support #127

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

akunszt
Copy link
Contributor

@akunszt akunszt commented May 24, 2024

This PR enables to use the server side encryption on the s3() destination. It is a common pattern that data stored on the S3 bucket is encrypted at the rest.

S3 supports multiple server side encryption possibilities. This is only for the aws:kms method. In theory aws:kms:dsse should also work but as it uses the same arguments but I didn't test it so I disabled that intentionally.

Check the s3api documentation for the details.

This means that the server-side-encryption() option accepts only aws:kms or an empty string, which is the default.

The kms-key() should be:

  • an ID of a key
  • an alias of a key, but in that case you have to add the alias/ prefix
  • an ARN of a key

For example:

destination d_s3 {
        s3(
                bucket( "log-archive-bucket" )
                object-key( "logs/syslog" )
                server-side-encryption( "aws:kms" )
                kms-key( "alias/log-archive" )
        );
};

To be able to use the aws:kms encryption the AWS Role or User has to have the following permissions on the given key:

  • kms:Decrypt
  • kms:Encrypt
  • kms:GenerateDataKey

Check this page on why the kms:Decrypt is mandatory.

Setting server-side-encryption() to aws:kms without setting kms-key() will terminate the syslog-ng process.

Setting kms-key() without setting server-side-encryption() emits a warning and ignores the kms-key(). The server side encryption will be disabled in this case.

It also worth mentioning that kms-key() cannot be changed after the create_multipart_upload executed. I am not sure what will happen if someone modifies the key and then just reloads the syslog-ng configuration. A normal restart would work as syslog-ng finishes the multipart upload during the shutdown.

Adding the server-side-encryption() and kms-key() options to the s3()
destination.

The server-side-encryption() supports only aws:kms at the moment.

Fixes #4920.

Signed-off-by: Arpad Kunszt <[email protected]>
@bazsi
Copy link
Member

bazsi commented May 25, 2024

I have found just one problem that I pointed out in a review comment above. Other than that this looks good to me.

akunszt added 2 commits May 27, 2024 11:42
With this the process should not fail if the JSON file was created with
an earlier version and the `server-side-encryption` and the `kms-key`
keys are missing.

Signed-off-by: Arpad Kunszt <[email protected]>
As @alltilla pointed out using an old JSON and NOT setting the `kms-key`
and/or `server-side-encryption` settings can cause an issue.

Signed-off-by: Arpad Kunszt <[email protected]>
Copy link
Member

@alltilla alltilla left a comment

Choose a reason for hiding this comment

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

Thank you! :)

alltilla added a commit to alltilla/axosyslog that referenced this pull request Jun 4, 2024
Signed-off-by: Attila Szakacs <[email protected]>
@alltilla alltilla merged commit 8785f7e into axoflow:main Jun 4, 2024
21 checks passed
@akunszt akunszt deleted the issue-4920-s3-server-side-encryption branch June 4, 2024 08:23
@alltilla alltilla mentioned this pull request Jun 4, 2024
alltilla added a commit that referenced this pull request Jun 4, 2024
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