[Django] #31358: Increase default password salt size

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
56 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[Django] #31358: Increase default password salt size

Django
#31358: Increase default password salt size
-----------------------------------------+------------------------
               Reporter:  darakian       |          Owner:  nobody
                   Type:  Uncategorized  |         Status:  new
              Component:  Uncategorized  |        Version:  3.0
               Severity:  Normal         |       Keywords:
           Triage Stage:  Unreviewed     |      Has patch:  0
    Needs documentation:  0              |    Needs tests:  0
Patch needs improvement:  0              |  Easy pickings:  0
                  UI/UX:  0              |
-----------------------------------------+------------------------
 I've made a patch for this here
 https://github.com/django/django/pull/12553
 Which changes the default salt size from ~71 bits to ~131 bits

 The rational is that modern guidance suggests a 128 bit minimum on salt
 sizes
 OWASP:
 https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Password_Storage_Cheat_Sheet.md#salting
 NIST:
 https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf

 In the case of NIST this is technically a hard requirement.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/051.a8d04fd75600fd51fe6e8bb06601e49f%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size

Django
#31358: Increase default password salt size
-------------------------------+--------------------------------------
     Reporter:  Jon Moroney    |                    Owner:  nobody
         Type:  Uncategorized  |                   Status:  new
    Component:  Uncategorized  |                  Version:  3.0
     Severity:  Normal         |               Resolution:
     Keywords:                 |             Triage Stage:  Unreviewed
    Has patch:  0              |      Needs documentation:  0
  Needs tests:  0              |  Patch needs improvement:  0
Easy pickings:  0              |                    UI/UX:  0
-------------------------------+--------------------------------------
Description changed by Jon Moroney:

Old description:

> I've made a patch for this here
> https://github.com/django/django/pull/12553
> Which changes the default salt size from ~71 bits to ~131 bits
>
> The rational is that modern guidance suggests a 128 bit minimum on salt
> sizes
> OWASP:
> https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Password_Storage_Cheat_Sheet.md#salting
> NIST:
> https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf
>
> In the case of NIST this is technically a hard requirement.
New description:

 I've made a patch for this here
 https://github.com/django/django/pull/12553
 Which changes the default salt size from ~71 bits to ~131 bits

 The rational is that modern guidance suggests a 128 bit minimum on salt
 sizes
 OWASP:
 https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/Password_Storage_Cheat_Sheet.md#salting
 Python: https://docs.python.org/3/library/hashlib.html#hashlib.pbkdf2_hmac
 NIST:
 https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-132.pdf


 In the case of NIST this is technically a hard requirement.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.5ec9c59e8ec56a6932fbbacb9964cd37%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size. (was: Increase default password salt size)

Django
In reply to this post by Django
#31358: Increase default password salt size.
-------------------------------------+-------------------------------------
     Reporter:  Jon Moroney          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Utilities            |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by felixxm):

 * cc: Florian Apolloner (added)
 * type:  Uncategorized => Cleanup/optimization
 * version:  3.0 => master
 * component:  Uncategorized => Utilities


Comment:

 I'm not sure.This method is not strictly to generate a salt. I would
 rather change a `BasePasswordHasher.salt()` to return
 `get_random_string(22)`.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.d7bd7067976678b23a7ec9d6ac1b6145%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size.

Django
In reply to this post by Django
#31358: Increase default password salt size.
-------------------------------------+-------------------------------------
     Reporter:  Jon Moroney          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Utilities            |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Florian Apolloner):

 In general I like the idea of just increasing `get_random_string` and not
 doing it in X locations as needed. But I fear that a subtle change like
 this might break quite a few systems, so I am with Mariusz to do it just
 for the hasher (not every usage of get_random_string has the same security
 requirements).

 I didn't check, but do we have tests for changing salt lengths and how the
 update works?

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.783364f22be7e3a40bc20d1d127f5559%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size.

