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

Possible encryption feature? #561

Closed
tgrushka opened this issue Apr 6, 2022 · 13 comments
Closed

Possible encryption feature? #561

tgrushka opened this issue Apr 6, 2022 · 13 comments

Comments

@tgrushka
Copy link

tgrushka commented Apr 6, 2022

Hi @bensheldon , love this gem! I'm interested in encrypting the serialized_params in my app. Was wondering if you've considered that before, and how you might go about it if you were to do it (I'd be willing to give it a try).

At first glance, it looks like the jsonb Postgres queries would be the most affected, and that those attributes could be possibly left unencrypted:

  • job_id
  • job_class
  • executions

I'm using Rails encryption in my app, but also thought of using pgp_sym_encrypt in Postgres since this depends on Postgres anyway, and since you're already requiring the bcrypt extension.

I was also thinking of adding a config option with GOOD_JOB_ENCRYPTION_KEY or similar, and possibly adding an encrypted_serialized_params binary column that would only be populated/used if the encryption key is set.

What are your thoughts?

@bensheldon
Copy link
Owner

@tgrushka interesting! Could you say about why you want to encrypt the parameters?

@tgrushka
Copy link
Author

tgrushka commented Apr 6, 2022

Yes, @bensheldon , I'm using ActiveRecord encryption to ensure my database is encrypted at rest and want to make sure any PII or arguments that I might pass into jobs (i.e. text or e-mail messages) are encrypted. I suppose a better way to do this would be to store the message encrypted in another model and then just pass its ID. Would be nice to guarantee that all the job arguments in the db are just automatically encrypted, though.

e.g. sending e-mails with ActionMailer or text messages with Textris.

@reczy
Copy link
Contributor

reczy commented Apr 6, 2022

Just curious - assuming your db is encrypted at rest and in transit, is there something that you need to additionally protect against? Things showing up in logs? Showing up on the GoodJob dashboard? Something else?

Just to clarify, when you mention that you're using Rails encryption, are you referring to attribute-level encryption independent of your actual db being encrypted?

@tgrushka
Copy link
Author

tgrushka commented Apr 6, 2022

Yes, @reczy , I'm using ActiveRecord::Encryption for now. The GoodJob execution parameters are currently stored in Postgres unencrypted, meaning they are not encrypted in the database. I would like the parameters that don't need to be indexed (i.e. the actual arguments to the executions) to be encrypted.

I'm not suggesting that GoodJob be tied to ActiveRecord::Encryption, just asking what might be a good way to go about adding encryption to the serialized_params (since I tried the following and of course it did not work LOL):

class GoodJobRecord < ApplicationRecord
    self.table_name = 'good_jobs'
    serialize :serialized_params, JSON
    encrypts :serialized_params
    encrypts :error
end

@reczy
Copy link
Contributor

reczy commented Apr 6, 2022

Thanks, that's helpful context.

To dig just a little deeper so I understand - I think what I'm getting at is: are you looking to achieve 2 levels of encryption (1) full database encryption (eg, KMS encryption at rest in AWS RDS) AND (2) attribute level encryption with rails? Or just the latter?

@bensheldon
Copy link
Owner

I think this is behavior that would better live within ActiveJob, than within GoodJob. ActiveJob is responsible for serialization/deserialization of arguments; GoodJob just stores them.

Here's where ActiveJob marshals/unmarshals job arguments: https://github.com/rails/rails/blob/39b7bf7a00fd7abc339573a9f5b62cf0db7a8a9b/activejob/lib/active_job/arguments.rb#L30-L46

I think it would be fairly simple to intervene and encrypt the output before returning from serialize, and decrypt the output before feeding it into deserialize.

e.g.

module EncryptActiveJobArguments
  def serialize(arguments)
    serialized_args = super
    encrypt(serialized_args)
  end
  
  def deserialize(arguments)
    serialized_args = decrypt(arguments)
    super(serialized_args)
  end
end

ActiveJob::Base.prepend(EncryptActiveJobArguments)

@tgrushka
Copy link
Author

tgrushka commented Apr 6, 2022

Sorry about that. I'm not using AWS, just a plain Postgres DAAS. That I believe is encrypted in file storage but I want to encrypt the actual data in the database itself either at the app level or using pgcrypto.

@tgrushka
Copy link
Author

tgrushka commented Apr 6, 2022

Awesome, @bensheldon , I will try that, thank you!

@reczy
Copy link
Contributor

reczy commented Apr 6, 2022

Ah, got it! That makes total sense. Thanks for the follow-up context.

@bensheldon's suggestion looks really clean.

@bensheldon
Copy link
Owner

Just while I'm thinking of it.... 😄

You might want to have your encrypt method return a hash that has some metadata to do allow easier forward compatibility (changing cipher, key rotations, etc.) e.g. have it return something like:

  {
    _encrypted: true,
    cipher: ....,
    key_version: ...,
    encrypted_arguments: ..., 
  }

And I included something like _encrypted too so you could detect in decrypt whether the args themselves were encrypted or not, and pass the non-encrypted values through such that it was backwards compatible with unencrypted values.

...and then publish it as a gem 😉

@tgrushka
Copy link
Author

tgrushka commented Apr 6, 2022

I'm actually trying this now...

module EncryptActiveJobArguments
    def serialize(arguments)
        serialized_args = super
        encrypt(serialized_args)
    end

    def deserialize(arguments)
        serialized_args = decrypt(arguments)
        super(serialized_args)
    end

private

    def encrypt(serialized_args)
        ActiveRecord::Encryption::Encryptor.new.encrypt(serialized_args)
    end

    def decrypt(encrypted_args)
        ActiveRecord::Encryption::Encryptor.new.decrypt(encrypted_args)
    end
end

ActiveJob::Base.prepend(EncryptActiveJobArguments)

@bensheldon
Copy link
Owner

@tgrushka I'm going to close this because I think we arrived at something good. Please comment if I should reopen. And I'd love to know your final implementation.

@tgrushka
Copy link
Author

Yeah, thanks @bensheldon! I just couldn't get it to work easily, then I thought of just encrypting the params I needed, like this, and leaving any models as they are as they're already encrypted:

class AdminMailer < ApplicationMailer
    def signup_alert(encrypted_message:, locality:, success:)
        return if encrypted_message.blank?

        @message = ActiveRecord::Encryption::Encryptor.new.decrypt(encrypted_message)
        return if @message.blank?

        @success = success
        status = success ? 'Success' : 'Failure'

        @subject = "Signup #{status}: #{locality}"

        mail to: App.credentials.settings.admin_email, subject: @subject
    end
end

...

class SignupsController ...
            if @signup.passed_basic_form_validation
                AdminMailer.signup_alert(
                    encrypted_message: ActiveRecord::Encryption::Encryptor.new.encrypt(@signup.to_mail),
                    locality: "#{@signup.locality_name}, #{@signup.state}",
                    success: @signup.persisted?
                ).deliver_later
                AdminTexter.signup_alert(
                    encrypted_message: ActiveRecord::Encryption::Encryptor.new.encrypt(@signup.to_text),
                    success: @signup.persisted?
                ).deliver_later
            end

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

No branches or pull requests

3 participants