Skip to content

Support session data encryption#1750

Merged
aeneasr merged 1 commit intoory:masterfrom
chaopeng:master
Mar 13, 2020
Merged

Support session data encryption#1750
aeneasr merged 1 commit intoory:masterfrom
chaopeng:master

Conversation

@chaopeng
Copy link
Copy Markdown
Contributor

@chaopeng chaopeng commented Mar 4, 2020

Related issue

#1649
@aeneasr

Currently, Hydra stores claims from consent application as json in database. In our case, we will have a field in /userinfo endpoint stores JWT. Similar to Aggregated Claims.

This can be supported by adding claims to id token session in hydra consent application. But hydra will save claims in plain text. Attacker can use the JWT if they grant the database access.

Proposed changes

  1. Add a config flagdb.encrypt_session_data to hydra
  2. If flag is true, encrypt data before writing in database and decrypt after reading from database.
    The encryption is using AES via KeyCipher in RegistryBase.

Checklist

  • I have read the contributing guidelines
  • I have read the security policy
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
    vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)
  • I have documented my changes in the developer guide (if appropriate)

Copy link
Copy Markdown
Member

@aeneasr aeneasr 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! This looks pretty good already. I think that we could also encrypt the session data per default and have a configuration option to disable the behavior if needed.

To make this backwards compatible, we could first try to decode the session string using JSON and if we get a JSON Syntax Error, we decrypt it and then JSON Decode it. Alternatively we could also check if the first and last char are { and } (or null) to check if it is a valid JSON which would be faster.

We would then always write the data encrypted to storage, thus slowly migrating all session to the encrypted representation.

What do you think?

@chaopeng chaopeng force-pushed the master branch 3 times, most recently from bb7399c to a816246 Compare March 6, 2020 04:05
@chaopeng
Copy link
Copy Markdown
Contributor Author

chaopeng commented Mar 6, 2020

Updated, PTAL, Thank you

@chaopeng chaopeng requested a review from aeneasr March 9, 2020 14:34
@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Mar 9, 2020 via email

Copy link
Copy Markdown
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Looks so good! One last thing and ready to merge! Sorry for the late review but I was without a laptop!

@aeneasr
Copy link
Copy Markdown
Member

aeneasr commented Mar 13, 2020

Awesome 🎉

Thank you for your contribution!

@aeneasr aeneasr merged commit caec461 into ory:master Mar 13, 2020
DennisPattmann5012 pushed a commit to DennisPattmann5012/hydra that referenced this pull request Mar 16, 2020
Closes ory#1649

Currently, Hydra stores claims from consent application as a raw JSON string in the database. If a threat agent gains access to the database, he/she will be able to get access to those strings. This patch introduces a new feature which encrypts the raw JSON string in the same way we encrypt JSON Web Keys using AES-GCM.

This patch is backwards compatible and will encrypt old sessions over time when they are used or updated. This does not affect sessions that have already expired and we recommend purging those sessions from the database if you want sessions to be encrypted.

To disable this feature, set `oauth2.session.encrypt_at_rest` to `false`.
eli-zh pushed a commit to eli-zh/hydra that referenced this pull request Mar 22, 2020
Closes ory#1649

Currently, Hydra stores claims from consent application as a raw JSON string in the database. If a threat agent gains access to the database, he/she will be able to get access to those strings. This patch introduces a new feature which encrypts the raw JSON string in the same way we encrypt JSON Web Keys using AES-GCM.

This patch is backwards compatible and will encrypt old sessions over time when they are used or updated. This does not affect sessions that have already expired and we recommend purging those sessions from the database if you want sessions to be encrypted.

To disable this feature, set `oauth2.session.encrypt_at_rest` to `false`.
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.

2 participants