Django
In reply to this post by Django
#31358: Increase default password salt size.
-------------------------------------+-------------------------------------
     Reporter:  Jon Moroney          |                    Owner:  nobody
         Type:                       |                   Status:  new
  Cleanup/optimization               |
    Component:  Utilities            |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:                       |             Triage Stage:
                                     |  Unreviewed
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Claude Paroz):

 BTW, I think we should deprecate calling `get_random_string` without a
 length argument, but this may be the subject of a separate ticket?

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.d936c9230936b472da91856daa9336dd%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher. (was: Increase default password salt size.)

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------
Changes (by felixxm):

 * needs_better_patch:  0 => 1
 * has_patch:  0 => 1
 * stage:  Unreviewed => Accepted


Comment:

 Florian, it seems that it's tested only in
 [https://github.com/django/django/blob/master/tests/auth_tests/test_views.py#L1252-L1260
 auth_tests.test_views.ChangelistTests].

 Claude, yes a separate ticket.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.71b7c271696c602e2f41efe9d0c3ab20%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Jon Moroney):

 Thanks for the comments all. I've rebased on the current master and
 changed the return of `BasePasswordHasher.salt` as suggested.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.72a5212980848884ac83fe9d8f3fc543%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Jon Moroney):

 As an aside, would it be possible to get this back ported to the django
 2.2.x branch?

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.f31a1da23fcfae9fd10528fa8f218574%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Florian Apolloner):

 Replying to [comment:5 felixxm]:
 > Florian, it seems that it's tested only in
 [https://github.com/django/django/blob/master/tests/auth_tests/test_views.py#L1252-L1260
 auth_tests.test_views.ChangelistTests].

 Mhm, what does this mean for existing password hashes, will they get
 updated to the new salt length? I get the feeling that the module level
 constant `CRYPTO_SALT_LENGTH` should be an attribute `salt_length` on
 `BasePasswordHasher` and `must_update` should take this into account.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.5877edfbf19bb4309bd5eee210adf4bc%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Jon Moroney):

 Replying to [comment:8 Florian Apolloner]:
 > Replying to [comment:5 felixxm]:
 > > Florian, it seems that it's tested only in
 [https://github.com/django/django/blob/master/tests/auth_tests/test_views.py#L1252-L1260
 auth_tests.test_views.ChangelistTests].
 >
 > Mhm, what does this mean for existing password hashes, will they get
 updated to the new salt length? I get the feeling that the module level
 constant `CRYPTO_SALT_LENGTH` should be an attribute `salt_length` on
 `BasePasswordHasher` and `must_update` should take this into account.

 Would that change `must_update` at the `BasePasswordHasher` level to
 something like
 {{{
     def must_update(self, encoded):
         return self.salt_length == encoded.salt_length
 }}}
 ?
 If so, would that first require an update to go out with the attribute set
 to 12?

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.9419fd7a4eee524d7d73a19aba51ba18%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by felixxm):

 Florian, I checked builtin hashers:

 - `BCryptSHA256PasswordHasher`, `BCryptPasswordHasher`,
 `UnsaltedSHA1PasswordHasher`, `UnsaltedMD5PasswordHasher`,
 `CryptPasswordHasher` are not affected because they override `salt()`,

 - `PBKDF2PasswordHasher`, `PBKDF2SHA1PasswordHasher`,
 `Argon2PasswordHasher`, `SHA1PasswordHasher`, and `MD5PasswordHasher` use
 `BasePasswordHasher.salt()`.

 We should introduce `salt_length` attribute in a separate PR/commit and
 take it into account in `must_update()` for affected hashers. I'm not sure
 how to set `salt_length` for hashers that override `salt()`.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.fef7a29b74467475c68f4c430e1394c0%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Florian Apolloner):

 Replying to [comment:10 felixxm]:
 > We should introduce `salt_length` attribute in a separate PR/commit and
 take it into account in `must_update()` for affected hashers.

 Ok, I am fine with that approach too.

 > I'm not sure how to set `salt_length` for hashers that override
 `salt()`.

 That is a good question indeed. For the unsalted variants we can set it to
 zero just fine and afaik bcrypt also defines it with a fixed length:
 https://github.com/pyca/bcrypt/blob/master/src/bcrypt/__init__.py#L50 and
 is unlikely to change. So we could set `salt_length` everywhere and update
 the hashers to use the builtin `must_update` in addition to their own.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.f84ce6de6853db5a02f90c97c01bce50%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Florian Apolloner):

 Actually handling in `must_update` of the base hasher might be hard,
 because the salt format is specific to the hasher, so we might need to add
 a decode function? *yikes*

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.8769a32399b5f95897ba401f3c4eec4f%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by felixxm):

 I think we can assume that `safe_summary()` returns `salt` and if not then
 salt's length is equal to 0.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:13>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.d17b12a494a7d60b4f1c6508fe889146%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Jon Moroney):

 Replying to [comment:13 felixxm]:
 > I think we can assume that `safe_summary()` returns `salt` and if not
 then salt's length is equal to 0.

 I agree. Though I think it's better to assume that salt's length is
 undefined if the safe_summary dict has no entry. Assuming zero will result
 in 0 != self.salt_length and that will always trigger.

  On my branch I've had to account for two cases to pass the tests
 {{{
     def must_update(self, encoded):
         try:
             return len(self.safe_summary(encoded)['salt']) !=
 self.salt_length
         except (KeyError, NotImplementedError):
             return False
 }}}

 One is just the lack of a salt in the dict, but the second is that not all
 derived classes implement `safe_summary`. I think it would be useful to
 enforce an implementation if possible, but I'm not certain that's possible
 in python and that's probably a separate PR anyway.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:14>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.ceb3502469f35e974bb1e58e71a69705%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Florian Apolloner):

 Replying to [comment:13 felixxm]:
 > I think we can assume that `safe_summary()` returns `salt` and if not
 then salt's length is equal to 0.

 Yes but if it does return a `salt` it doesn't neccessarily tell you
 anything about the length. (It wouldn't be unheard off if a hasher were to
 return a truncated salt there…)

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:15>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.9d6bff424cc45dc775a184a4e88a2889%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Jon Moroney):

 Replying to [comment:15 Florian Apolloner]:
 > Yes but if it does return a `salt` it doesn't neccessarily tell you
 anything about the length. (It wouldn't be unheard off if a hasher were to
 return a truncated salt there…)

 So then is it better to force each hasher to implement it's own
 `must_update` and move the `salt_length`? Variable to the hashers?

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:16>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.ec60350b728be659b803d11b7724b8b5%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Florian Apolloner):

 That is the million dollar question :) Salt is common enough through all
 hashers (compared to something like memory requirements) that it is
 something that could be in the base hasher.

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:17>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.edd60577c287b53a06f029c9d0c0b0d3%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Jon Moroney):

 Let me ask the question a different way then; which route should I
 implement in my pr? :p

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:18>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.838b38db258f7f39ee77a2a640768412%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #31358: Increase default password salt size in BasePasswordHasher.

