Conversation
aeneasr
left a comment
There was a problem hiding this comment.
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?
bb7399c to
a816246
Compare
|
Updated, PTAL, Thank you |
|
Sorry, traveling right now, will take a look asap
… On 6. Mar 2020, at 01:06, 巢鹏 ***@***.***> wrote:
Updated, PTAL, Thank you
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
aeneasr
left a comment
There was a problem hiding this comment.
Looks so good! One last thing and ready to merge! Sorry for the late review but I was without a laptop!
|
Awesome 🎉 Thank you for your contribution! |
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`.
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`.
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
db.encrypt_session_datato hydraThe encryption is using AES via
KeyCipherinRegistryBase.Checklist
vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.