Rename salt to mask in CSRF

[Ram Rachum]

Hi guys,

Recently I was working with Django's CSRF protection, customizing it to my needs, and discussing with co-workers exactly how it works and how it has protection against the BREACH attack being used to retrieve the CSRF key.

Relevant code here: https://github.com/django/django/blob/master/django/middleware/csrf.py#L45

One point of confusion is the use of the term salt in Django's source code. People expect salt to mean the same as salt in the database, that works quite differently and doesn't mask the actual secret.

I'm not a security expert so I may be wrong, but I think that "One-time pad", "XOR mask" or just "mask" would be more accurate terms.

I propose to change the "salt" to "mask" everywhere these terms appear in the CSRF code, and similarly "unsalt" to "unmask". As far as I know this wouldn't affect functionality at all, because the term "salt" doesn't appear in actual tokens.

What do you think? 

Ram.

[Matemática A3K]

On Tue, Feb 18, 2020 at 3:31 AM Ram Rachum <ram.r...@gmail.com> wrote:

Hi guys,

Recently I was working with Django's CSRF protection, customizing it to my needs, and discussing with co-workers exactly how it works and how it has protection against the BREACH attack being used to retrieve the CSRF key.

 

https://github.com/django/django/pull/11919#issuecomment-549000592

Relevant code here: https://github.com/django/django/blob/master/django/middleware/csrf.py#L45

One point of confusion is the use of the term salt in Django's source code. People expect salt to mean the same as salt in the database, that works quite differently and doesn't mask the actual secret.

I'm not a security expert so I may be wrong, but I think that "One-time pad", "XOR mask" or just "mask" would be more accurate terms.

I propose to change the "salt" to "mask" everywhere these terms appear in the CSRF code, and similarly "unsalt" to "unmask". As far as I know this wouldn't affect functionality at all, because the term "salt" doesn't appear in actual tokens.

What do you think? 

I think the term is accurate in the domain - https://en.wikipedia.org/wiki/Salt_(cryptography) - no need to rename it.

Ram.

--
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-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/3cf02beb-e292-4991-b75e-2f3f6e28d371%40googlegroups.com.

[Adam Johnson]

The wikipedia page says:

In cryptography, a salt is random data that is used as an additional input to a one-way function that hashes data, a password or passphrase. Salts are used to safeguard passwords in storage.

Ram is right - this variable is not a salt in that definition. We aren't using a one-way hash function, but a reversible rotational cipher function. The 'salt' is really the key/mask/one-time-pad to that cipher function.

I think "mask" and "unmask" are more correct terms to use in the code and docstrings. Not sure if it's worth the effort to change though

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CA%2BFDnhKZdF2QbSy9cL643t9eugzHm6GMaeXtxFVCgHEvYHJwSQ%40mail.gmail.com.

--

Adam

[Ram Rachum]

In any case, if it's decided that it's a worthwhile change, I'll be happy to make the patch.

You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/SokWmLcIkds/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM1Q8zicOH_PvZcSsXGM1fjhVZ2Yq5K6%3D6JT43XG5zqu1Q%40mail.gmail.com.

[charettes]

I think it's a non-invasive worthwhile change and that a PR for it would be well received.

Le mercredi 19 février 2020 11:13:15 UTC-5, Ram Rachum a écrit :

In any case, if it's decided that it's a worthwhile change, I'll be happy to make the patch.

On Wed, Feb 19, 2020 at 10:31 AM Adam Johnson <m...@adamj.eu> wrote:

The wikipedia page says:

In cryptography, a salt is random data that is used as an additional input to a one-way function that hashes data, a password or passphrase. Salts are used to safeguard passwords in storage.

Ram is right - this variable is not a salt in that definition. We aren't using a one-way hash function, but a reversible rotational cipher function. The 'salt' is really the key/mask/one-time-pad to that cipher function.

I think "mask" and "unmask" are more correct terms to use in the code and docstrings. Not sure if it's worth the effort to change though

On Wed, 19 Feb 2020 at 06:39, Matemática A3K <matema...@gmail.com> wrote:

On Tue, Feb 18, 2020 at 3:31 AM Ram Rachum <ram....@gmail.com> wrote:

Hi guys,

Recently I was working with Django's CSRF protection, customizing it to my needs, and discussing with co-workers exactly how it works and how it has protection against the BREACH attack being used to retrieve the CSRF key.

 

https://github.com/django/django/pull/11919#issuecomment-549000592

Relevant code here: https://github.com/django/django/blob/master/django/middleware/csrf.py#L45

One point of confusion is the use of the term salt in Django's source code. People expect salt to mean the same as salt in the database, that works quite differently and doesn't mask the actual secret.

I'm not a security expert so I may be wrong, but I think that "One-time pad", "XOR mask" or just "mask" would be more accurate terms.

I propose to change the "salt" to "mask" everywhere these terms appear in the CSRF code, and similarly "unsalt" to "unmask". As far as I know this wouldn't affect functionality at all, because the term "salt" doesn't appear in actual tokens.

What do you think? 

I think the term is accurate in the domain - https://en.wikipedia.org/wiki/Salt_(cryptography) - no need to rename it.

Ram.

--
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/3cf02beb-e292-4991-b75e-2f3f6e28d371%40googlegroups.com.

--
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/CA%2BFDnhKZdF2QbSy9cL643t9eugzHm6GMaeXtxFVCgHEvYHJwSQ%40mail.gmail.com.

--

Adam

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/SokWmLcIkds/unsubscribe.

To unsubscribe from this group and all its topics, send an email to django-d...@googlegroups.com.

[Adam Johnson]

I guess it's not a very big change so could be worth it to increase readability.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CANXboVbEVQ0d2TmzuKhqDt8t0iQtbgdfdD9MCT7g396ESGaHbw%40mail.gmail.com.

--

Adam

[Ram Rachum]

Cool, PR here:  https://github.com/django/django/pull/12479

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM1LmKcWA%2BBz0Rkiqhx2L7BiS4UZdonFu%3Doq0EMG-KhRPQ%40mail.gmail.com.