Django
In reply to this post by Django
#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
     Reporter:  Jon Moroney           |                    Owner:  nobody
         Type:  Cleanup/optimization  |                   Status:  new
    Component:  Utilities             |                  Version:  master
     Severity:  Normal                |               Resolution:
     Keywords:                        |             Triage Stage:  Accepted
    Has patch:  1                     |      Needs documentation:  0
  Needs tests:  0                     |  Patch needs improvement:  1
Easy pickings:  0                     |                    UI/UX:  0
--------------------------------------+------------------------------------

Comment (by Florian Apolloner):

 Replying to [comment:18 Jon Moroney]:
 > Let me ask the question a different way then; which route should I
 implement in my pr? :p

 Ha, sorry for not being clearer. What I wanted to say is that I don't have
 a good answer for you. In a perfect world (ie if you are up to the
 challenge :D) I would suggest adding a `decode` function to the hashers
 that basically does the reverse of `encode`. `safe_summary` could then use
 the decoded values and mask them as needed.

 Adding a `decode` function seems to make sense since
 `Argon2PasswordHasher` already has a `_decode` and others manually repeat
 (the simpler logic) ala `algorithm, empty, algostr, rounds, data =
 encoded.split('$', 4)` over multiple functions.

 This new `decode` functionality could be in a new PR and your current PR
 would be blocked by it and the use that. Interested to hear your and
 Mariusz' thoughts

--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:19>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/066.8adf6ed963771bc9dac8999c663108c1%40djangoproject.com.
123