Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #24556 -- Added note about transport security to passwords documentation #4409

Closed
wants to merge 1 commit into from

Conversation

ssssam
Copy link
Contributor

@ssssam ssssam commented Mar 30, 2015

The 'Password management in Django' page is comprehensive on the subject
of storing passwords in the server. But was missing out a key point
about password security: traffic between client and server needs to be
encrypted when sending user's login details.

Personally, I found the existing documentation so comprehensive that I
thought 'great, someone has thought about all this for me and I don't
need to worry about password security' and forgot all about the need for
HTTPS until someone reminded me (several weeks later). So I think there
needs to be a note on this page about HTTPS.

This is especially true if you require the user to send any sensitive
information using a HTML form, such as login details. With plain HTTP, the
password will be sent in plaintext from the user's computer to your server,
making it vulnerable to 'password sniffing'. With :ref:`HTTPS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap password sniffing in * (italics) instead: "... vulnerable to *password sniffing*. With ..."

The 'Password management in Django' page is comprehensive on the subject
of storing passwords in the server. But was missing out a key point
about password security: traffic between client and server needs to be
encrypted when sending user's login details.

Personally, I found the existing documentation so comprehensive that I
thought 'great, someone has thought about all this for me and I don't
need to worry about password security' and forgot all about the need for
HTTPS until someone reminded me (several weeks later). So I think there
needs to be a note on this page about HTTPS.
@ssssam
Copy link
Contributor Author

ssssam commented Mar 30, 2015

Thanks for the detailed review! I've updated this with your comments.

@shaib
Copy link
Member

shaib commented Mar 30, 2015

I think the mention of HTTPS specifically is not required on a page dealing specifically with passwords. It makes sense to mention that passwords are sensitive information, and so their handling requires attention to security; at that point, I'd rather have a link to the top of topics/security, rather than the middle. XSS or insecure cookies could be just as bad as missing HTTPS.

@MarkusH
Copy link
Member

MarkusH commented Mar 31, 2015

Thinking about that change again I partially agree with @shaib. The focus of this note shouldn't be on HTTPS and only link that section, but HTTPS is the key point when it comes to password security that needs to be considered, in my opinion. I also think that a node might make more sense a bit higher on that page, e.g. right after "Password upgrading".

Suggestion: What do you think about adding something like this:

diff --git a/docs/topics/auth/passwords.txt b/docs/topics/auth/passwords.txt
index 910d08f..d4758b2 100644
--- a/docs/topics/auth/passwords.txt
+++ b/docs/topics/auth/passwords.txt
@@ -192,6 +192,15 @@ when changing the PBKDF2 iteration count.
 .. _bcrypt: http://en.wikipedia.org/wiki/Bcrypt
 .. _`bcrypt library`: https://pypi.python.org/pypi/bcrypt/

+General security considerations
+===============================
+
+.. note::
+
+    Even though your users may use strong passwords, attackers might be able to
+    eavesdrop on their connections. You should :doc:`take your security
+    settings serious </topics/security>`, e.g. run your page over HTTPS.
+

 Manually managing a user's password
 ===================================

@MarkusH
Copy link
Member

MarkusH commented Mar 31, 2015

Since this seems to become a more complex topic than initially though, do you mind opening a ticket on Trac so we can keep track of this change.

@shaib
Copy link
Member

shaib commented Mar 31, 2015

s/serious/seriously/, other than that 👍

@ssssam
Copy link
Contributor Author

ssssam commented Mar 31, 2015

@MarkusH I'm happy with what you've proposed above. I think adding 'In particular, avoid sending passwords or any other sensitive data over HTTP connections because they will be vulnerable to password sniffing.' would still be good to make it stronger.

I've opened a bug: https://code.djangoproject.com/ticket/24556

@MarkusH MarkusH changed the title Add note to passwords documentation recommending the use of HTTPS Fixed #24556 -- Added note about transport security to passwords documentation Mar 31, 2015
@timgraham
Copy link
Member

Do we need a section heading and a note box? Could the section heading be put in the note box? Otherwise, looks fine to me.

@timgraham
Copy link
Member

merged in 1119063, thanks!

@timgraham timgraham closed this Apr 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants