Hi Mark

Thanks for looking into this tricky security issue.

I'm suggesting to use the names frontend_key and backend_key for these two
> concepts.
>

They seem reasonable to me, as long as there's an explanatory comment.
Perhaps it even needs documenting for third party backends.

My second suggestion is to refactor the SessionBase class to make sure the
> session-key-hashing happens in one place and isn't spread across all
> different backend implementations as is the case now because the subclasses
> have to implemented public methods that receive the frontend_key.
>

Yes that seems reasonable, although I haven't looked closely. Is there a
way to avoid breaking third party backends, but raising deprecation
warnings? Perhaps using func_supports_parameter
<https://github.com/django/django/blob/5b884d45ac5b76234eca614d90c83b347294c332/django/utils/inspect.py#L62>
or similar that we've used in past deprecations?

I see the PR is quite old and not owned by you. If you want to open a new
PR, rebase the existing code, and refactor it as you see fit, you can use
Co-Authored-By
<https://github.blog/2018-01-29-commit-together-with-co-authors/> to
acknowledge Chris' original work.

Thanks,

Adam

On Fri, 10 Apr 2020 at 10:41, mark <[email protected]> wrote:

> After renewed interest because of potential database timing attacks (
> T31412 <https://code.djangoproject.com/ticket/31412>) I'm looking into an
> existing PR (PR8736 <https://github.com/django/django/pull/8736> for
> T21076 <https://code.djangoproject.com/ticket/21076>) for adding the
> possibility of storing hashes of session keys.
>
> I'm looking to get some feedback on two things;
>
> After going through the existing commits of Chris Griffin, I agree with
> Aymeric Augustin (who did an initial review of the pull request) that there
> should be a clearer distinction between the incoming session key (Aymeric
> talks about a "clear text session key") and the key that gets stored in the
> sessions backend (Aymeric talks about a "hashed if needed session key").
> I'm suggesting
> <https://github.com/django/django/pull/8736#issuecomment-610986822> to
> use the names *frontend_key* and *backend_key* for these two concepts.
>
> My second suggestion
> <https://github.com/django/django/pull/8736#issuecomment-611934012> is to
> refactor the *SessionBase* class to make sure the
> session-key-hashing happens in one place and isn't spread across all
> different backend implementations as is the case now because the subclasses
> have to implemented public methods that receive the frontend_key. I'm
> suggesting to basically have subclasses implement private methods that
> receive a backend_key, which will be invoked by the public methods in the
> BaseClass. Obviously this will have consequences for any existing custom
> backends out there, though I think those will be affected either way.
>
> I welcome any thoughts on both the naming convention and the refactoring.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/41c26919-f15f-4151-aa82-1281e24656da%40googlegroups.com
> <https://groups.google.com/d/msgid/django-developers/41c26919-f15f-4151-aa82-1281e24656da%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>


-- 
Adam

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAMyDDM0Bd59HR%3DsvODPA15xvZHtvdcmLzaD49X4K52kMp8g8yQ%40mail.gmail.com.

Reply via email to