[issue10924] Adding salt and Modular Crypt Format to crypt library.
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 74a1110a3b50 by Christian Heimes in branch 'default': Issue 10924: Fixed mksalt() to use a RNG that is suitable for cryptographic purpose http://hg.python.org/cpython/rev/74a1110a3b50 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Christian Heimes li...@cheimes.de: -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Christian Heimes li...@cheimes.de added the comment: I just found mksalt in the whatsnew section and got curious how you've implemented the function. IMHO it has one major security flaw. The function uses random.choice(). The choice() function generates random values with a Mersenne Twister. However MTs are not suited for any cryptographic purpose and must not be used to generate passwords, session keys or salts. The random.SystemRandom class uses os.urandom() as source which is a wrapper around /dev/urandom or the Windows crypto API. The output is suitable for short living states and salts. I'm going to chance the implementation to a global instance of random.SystemRandom() and _sr.samples() as soon as Georg has cut beta 1. _sr = random.SystemRandom() s += ''.join(_sr.samples(_saltchars, method.salt_chars)) -- assignee: brett.cannon - christian.heimes nosy: +christian.heimes resolution: fixed - status: closed - open type: enhancement - security ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
SilentGhost ghost@gmail.com added the comment: Above-mentioned fix was commited in rev 62994662676a -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
SilentGhost ghost@gmail.com added the comment: Above-mentioned fix was committed in 0586c699d467 and 62994662676a -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by SilentGhost ghost@gmail.com: -- Removed message: http://bugs.python.org/msg130171 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Dave Malcolm dmalc...@redhat.com: -- nosy: +dmalcolm ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Committed in r88500. -- stage: commit review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Ezio Melotti ezio.melo...@gmail.com added the comment: Some buildbots are failing after the commit. Also in the crypt.py module I still see things that according to msg126453 should be fixed already: * more statements on the same line (e.g. if salt == None: salt = mksalt()); * the hardcoded salt values instead of string.ascii_letters; According to the PEP8 there shouldn't be any spaces after the '[' and before the ']' (e.g. method_list = [ METHOD_SHA512, METHOD_SHA256, METHOD_MD5 ] and in the listcomps) and around the = in the function/method declarations/calls (e.g. def crypt(word, salt = None):). -- keywords: +buildbot nosy: +ezio.melotti ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
SilentGhost ghost@gmail.com added the comment: Here is the patch fixing pep-8 compatibility and test. It is against the latest commit. -- nosy: +SilentGhost status: closed - open Added file: http://bugs.python.org/file20840/crypt.py.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Brett Cannon br...@python.org added the comment: I will look at the patch. -- assignee: jafo - brett.cannon nosy: +brett.cannon stage: committed/rejected - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Brett Cannon br...@python.org added the comment: The patch didn't even import as-is or past the tests, but I tweaked it so it did (and made method() just an attribute on the module). -- resolution: accepted - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Thanks. I had just read that a day or so ago, reviewing it for Brett's work. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: At this point I'm going to consider this good to go, and will commit it after the 3.2 final release. Thanks for the review everyone. Of course, I'm open to further suggestions until then, just not expecting any... -- assignee: - jafo keywords: -needs review resolution: - later status: open - pending ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Antoine Pitrou pit...@free.fr added the comment: Actually, the pending stage is only for when things have been committed :) See http://docs.python.org/devguide/triaging.html#triaging -- resolution: later - accepted stage: patch review - commit review status: pending - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Sean Reifschneider j...@tummy.com: Removed file: http://bugs.python.org/file20428/python-underscore_crypt-3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Antoine Pitrou pit...@free.fr added the comment: Can you use diff -u (or simply svn diff) when generating a patch? 6) I don't know, I thought everything in Python 3 was a new style class? It is indeed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Sure thing, here's an svn diff. I had switched to the diff because I couldn't get it to patch into a fresh trunk, but the format looked fine; not sure why it couldn't find the files. Anyway, here's a new version. -- Added file: http://bugs.python.org/file20442/python-underscore_crypt-5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Antoine Pitrou pit...@free.fr added the comment: Sure thing, here's an svn diff. I had switched to the diff because I couldn't get it to patch into a fresh trunk, but the format looked fine; not sure why it couldn't find the files. Anyway, here's a new version. You also have to svn add the relevant files :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Not sure if that was meant to be a suggestion for why my local patching wasn't working from the svn diff output, but obviously -5 was messed up. Here's a new version that I can apply to my fresh trunk and passes make test. If the suggestion was how to fix my patching from svn diff, the problem I ran into was that it had the files in it, say crypt.py, but it was trying to apply them as if I had specified patch -p1, even though the svn diff contained the paths and I hadn't done -p1. Anyway, this diff is diff -urN. I just can't win, I usually use diff -u, but in the distant past Guido asked me for diff -c instead. :-) -- Added file: http://bugs.python.org/file20443/python-underscore_crypt-6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Antoine Pitrou pit...@free.fr added the comment: Not sure if that was meant to be a suggestion for why my local patching wasn't working from the svn diff output, but obviously -5 was messed up. Here's a new version that I can apply to my fresh trunk and passes make test. Thank you! The important is that we now have a workable patch in unified diff format :) If the suggestion was how to fix my patching from svn diff, the problem I ran into was that it had the files in it, say crypt.py, but it was trying to apply them as if I had specified patch -p1, even though the svn diff contained the paths and I hadn't done -p1. For the record, when using svn diff, you have to use patch -p0 to apply the resulting patch. Not -p1. Anyway, this diff is diff -urN. I just can't win, I usually use diff -u, but in the distant past Guido asked me for diff -c instead. :-) I would bet even Guido changed his habits :) Rietveld computes and displays unified diffs as far as I remember. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Thanks for the pointer about patch -p0. I *HAD* tried that, but it didn't seem to work either. I'll double check that though... svn diff is what I'd prefer, because then I can svn commit it when it's ready. Any other review feedback? I'll probably let this sit until 3.2 goes to maintenance and then check it into trunk, so there's some time yet... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Antoine Pitrou pit...@free.fr added the comment: Thanks for the pointer about patch -p0. I *HAD* tried that, but it didn't seem to work either. I'll double check that though... svn diff is what I'd prefer, because then I can svn commit it when it's ready. Ok, it seems the code inside crypt.py is duplicated in your patch. Also, when you commit, it'll be better if you use svn rename for the C file, so that history isn't broken. Any other review feedback? I'll probably let this sit until 3.2 goes to maintenance and then check it into trunk, so there's some time yet... Looks good mostly. Why do you need _MethodListClass()? Executing code at module startup sounds fine to me. Or, at worse, use a global variable. + *salt* (either a random 2 or 16 character string, possibly prefixed with + ``$digit$`` to indicate the method) which will be used to perturb the + encryption algorithm. The characters in *salt* must be in the set + ``[./a-zA-Z0-9]``, with the exception of Modular Crypt Format which + prefixes a ``$digit$``. That paragraph is a bit confusing. Also, other uses of *salt* are described separately two paragraphs above. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Affirmative on the svn mv for the C module. The duplicated code, thanks for pointing that out. Someone else mentioned it, but I didn't understand what they were saying and they didn't reply to my request for clarification. Fixed. On the modules() list, how about if I just make it a list and build it at import time? The class was the way I thought most straightforward to do it as a function, so maybe this is more reasonable? Per the documentation, I pulled down the description from above, which I think captured the uses of *salt* and removed the duplication. -- Added file: http://bugs.python.org/file20449/python-underscore_crypt-7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Sean Reifschneider j...@tummy.com: Removed file: http://bugs.python.org/file20433/python-underscore_crypt-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Sean Reifschneider j...@tummy.com: Removed file: http://bugs.python.org/file20442/python-underscore_crypt-5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Sean Reifschneider j...@tummy.com: Removed file: http://bugs.python.org/file20443/python-underscore_crypt-6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Raymond Hettinger rhettin...@users.sourceforge.net: -- nosy: +rhettinger ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Antoine Pitrou pit...@free.fr added the comment: You forgot to add the new files to your patch. -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Oops, thanks. It's in there now, though for some reason I can't get this patch to apply to trunk, but I'll have to look at that later this afternoon. I wanted to get this new version up in the interim since it definitely does include the Lib/crypt.py file, the heart of the changes. -- Added file: http://bugs.python.org/file20427/python-underscore_crypt-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Sean Reifschneider j...@tummy.com: Removed file: http://bugs.python.org/file20422/python-underscore_crypt.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: I've made a new .patch file using diff -c rather than svn diff. This is the same code, but applies without manual intervention. -- Added file: http://bugs.python.org/file20428/python-underscore_crypt-3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Changes by Sean Reifschneider j...@tummy.com: Removed file: http://bugs.python.org/file20427/python-underscore_crypt-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Nicolas Dumazet nicd...@gmail.com added the comment: Hello, 1) Can you please avoid putting several statements in the same line? 2) wouldnt it be better to compute only once the contents of methods()? I'm not sure that module-initialization time is okay for CPython, but at the very least you can lazily fill a module-level variable, and return it directly from methods()? 3) what happens when a user uses one of the Crypt methods that are referenced from the Module, if this method is not available? Arguably, if I know what I'm doing, I will call mksalt(METHOD_SHA512) without checking that METHOD_SHA512 was in methods(). That's not very intuitive, and it seems that mksalt could break. 4) saltchars should probably be string.ascii_letters+string.digits instead of the hardcoded value 5) you should mention in the documentation that if not salt parameter is given, a different salt will be used for each crypt() call 6) is _MethodClass an old-style class? 7) it seems that the patch duplicates twice the diff of crypt.py, not sure of what happened there? -- nosy: +nicdumz ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
Sean Reifschneider j...@tummy.com added the comment: Thanks for the review. Attached is a new version of the patch. 1) Done. 2) Good point, I didn't think of that. I've changed it into a class that stores the methods list, and made the module methods point to that method on an instance of that class. 3) This entirely depends on the underlying C library implementation of crypt. It won't cause mksalt() to blow up, it's just that the crypt(3) call won't know how to deal with it. On my Linux system using glibc, it simply uses the first two characters as the salt, which isn't entirely surprising except that $ is not a valid salt character according to the standards. 4) I was being lazy and not looking up the locale implications of doing that. They look fine, so I've changed it to use that. Good suggestion. 5) I almost did that, but I figured that, generating a random salt, it was obvious that the return value would be different for the same result. However, since you mentioned it as well, I've added a note. 6) I don't know, I thought everything in Python 3 was a new style class? 7) I don't see that. Perhaps you mis-read Lib/test/test_crypt.py as being another copy of Lib/crypt.py? In any case, I don't see it in v3 or v4 (the one addressing your questions). -- Added file: http://bugs.python.org/file20433/python-underscore_crypt-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10924] Adding salt and Modular Crypt Format to crypt library.
New submission from Sean Reifschneider j...@tummy.com: Over the years I've written the same code over and over to create a random salt string of 2 characters. Worse, the Modular Crypt Format is difficult to find documentation on, so creating stronger hashed passwords is difficult to get right. To that end, I'm proposing the addition of a mksalt() method which will generate a salt, and several METHOD_* values to select which hashing method to use. I also figure there will need to be a methods() call that figures out what methods are available in the library crypt() and return a list of the available ones. If we have a way to generate a salt, then I figure we could drop the salt argument of crypt.crypt(), and if not specified to generate one. So to hash a password you could do: crypt.crypt('password'). I figure that the best way to accomplish this is to implement this all in Python and move the existing C crypt module to _crypt. A patch accomplishing this is attached. Please review. Attached is a patch to accomplish this. -- components: Library (Lib) files: python-underscore_crypt.patch keywords: easy, needs review, patch messages: 126393 nosy: jafo priority: normal severity: normal stage: patch review status: open title: Adding salt and Modular Crypt Format to crypt library. type: feature request versions: Python 3.3 Added file: http://bugs.python.org/file20422/python-underscore_crypt.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10924 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com