1. 8
  1. 23

    There are some pretty egregious issues with this code. Let’s leave aside the plain text storage is the password (storing at least a salted hash is easy), but the use of string interpolation in database queries is bad, especially when using prepared statements is easier!

    Instead of:

    	cursor
    		.execute("""
    		INSERT INTO user
    		(username, password)
    		VALUES ('%s', '%s')
    		"""
    		% (username, password))
    

    You do:

    	cursor
    		.execute("""
    		INSERT INTO user
    		(username, password)
    		VALUES (?, ?)
    		""",
    		(username, password))
    

    It’s best not to take shortcuts like this, even in demonstration code. Not only is it a bad habit, but you can be guaranteed that somebody will copy the code and propagate the error, and in this case the right thing is easier to do than the wrong thing.

    1. 9

      Thanks so much, I don’t know really much of anything about any of these was just getting it working for a project, but boy am I happy to get feedback like this to make it better!

    2. 8
      if realPassword[0] == password
      

      I know this wasn’t the point of the article, but can we please either write pseudo code for this or a safe implementation? :(

      1. 6

        I am going to keep this whole thing really simple. This should NEVER be done in production for authentication.

        This is only for demonstration purposes. Overly simplified to have the core connections exemplified.

        I do agree though, that at least short comment about proper way would be good.

        1. 12

          The right way would be to use the secrets.compare_digest() function. It’s right there in the standard library and has been since 3.6. It’s an alias of hmac.compare_digest(), which has been in the standard library since 3.3.

          1. 5

            More context on why you should do this: if you want to compare two things for security, use compare_digest() function. If you do naive comparison in python, it will stop at the first mismatch character, and therefore, prone to timing attacks.

        2. 3

          I didn’t know how to do a safe implementation but getting these comments, I’m learning quickly. Will make another article on how to do it the right way after learning more!

        3. 3

          Lots of comments here about the safety of the password implementation, and how you might be able to improve it. I’m not sure that would be a great thing to encourage either.

          I really think that in the vast majority of cases, rather than writing a half-hearted example implementation or tolling your own auth, it’s worth the extra dependency to use a reputable library that assists in dealing with password generation and authentication. Especially as someone just getting started with writing server side applications.

          Not at all saying that the topic of authentication and proper password storage isn’t worth learning about, it’s just very much not a thing you should be reasonably expected to totally understand before making websites. Completely valid to use the resources at our disposal.

          1. 2

            Adding an additional page including a salt, hash or scrypt brcrypt algorithm would be really good.

            1. 5

              BLAKE2 is probably a good choice, given it’s in th standard library and easy enough to use. It would require an extra column to store the salt, naturally.

              The method for producing the salted hash in the first place is:

              from hashlib import blake2b
              import os
              
              # ...
              
              salt = os.urandom(blake2b.SALT_SIZE)
              hash = blake2b(pwd, salt=salt).hexdigest()
              

              You can then use either hmac.compare_digest() or secrets.compare_digest() (they’re the same function) to do the comparison securely without any timing information leaking out.

              1. 3

                It’s a shame the standard library has no “password” module with an “hash password(password, version)” function that returns an opaque string blob that contains the hash, salt and version you can then use with “compare password(stored_hash, input)”. You should never have to type the name of some crypto algo name, even less be expected to know how to safely generate, store and compare hash. A generic “safe-enough” standard module would cover 99% of developers need.

                1. 3

                  There’s a middle ground between “developer must manually roll their own” and “standard library does everything”, and it’s “third-party libraries/frameworks implement this, with knowledge of their domain”. Which is really where people ought to be.

                  The standard library provides the constant-time comparison utility, but beyond that does not move fast enough, have enough ability to do hard compatibility breaks, or have enough context to make the choice of the One True KDF For All Use Cases Everywhere. Third-party libraries/frameworks can move fast enough, do have extra context from being closer to specific use cases, and can provide migration paths as needed.

                  1. 2

                    Sounds like secrets