Initial (draft) Pull Request: https://github.com/django/django/pull/12814
The pull request at the very least still needs documentation, but would be good to have a review of the implementation first. On Thursday, April 23, 2020 at 11:22:39 AM UTC+2, mark wrote: > > Hey Adam, thanks for the feedback, I'll make sure to credit Chris' > original work in a new PR, I think I'm getting close to having one ready. > > Is there a way to avoid breaking third party backends, but raising >> deprecation warnings? >> > > I could create a new SessionBase child class (something like > *HashingSessionBase*) and make all the default backends inherit from this > class, instead of from *SessionBase* directly. Any sessions backend that > does not inherit from the new *HashingSessionBase* could then be > nominated for deprecation. This will also allow making no/minimal changes > to the *SessionBase* class to optimise backward compatibility. > > > > > On Wednesday, April 15, 2020 at 1:41:00 AM UTC+2, Adam Johnson wrote: >> >> 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 <dr.t...@gmail.com> 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 django-d...@googlegroups.com. >>> 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 django-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/8bc153e4-9422-421e-bd49-4c457942864e%40googlegroups.com.
