Skip to content

LG-8056 Encrypt document submissions and write them to S3#7351

Merged
jmhooper merged 10 commits intomainfrom
jmhooper-encrypted-document_storage
Nov 17, 2022
Merged

LG-8056 Encrypt document submissions and write them to S3#7351
jmhooper merged 10 commits intomainfrom
jmhooper-encrypted-document_storage

Conversation

@jmhooper
Copy link
Contributor

This commit adds tooling for encrypting documents and writing them to S3 after upload.

This is an addition to the attempts API. Eventually a reference for the image and an encryption key will be shared with IRS via the attempts API. IRS will be able to use that reference and key to request the images associated with a document upload event. The changes to add those values to the attempts API are out of scope for this change and will follow in another commit.

The images are encrypted first with AES-256 using a randomly generated key. The images are then uploaded to an S3 bucket with KMS encryption enabled. This offers protection that matches our current approach to PII storage, but with a partner controlled key instead of the user's password.

This implementation is partner specific. Since the images are only available to service providers that are using the attempts API it should only be enabled when the attempts API is also enabled.

This commit adds tooling for encrypting documents and writing them to S3 after upload.

This is an addition to the attempts API. Eventually a reference for the image and an encryption key will be shared with IRS via the attmepts API. IRS will be able to use that reference and key to request the images associated with a document upload event. The changes to add those values to the attempts API are out of scope for this change and will follow in another commit.

The images are encrypted first with AES-256 using a randomnly generated key. The images are then uploaded to an S3 bucket with KMS encryption enabled. This offers protection that matches our current approach to PII storage, but with a partner controlled key instead of the user's password.

This implementation is partner specific. Since the images are only available to service providers that are using the attempts API it should only be enabled when the attempts API is also enabled.

[skip changelog]
@jmhooper
Copy link
Contributor Author

This is currently WIP and in draft. Opening here early to get early feedback on the design and to create pairing opportunities.

)
end

def store_encrypted_images?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is not great coverage here. The way I'd like to test this is to look for the reference UUIDs that get sent to the fake attempts API and make sure a file was written there. Since these aren't getting bubbled up to the attempts API yet we'll need to either come up with something else temporarily or hold off.

@jmhooper jmhooper marked this pull request as ready for review November 17, 2022 15:54
@jmhooper
Copy link
Contributor Author

I just flipped this to ready to review. There are 2 things that still need to be done 2 get the story done:

1.) Add a test for the controller mentioned in a comment above ☝️
2.) Add the document writing to the background job. I started that and it got messy. I'm going to manage that in a separate PR.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM


def initialize(params, service_provider:, analytics: nil,
uuid_prefix: nil, irs_attempts_api_tracker: nil)
uuid_prefix: nil, irs_attempts_api_tracker: nil, store_encrypted_images: false)
Copy link
Contributor

Choose a reason for hiding this comment

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

another quick thought (maybe follow-up PR material) should also add this to the DocumentProofingJob in case we ever ship async image upload?

Copy link
Contributor

Choose a reason for hiding this comment

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

just saw this :jinx:

2.) Add the document writing to the background job. I started that and it got messy. I'm going to manage that in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A note on the DocumentProofingJob. I just looked for where the info actually gets dropped into the IRS attempts API from there. From what I could tell we don't actually track anything in that job, those events are not visible to the IRS attempts API. I think integrating that may actually be its own story. I'll follow up with @benjaminchait.

Copy link
Contributor

Choose a reason for hiding this comment

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

Related ticket LG-7390

@jmhooper jmhooper merged commit db38500 into main Nov 17, 2022
@jmhooper jmhooper deleted the jmhooper-encrypted-document_storage branch November 17, 2022 18:46
mdiarra3 added a commit that referenced this pull request Nov 21, 2022
* Remove unreachable blank config lockout default logic (#7357)

* Remove unreachable blank config lockout default logic

changelog: Internal, Code Quality, Remove unreachable code paths

* Replace references for removed constant

* Use Rails ActiveSupport for "time ago"

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* LG-8056 Encrypt document submissions and write them to S3 (#7351)

This commit adds tooling for encrypting documents and writing them to S3 after upload.

This is an addition to the attempts API. Eventually a reference for the image and an encryption key will be shared with IRS via the attmepts API. IRS will be able to use that reference and key to request the images associated with a document upload event. The changes to add those values to the attempts API are out of scope for this change and will follow in another commit.

The images are encrypted first with AES-256 using a randomnly generated key. The images are then uploaded to an S3 bucket with KMS encryption enabled. This offers protection that matches our current approach to PII storage, but with a partner controlled key instead of the user's password.

This implementation is partner specific. Since the images are only available to service providers that are using the attempts API it should only be enabled when the attempts API is also enabled.

[skip changelog]

* LG-8139: Increase max OTP confirmation attempts (#7358)

* LG-8139: Increase max OTP confirmation attempts

changelog: Improvements, Multi-factor Authentication, Increase number of allowed MFA confirmation attempts before lock-out

* Replace hard-coded max OTP attempts in specs

* Fix specs, split by max attempts bucket

* LG-8046: stop webauthn platform for new registrations/accounts (#7338)

* changelog: Improvements, Authentication, Disable new registering of platform auth accounts

* default webauthn off for now

* disable webauthn

* change naming convention for feature toggle

* change naming convention

* update webauthn platform

* add feature spec for sign in

* add test to ensure users dont see unneeded adding of platform auth in their account page

* fix html

* update spec and yml file

* remove unneeded spec

* dont show if u dont have face/touch unlock

* update to split up webauthn platform and romaing

* switch roaming and platform

* Drop ial2_quota tables (#7339)

[skip changelog]

* Shannon/lg 7522 update contact strings (#7362)

* update strings and links

* update failed fraud to include correct strings and links

* changelog: Improvements, Results emails, update text

* update reset pw link

* Drop proofing_costs table (LG-8028) (#7346)


[skip changelog]

* Try to fix flakey email spec (#7359)

changelog: Internal, Automated Testing, Improve reliability of successful automated tests

* Fix flakey IPP sample data rake spec (#7363)

* Fix flakey IPP sample data rake spec

changelog: Internal, Automated Testing, Improve reliability of successful automated tests

* Call / stub Kernel.sleep

See: #7363 (comment)

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Add configurable phone carrier registration blocklist (#7366)

changelog: Improvements, Phone Registration, Add configurable phone carrier registration blocklist

* Remove unused PartnerApiReport(#7372)

- Remove associated API code, basically a revert of #5054

changelog: Internal, Reporting, Remove unused reporting code

* Prepare build-sass package for publish (#7370)

* Prepare build-sass package for publish

[skip changelog]

* Re-add private field

Required by linter

* Add README.md

* Add more package.json metadata

* Add LICENSE.md

* Remove files from knapsack report that no longer exist (#7373)

[skip changelog]

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov>
Co-authored-by: Shannon A <20867088+svalexander@users.noreply.github.com>
Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
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