AuthTktAuthenticationPolicy using MD5
131 views
Skip to first unread message
Florian Rüchel
Sep 9, 2012, 9:55:34 AM9/9/12
to pylons...@googlegroups.com
I was getting interested in how Pyramid's authentication works and looked through the commonly used AuthTktAuthenticationPolicy code. I found out it uses MD5 and the only thing keeping the cookie from being forged is the secret.
I see two different issues here:
First, MD5 is already known to have weaknesses and it would be a good idea to have different algorithms available so they can be set. This shouldn't be very hard to implement (I can write a patch if you desire) and it can improve the security of any site.
Second, since everything depends on the single secret, I think it should be documented better (communicated on at least the docstring and the documentation) that the secret has to be strong (long, random, maybe state a minimum length).
This sums up my concerns but I will go into detail to give some explanation to my concerns.
I want to start off with saying that I searched the issue tracker, looked at current feature branches, searched this list and searched through google but couldn't find anything that addresses this issue.
Now on to the details:
The basic problem I see is that if you hand over a cookie to a user, he can try to bruteforce (or attack intelligently with MD5 collision attacks and such, see above) and find the secret. This allows him to log in as any user on the system and the complete security of the system can depend on the security of this mechanism. Now if your secret and algorithm are strong enough then you could use this mechanism as a simple but secure approach for authentication. But with a short / unsecure secret your whole system breaks. This is why I recommend better documentation to make sure a deveolper insures a strong secret (that is different for each installation that is deployed) is used in his application. Additionally you could provide help on how to generate such a secret (but that's extra candy). I have looked through various parts of the documentation and it is always set to something like 'seekrit' and similar, but it is never mentioned how to make sure that this is secure.
I get that MD5 is widely spread and easy to deploy, but how about offering an interface to add additional algorithms and document that (of course)? You could just add an additional parameter that, if set, will use the digest algorithm passed to it (e.g. SHA1, SHA256 or maybe even an HMAC where the secret is your key). To furthermore increase the security you could use multiple hashings and key derivation (see bcrypt). Nothing will help if the secret is weak but brute-forcing is made harder if such a mechanism is used.
Of course, the authentication policy should be simple and is not considered the most secure (because for this you would need persisent data storage on the server's side and that should not be a requirement to deploy authentication). But I feel if every possible angle is used, you could at least give developers more power and awareness on their site's security.
Thank you for your attention, I would love to hear some feedback on this. Maybe I am wrong and in this case please say so.
Regards,
Florian
I see two different issues here:
First, MD5 is already known to have weaknesses and it would be a good idea to have different algorithms available so they can be set. This shouldn't be very hard to implement (I can write a patch if you desire) and it can improve the security of any site.
Second, since everything depends on the single secret, I think it should be documented better (communicated on at least the docstring and the documentation) that the secret has to be strong (long, random, maybe state a minimum length).
This sums up my concerns but I will go into detail to give some explanation to my concerns.
I want to start off with saying that I searched the issue tracker, looked at current feature branches, searched this list and searched through google but couldn't find anything that addresses this issue.
Now on to the details:
The basic problem I see is that if you hand over a cookie to a user, he can try to bruteforce (or attack intelligently with MD5 collision attacks and such, see above) and find the secret. This allows him to log in as any user on the system and the complete security of the system can depend on the security of this mechanism. Now if your secret and algorithm are strong enough then you could use this mechanism as a simple but secure approach for authentication. But with a short / unsecure secret your whole system breaks. This is why I recommend better documentation to make sure a deveolper insures a strong secret (that is different for each installation that is deployed) is used in his application. Additionally you could provide help on how to generate such a secret (but that's extra candy). I have looked through various parts of the documentation and it is always set to something like 'seekrit' and similar, but it is never mentioned how to make sure that this is secure.
I get that MD5 is widely spread and easy to deploy, but how about offering an interface to add additional algorithms and document that (of course)? You could just add an additional parameter that, if set, will use the digest algorithm passed to it (e.g. SHA1, SHA256 or maybe even an HMAC where the secret is your key). To furthermore increase the security you could use multiple hashings and key derivation (see bcrypt). Nothing will help if the secret is weak but brute-forcing is made harder if such a mechanism is used.
Of course, the authentication policy should be simple and is not considered the most secure (because for this you would need persisent data storage on the server's side and that should not be a requirement to deploy authentication). But I feel if every possible angle is used, you could at least give developers more power and awareness on their site's security.
Thank you for your attention, I would love to hear some feedback on this. Maybe I am wrong and in this case please say so.
Regards,
Florian
Chris McDonough
Sep 9, 2012, 10:56:03 AM9/9/12
to pylons...@googlegroups.com
On Sun, 2012-09-09 at 06:55 -0700, Florian Rüchel wrote:
> I was getting interested in how Pyramid's authentication works and
> looked through the commonly used AuthTktAuthenticationPolicy code. I
> found out it uses MD5 and the only thing keeping the cookie from being
> forged is the secret.
>
> I see two different issues here:
> First, MD5 is already known to have weaknesses and it would be a good
> idea to have different algorithms available so they can be set. This
> shouldn't be very hard to implement (I can write a patch if you
> desire) and it can improve the security of any site.
> Second, since everything depends on the single secret, I think it
> should be documented better (communicated on at least the docstring
> and the documentation) that the secret has to be strong (long, random,
> maybe state a minimum length).
>
It would be fine by me if we made it possible to change the hashing
> I was getting interested in how Pyramid's authentication works and
> looked through the commonly used AuthTktAuthenticationPolicy code. I
> found out it uses MD5 and the only thing keeping the cookie from being
> forged is the secret.
>
> I see two different issues here:
> First, MD5 is already known to have weaknesses and it would be a good
> idea to have different algorithms available so they can be set. This
> shouldn't be very hard to implement (I can write a patch if you
> desire) and it can improve the security of any site.
> Second, since everything depends on the single secret, I think it
> should be documented better (communicated on at least the docstring
> and the documentation) that the secret has to be strong (long, random,
> maybe state a minimum length).
>
algorithm. But it probably needs to continue to support md5, because
it's purpose is to be compatible with Apache mod_auth_tkt cookies. I
would be happy to accept a patch that allowed folks to plug in a
different hashing algorithm, and explain to them that if they do, it
will no longer be compatible with those cookies.
There are also existing options that can help make it stronger
regardless of the hash, such as including the IP in the token, IIRC.
- C
Domen Kožar
Sep 9, 2012, 11:40:47 AM9/9/12
to pylons...@googlegroups.com
According to https://github.com/gavincarr/mod_auth_tkt/blob/master/conf/02_auth_tkt.conf and
http://linux.die.net/man/3/mod_auth_tkt, mod_auth_tkt supports SHA256 and SHA512 since version 2.1
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
To post to this group, send email to pylons...@googlegroups.com.
To unsubscribe from this group, send email to pylons-devel...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/pylons-devel?hl=en.
Chris McDonough
Sep 9, 2012, 11:45:56 AM9/9/12
to pylons...@googlegroups.com
On Sun, 2012-09-09 at 17:40 +0200, Domen Kožar wrote:
> According
> to https://github.com/gavincarr/mod_auth_tkt/blob/master/conf/02_auth_tkt.conf and
> http://linux.die.net/man/3/mod_auth_tkt, mod_auth_tkt supports SHA256
> and SHA512 since version 2.1
>
>
> Relevant: https://bitbucket.org/ianb/paste/changeset/7f90a96378ed\
> According
> to https://github.com/gavincarr/mod_auth_tkt/blob/master/conf/02_auth_tkt.conf and
> http://linux.die.net/man/3/mod_auth_tkt, mod_auth_tkt supports SHA256
> and SHA512 since version 2.1
>
>
Cool. We should do something similar I guess.
> For more options, visit this group at
> http://groups.google.com/group/pylons-devel?hl=en.
>
>
>
>
>
> http://groups.google.com/group/pylons-devel?hl=en.
>
>
>
>
>
> --
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To post to this group, send email to pylons...@googlegroups.com.
> To unsubscribe from this group, send email to pylons-devel
> +unsub...@googlegroups.com.
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To post to this group, send email to pylons...@googlegroups.com.
> To unsubscribe from this group, send email to pylons-devel
Domen Kožar
Sep 9, 2012, 2:23:43 PM9/9/12
to pylons...@googlegroups.com
Florian: do you plan to provide a patch?
To unsubscribe from this group, send email to pylons-devel...@googlegroups.com.
Daniel Holth
Sep 9, 2012, 2:25:44 PM9/9/12
to pylons...@googlegroups.com
I dislike md5 as much as the next guy, but auth_tkt uses a double hashing scheme that is almost hmac. Hmac overcomes most of the problems of an otherwise weak hash function. It isn't as bad as you might think.
The sha2 functions are a great replacement. Sha2 auth_tkt is what I would use. Sha1 is discouraged these days. You don't need bcrypt because the secret you are protecting is very long. Password hashing functions are not MACs.
uuids make good auth_tkt secrets.
Florian Rüchel
Sep 9, 2012, 3:25:38 PM9/9/12
to pylons...@googlegroups.com
On Sunday, September 9, 2012 8:23:45 PM UTC+2, Domen Kožar wrote:
Florian: do you plan to provide a patch?
I am willing to provide a patch but I am new to pyramid and would definitely need someone to double check which places need changing. For example we need a dynamic split depending on the algorithm used as otherwise we cannot determine where the hash ends and the timestamp begins. Furthermore, I don't know which other parts might use the digest parts (so they would need change, too). And last, I haven't looked into tests yet. If they cover this, they should at least be extended to also test a different algorithm.
If someone is willing to help me, then yes, I would like to develop this. It might take some time though, so if someone is eager to get this quickly (like by the end of this week, i.e. on Sunday, 16th), then I am probably not the right guy right know. I could probably do it by the end of next week)
Regards,
Florian
Chris McDonough
Sep 9, 2012, 3:47:51 PM9/9/12
to pylons...@googlegroups.com
On Sun, 2012-09-09 at 12:25 -0700, Florian Rüchel wrote:
>
>
> On Sunday, September 9, 2012 8:23:45 PM UTC+2, Domen Kožar wrote:
> Florian: do you plan to provide a patch?
>
> I am willing to provide a patch but I am new to pyramid and would
> definitely need someone to double check which places need changing.
> For example we need a dynamic split depending on the algorithm used as
> otherwise we cannot determine where the hash ends and the timestamp
> begins. Furthermore, I don't know which other parts might use the
> digest parts (so they would need change, too). And last, I haven't
> looked into tests yet. If they cover this, they should at least be
> extended to also test a different algorithm.
>
> If someone is willing to help me, then yes, I would like to develop
> this. It might take some time though, so if someone is eager to get
> this quickly (like by the end of this week, i.e. on Sunday, 16th),
> then I am probably not the right guy right know. I could probably do
> it by the end of next week)
I don't think there's any hurry.
>
>
> On Sunday, September 9, 2012 8:23:45 PM UTC+2, Domen Kožar wrote:
> Florian: do you plan to provide a patch?
>
> I am willing to provide a patch but I am new to pyramid and would
> definitely need someone to double check which places need changing.
> For example we need a dynamic split depending on the algorithm used as
> otherwise we cannot determine where the hash ends and the timestamp
> begins. Furthermore, I don't know which other parts might use the
> digest parts (so they would need change, too). And last, I haven't
> looked into tests yet. If they cover this, they should at least be
> extended to also test a different algorithm.
>
> If someone is willing to help me, then yes, I would like to develop
> this. It might take some time though, so if someone is eager to get
> this quickly (like by the end of this week, i.e. on Sunday, 16th),
> then I am probably not the right guy right know. I could probably do
> it by the end of next week)
- C
Florian Rüchel
Sep 9, 2012, 6:10:51 PM9/9/12
to pylons...@googlegroups.com
I have a question on tests:
Four tests in "pyramid.tests.test_authentication.TestAuthTktCookieHelper" worry me: They do something like
Unfortunately, I don't understand the purpose of those tests. Thus, I can't fix them without risking breaking the test. What do tests like "test_remember_binary_userid" do? Could I just remove the "hashalg" key from the dict or should I fix this another way.
Note on progress: I took myself some time and started implementing it. When I was done, I ran the tests. Where they failed, I looked into the code and set the hashalg to md5 where appropriate. Thus, I think if all tests pass, I should have insured the old system is not broken in any way. Next step then: I will create tests for the new mechanism (for which I need help then).
On functionality: I decided to add a new parameter called 'hashalg'. It accepts either a new from the hashlib suite (like 'sha256' in all notations like 'SHA-256', 'SHA256' or 'sha-256'), a dotted python name (like 'hashlib.sha256') or the function itself (e.g. hashalg=hashlib.sha256). Requirement for anything passed is that the resulting object can be used like this: hashalg("string").hexdigest() and it must be able to operate on an empty string (to determine the length) and of course it must return a fixed length. HMAC is not supported (it would require more complex methods to pass a key and a string instead of just a string).
Current progress can be found on github, but I am not done yet and there may be errors. If you already see some flaws in the concept please point them out. But you may as well just wait until I report that I am done ;)
Four tests in "pyramid.tests.test_authentication.TestAuthTktCookieHelper" worry me: They do something like
values = self._parseHeaders(result)
val = self._cookieValue(values[0])
Which fails withval = self._cookieValue(values[0])
return eval(cookie.value)
{'secure': False, 'remote_addr': '0.0.0.0', 'cookie_name': 'auth_tkt', 'userid': 'dXNlcmlk', 'user_data': 'userid_type:b64str', 'hashalg': <built-in function openssl_md5>, 'tokens': (), 'secret': 'secret'}
SyntaxError: invalid syntaxUnfortunately, I don't understand the purpose of those tests. Thus, I can't fix them without risking breaking the test. What do tests like "test_remember_binary_userid" do? Could I just remove the "hashalg" key from the dict or should I fix this another way.
Note on progress: I took myself some time and started implementing it. When I was done, I ran the tests. Where they failed, I looked into the code and set the hashalg to md5 where appropriate. Thus, I think if all tests pass, I should have insured the old system is not broken in any way. Next step then: I will create tests for the new mechanism (for which I need help then).
On functionality: I decided to add a new parameter called 'hashalg'. It accepts either a new from the hashlib suite (like 'sha256' in all notations like 'SHA-256', 'SHA256' or 'sha-256'), a dotted python name (like 'hashlib.sha256') or the function itself (e.g. hashalg=hashlib.sha256). Requirement for anything passed is that the resulting object can be used like this: hashalg("string").hexdigest() and it must be able to operate on an empty string (to determine the length) and of course it must return a fixed length. HMAC is not supported (it would require more complex methods to pass a key and a string instead of just a string).
Current progress can be found on github, but I am not done yet and there may be errors. If you already see some flaws in the concept please point them out. But you may as well just wait until I report that I am done ;)
Domen Kožar
Sep 13, 2012, 6:18:19 AM9/13/12
to pylons...@googlegroups.com
Hi Florian,
It's probably enough to support "MD5", "SHA256" and "SHA512". Maybe just do case insensitive comparison and default to "MD5". I wouldn't complicate too much, as long as we can get the benefit of being compatible with specs and modern crypto.
my 2 cents, cheers Domen
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
To view this discussion on the web visit https://groups.google.com/d/msg/pylons-devel/-/OUp_z4YLZLAJ.
Florian Rüchel
Sep 14, 2012, 3:37:07 PM9/14/12
to pylons...@googlegroups.com
Hi Domen,
I would agree with you if it would really complicate things much, but I can't see why it would. All those cases are really simple to implement and I don't see any security risks either as the configuration of the hash algorithm is under full control of the application developer using it. However, if Chris disagrees, I can remove all those features and really restrict it to basic algorithms (in this case I would prefer to offer all those currently supported by hashlib.
Now on to the work: I have finished the code and also implemented some new tests to assure it works. But here is where I will now need help from more experienced developers: How can I make sure I have full coverage of all situations? This seems to be the most tricky part. In fact, I think I should have covered all parts but this is because I assume that there is no interaction between the hashalg and the parameters I put in (e.g. should I test with and without tokens explicitly? I chose not to).
Before I submit a patch for pyramid on github, I would kindly ask for some code review by experienced developers. Under https://github.com/Javex/pyramid/tree/feature.auth_multiple_hashalgs you will find my cloned repository's feature branch. Please have a look at it, maybe clone it and tell me what I possibly missed. If feedback is positive, I will submit a pull request to pyramid.
Please note: I consider this finished, so I now await feedback.
Regards,
Florian
I would agree with you if it would really complicate things much, but I can't see why it would. All those cases are really simple to implement and I don't see any security risks either as the configuration of the hash algorithm is under full control of the application developer using it. However, if Chris disagrees, I can remove all those features and really restrict it to basic algorithms (in this case I would prefer to offer all those currently supported by hashlib.
Now on to the work: I have finished the code and also implemented some new tests to assure it works. But here is where I will now need help from more experienced developers: How can I make sure I have full coverage of all situations? This seems to be the most tricky part. In fact, I think I should have covered all parts but this is because I assume that there is no interaction between the hashalg and the parameters I put in (e.g. should I test with and without tokens explicitly? I chose not to).
Before I submit a patch for pyramid on github, I would kindly ask for some code review by experienced developers. Under https://github.com/Javex/pyramid/tree/feature.auth_multiple_hashalgs you will find my cloned repository's feature branch. Please have a look at it, maybe clone it and tell me what I possibly missed. If feedback is positive, I will submit a pull request to pyramid.
Please note: I consider this finished, so I now await feedback.
Regards,
Florian
Chris McDonough
Sep 14, 2012, 6:06:16 PM9/14/12
to pylons...@googlegroups.com
This is a lot more code that I thought it would be. A few things:
- You dont need to implement your own dotted name resolver. We have
one of those in pyramid (named DottedNameResolver or something).
- You probably dont even need to resolve dotted names. I'd just
make people pass the callable that returns a new hash or something,
don't bother with trying to figure out what they mean, just make
them pass a single kind of thing.
- C
> *Regards,
> Florian
>
> --
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/pylons-devel/-/QJfM_vSmF-cJ.
> --
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To view this discussion on the web visit
Daniel Holth
Sep 17, 2012, 10:40:21 AM9/17/12
to pylons...@googlegroups.com
On Friday, September 14, 2012 6:06:15 PM UTC-4, Chris McDonough wrote:
"valid argument to hashlib.new()" is good enough. Then you just make the default
def fn(hashalg='md5')
All simple.
Florian Rüchel
Sep 17, 2012, 12:43:11 PM9/17/12
to pylons...@googlegroups.com
I have reworked my code with your comments. I now only support callables, I changed the documentation and removed the now invalid tests. Everything seems to run just fine, so I merged all commits into one which you can now easily review. I hope this suits your needs more.
Please let me know if you need anything else.
Regards,
Florian
Please let me know if you need anything else.
Regards,
Florian
Michael Merickel
Sep 17, 2012, 12:56:28 PM9/17/12
to pylons...@googlegroups.com
In an effort to maintain compliance with the auth_tkt standard versus
flexibility I would've rather seen this just accept the strings "md5",
"sha256", etc rather than an arbitrary callable that may or may not be
compliant.
My 2 cents.
flexibility I would've rather seen this just accept the strings "md5",
"sha256", etc rather than an arbitrary callable that may or may not be
compliant.
My 2 cents.
> --
> You received this message because you are subscribed to the Google Groups
> "pylons-devel" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/pylons-devel/-/PZGqsHed7HIJ.
> You received this message because you are subscribed to the Google Groups
> "pylons-devel" group.
> To view this discussion on the web visit
Daniel Holth
Sep 17, 2012, 1:05:56 PM9/17/12
to pylons...@googlegroups.com
Clearly in that case you are on your own.
Wichert Akkerman
Sep 18, 2012, 3:37:14 AM9/18/12
to pylons...@googlegroups.com
I would prefer to use strings instead of callables as well actually.
On 9/17/12 19:05 , Daniel Holth wrote:
> Clearly in that case you are on your own.
>
> On Sep 17, 2012 12:57 PM, "Michael Merickel" <mmer...@gmail.com
> <mailto:pylons-devel%2Bunsu...@googlegroups.com>.
> <mailto:pylons-devel%2Bunsu...@googlegroups.com>.
On 9/17/12 19:05 , Daniel Holth wrote:
> Clearly in that case you are on your own.
>
> On Sep 17, 2012 12:57 PM, "Michael Merickel" <mmer...@gmail.com
> <mailto:mmer...@gmail.com>> wrote:
>
> In an effort to maintain compliance with the auth_tkt standard versus
> flexibility I would've rather seen this just accept the strings "md5",
> "sha256", etc rather than an arbitrary callable that may or may not be
> compliant.
>
> My 2 cents.
>
>
> In an effort to maintain compliance with the auth_tkt standard versus
> flexibility I would've rather seen this just accept the strings "md5",
> "sha256", etc rather than an arbitrary callable that may or may not be
> compliant.
>
> My 2 cents.
>
> On Mon, Sep 17, 2012 at 11:43 AM, Florian R�chel
> <florian...@gmail.com <mailto:florian...@gmail.com>> wrote:
> > I have reworked my code with your comments. I now only support
> callables, I
> > changed the documentation and removed the now invalid tests.
> Everything
> > seems to run just fine, so I merged all commits into one which
> you can now
> > easily review. I hope this suits your needs more.
> >
> > Please let me know if you need anything else.
> >
> >
> > Regards,
> > Florian
> >
> > --
> > You received this message because you are subscribed to the
> Google Groups
> > "pylons-devel" group.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msg/pylons-devel/-/PZGqsHed7HIJ.
> >
> > To post to this group, send email to
> pylons...@googlegroups.com <mailto:pylons...@googlegroups.com>.
> <florian...@gmail.com <mailto:florian...@gmail.com>> wrote:
> > I have reworked my code with your comments. I now only support
> callables, I
> > changed the documentation and removed the now invalid tests.
> Everything
> > seems to run just fine, so I merged all commits into one which
> you can now
> > easily review. I hope this suits your needs more.
> >
> > Please let me know if you need anything else.
> >
> >
> > Regards,
> > Florian
> >
> > --
> > You received this message because you are subscribed to the
> Google Groups
> > "pylons-devel" group.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msg/pylons-devel/-/PZGqsHed7HIJ.
> >
> > To post to this group, send email to
> <mailto:pylons-devel%2Bunsu...@googlegroups.com>.
> > For more options, visit this group at
> > http://groups.google.com/group/pylons-devel?hl=en.
>
> --
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To post to this group, send email to pylons...@googlegroups.com
> <mailto:pylons...@googlegroups.com>.
> > http://groups.google.com/group/pylons-devel?hl=en.
>
> --
> You received this message because you are subscribed to the Google
> Groups "pylons-devel" group.
> To post to this group, send email to pylons...@googlegroups.com
> <mailto:pylons-devel%2Bunsu...@googlegroups.com>.
Jonathan Vanasco
Sep 21, 2012, 1:50:46 PM9/21/12
to pylons...@googlegroups.com
Additionally you could provide help on how to generate such a secret (but that's extra candy). I have looked through various parts of the documentation and it is always set to something like 'seekrit' and similar, but it is never mentioned how to make sure that this is secure.
fwiw, wordpress has had this feature for a while: the docs instruct you to visit the following url , which generates valid secret strings.
i think most of the application scaffolds will generate a secret - but it might be useful feature to just have a secret-key generator on the pylonsproject.org site and referenced in the docs.
I've also generally disliked the mod_authtkt for a few years. i've opted for an approach where the secret rotates based on the timestamp and/or ip/other data. it's a bit harder to set up in a clustered environment, but the tickets are HMAC with SHA512 with rotating keys. it doesn't make it unbreakable, but just a bit more of pain to break and with some sort of timed window before you need to break it again.
Domen Kožar
Sep 21, 2012, 1:54:34 PM9/21/12
to pylons...@googlegroups.com
For generating secrets it's important to discourage usage of random module, but use something like:
secret = ''.join('%02x' % ord(x) for x in os.urandom(128))
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
To view this discussion on the web visit https://groups.google.com/d/msg/pylons-devel/-/4dxNXUSoAPAJ.
Daniel Holth
Sep 21, 2012, 2:26:00 PM9/21/12
to pylons...@googlegroups.com
On Fri, Sep 21, 2012 at 1:54 PM, Domen Kožar <do...@dev.si> wrote:
> For generating secrets it's important to discourage usage of random module,
> but use something like:
>
> secret = ''.join('%02x' % ord(x) for x in os.urandom(128))
Great wordpress site! Secrets transmitted in the clear without mandatory SSL.
> For generating secrets it's important to discourage usage of random module,
> but use something like:
>
> secret = ''.join('%02x' % ord(x) for x in os.urandom(128))
My apps use a generic key/value settings table and a function to
generate a named secret if it is missing. It is easy.
https://bitbucket.org/dholth/stucco_auth/src/8d5faddc8ff9/stucco_auth/__init__.py#cl-49
128 bytes (1024 bits) is massive overkill. 16 or 32 bytes (128/256
bits) is enough of a secret. 256-bit hashes are enough.
HMAC-SHA256 or HMAC-SHA512 are absolutely unbreakable given that you
maintain the secrecy of the key. 2**128 operations to brute force
minimum. The other attack vector is of course firesheep (capturing the
cookie). If you do plan on losing the secret, by all means rotate it.
I don't think even auth_tkt with md5 is insecure, due to the double
hashing, but it is a good idea to switch to sha256.
There's a mod_auth_hmac if you can read Japanese.
Domen Kožar
Sep 21, 2012, 2:29:45 PM9/21/12
to pylons...@googlegroups.com
Agreed. For that reason, I'd just use session authentication policy and TLS without decompression.
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
Vlad K.
Sep 21, 2012, 8:33:48 PM9/21/12
to pylons...@googlegroups.com
On 09/21/2012 07:50 PM, Jonathan Vanasco wrote:
> i think most of the application scaffolds will generate a secret - but
> it might be useful feature to just have a secret-key generator on
> the pylonsproject.org site and referenced in the docs.
>
How about a script that's part of the framework itself? We have pserve,
> i think most of the application scaffolds will generate a secret - but
> it might be useful feature to just have a secret-key generator on
> the pylonsproject.org site and referenced in the docs.
>
pcreate... how about
pkeygen [-w <filename>]
or
pyramid-keygen [-w <filename>]
I prefer those secrets to exist outside of the code anyway, in separate
file loaded by the app on start. I don't want them visible by teh GitHub
staff. :)
--
.oO V Oo.
Work Hard,
Increase Production,
Prevent Accidents,
and Be Happy! ;)
Florian Rüchel
Sep 23, 2012, 8:54:16 AM9/23/12
to pylons...@googlegroups.com, vl...@haronmedia.com
How about a script that's part of the framework itself? We have pserve,
pcreate... how about
pkeygen [-w <filename>]
or
pyramid-keygen [-w <filename>]
I like this idea very much. I would like to either get this usage approved or I would just build a simple function inside pyramid. However, such a function belongs more into an installation than into application code. Can you tell me how to build such a script that runs on both Windows and Linux? I would like to see it implemented in this way if Chris approves.
On a seperate note: I have started on improving the documentation. As a first step, I have edited the `narr/authentication.rst` to include a note and have documented the API for `pyramid.authentication.AuthTktAuthenticationPolicy` (better documentation for secret, add documentation for hashalg). My question is now how would you handle this in regards to the documentation. I thought about adding this (or a similar) note everywhere this policy is used. This should raise the awareness everywhere the docs are read (e.g. tutorials). Furthermore, since we would clearly recommend to use something like SHA256 if MD5 is not explicitly needed, should we change the code examples to include a better hashalg (instead of just documenting it)? I would vote for a yes, since I don't see any disadvantage: If you build a new application, you should always use another algorithm and as shown above mod_auth_tkt can also easily handle other algorithms if configured correctly.
I would like to hear some opinions on this matter before I start to make big changes and only end up reverting them because you don't like it. My first version can be found here: https://github.com/Javex/pyramid/commit/549db4b02cbff2c511eb026d3a5856b0b8fb3236
I have also created a small `gensecret` function based on the ideas of Daniel and Domen (but with added Python3 compatibility): https://github.com/Javex/pyramid/commit/d4f2943fa50e34f682f8097dccee2ce3ef1e998e
This function is not what I expect in the final version but it shows where I would like to go with this: Provide a function that makes it easier for a user to obtain a strong secret. Either we use it this way or the above mentioned seperate script, that really doesn't matter.
Please tell me your thoughts on both topics.
Regards,
Florian
Chris McDonough
Sep 23, 2012, 9:11:48 AM9/23/12
to pylons...@googlegroups.com, vl...@haronmedia.com
On Sun, 2012-09-23 at 05:54 -0700, Florian Rüchel wrote:
>
> How about a script that's part of the framework itself? We
> have pserve,
> pcreate... how about
>
> pkeygen [-w <filename>]
>
> or
>
> pyramid-keygen [-w <filename>]
>
> I like this idea very much. I would like to either get this usage
> approved or I would just build a simple function inside pyramid.
> However, such a function belongs more into an installation than into
> application code. Can you tell me how to build such a script that runs
> on both Windows and Linux? I would like to see it implemented in this
> way if Chris approves.
Who will use it and when would they use it?
>
> How about a script that's part of the framework itself? We
> have pserve,
> pcreate... how about
>
> pkeygen [-w <filename>]
>
> or
>
> pyramid-keygen [-w <filename>]
>
> I like this idea very much. I would like to either get this usage
> approved or I would just build a simple function inside pyramid.
> However, such a function belongs more into an installation than into
> application code. Can you tell me how to build such a script that runs
> on both Windows and Linux? I would like to see it implemented in this
> way if Chris approves.
> On a seperate note: I have started on improving the documentation. As
> a first step, I have edited the `narr/authentication.rst` to include a
> note and have documented the API for
> `pyramid.authentication.AuthTktAuthenticationPolicy` (better
> documentation for secret, add documentation for hashalg). My question
> is now how would you handle this in regards to the documentation. I
> thought about adding this (or a similar) note everywhere this policy
> is used. This should raise the awareness everywhere the docs are read
> (e.g. tutorials). Furthermore, since we would clearly recommend to use
> something like SHA256 if MD5 is not explicitly needed, should we
> change the code examples to include a better hashalg (instead of just
> documenting it)? I would vote for a yes, since I don't see any
> disadvantage: If you build a new application, you should always use
> another algorithm and as shown above mod_auth_tkt can also easily
> handle other algorithms if configured correctly.
The last patch I saw seemed maybe a little overwrought. Until we figure
that out, I'd hold off on changing docs.
> I would like to hear some opinions on this matter before I start to
> make big changes and only end up reverting them because you don't like
> it. My first version can be found here:
> https://github.com/Javex/pyramid/commit/549db4b02cbff2c511eb026d3a5856b0b8fb3236
>
> I have also created a small `gensecret` function based on the ideas of
> Daniel and Domen (but with added Python3 compatibility):
> https://github.com/Javex/pyramid/commit/d4f2943fa50e34f682f8097dccee2ce3ef1e998e
> This function is not what I expect in the final version but it shows
> where I would like to go with this: Provide a function that makes it
> easier for a user to obtain a strong secret. Either we use it this way
> or the above mentioned seperate script, that really doesn't matter.
>
> Please tell me your thoughts on both topics.
Domen Kožar
Sep 23, 2012, 9:51:28 AM9/23/12
to pylons...@googlegroups.com, vl...@haronmedia.com
Thanks Florian, I'll try to come up with a sane patch based on this :) For generating secret, I would just update documentation to use something like:
$ openssl rand -base64 32
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
To view this discussion on the web visit https://groups.google.com/d/msg/pylons-devel/-/y-mh0zjghJIJ.
Florian Rüchel
Sep 23, 2012, 9:54:12 AM9/23/12
to pylons...@googlegroups.com, vl...@haronmedia.com
On Sunday, September 23, 2012 3:11:51 PM UTC+2, Chris McDonough wrote:
On Sun, 2012-09-23 at 05:54 -0700, Florian Rüchel wrote:
>
> How about a script that's part of the framework itself? We
> have pserve,
> pcreate... how about
>
> pkeygen [-w <filename>]
>
> or
>
> pyramid-keygen [-w <filename>]
>
> I like this idea very much. I would like to either get this usage
> approved or I would just build a simple function inside pyramid.
> However, such a function belongs more into an installation than into
> application code. Can you tell me how to build such a script that runs
> on both Windows and Linux? I would like to see it implemented in this
> way if Chris approves.
Who will use it and when would they use it?
Well it will be used by the user deploying the application. This can be the developer or even multiple end-users (if the application itself is distributed). In both cases there would be a simple deployment instruction:
"Upon installation you need to execute 'pyramid-keygen -w cookie_secret -b 256'" or similar. Then you have a secret file and can use it in your application. Otherwise each developer has to develop their own method of generating such a secret during the deployment process. Furthermore, we could then add a hint to the documentation to use this if they want a strong secret (see below).
> On a seperate note: I have started on improving the documentation. As
> a first step, I have edited the `narr/authentication.rst` to include a
> note and have documented the API for
> `pyramid.authentication.AuthTktAuthenticationPolicy` (better
> documentation for secret, add documentation for hashalg). My question
> is now how would you handle this in regards to the documentation. I
> thought about adding this (or a similar) note everywhere this policy
> is used. This should raise the awareness everywhere the docs are read
> (e.g. tutorials). Furthermore, since we would clearly recommend to use
> something like SHA256 if MD5 is not explicitly needed, should we
> change the code examples to include a better hashalg (instead of just
> documenting it)? I would vote for a yes, since I don't see any
> disadvantage: If you build a new application, you should always use
> another algorithm and as shown above mod_auth_tkt can also easily
> handle other algorithms if configured correctly.
I didn't know we already had a mergeable patch for the hashalg stuff.
The last patch I saw seemed maybe a little overwrought. Until we figure
that out, I'd hold off on changing docs.
Yeah, I took a step back, changed it to be only a callable and made it very simple again. Take look at the corresponding commit and tell me if it is still too much.
If you approve, I would really recommend changing the docs at least in regards to a notice that the default behavior is at least not *that* secure.
Regards,
Florian
Chris McDonough
Sep 23, 2012, 10:05:23 AM9/23/12
to pylons...@googlegroups.com, vl...@haronmedia.com
Instead, developers are expected to add a secret value to their .ini
file and use it as input to the AuthTktAuthenticationPolicy constructor.
So I'd be -1 on something that created a file, unless there's some other
use case that this sweater-thread of a topic has convinced us must be
implemented along the lines of "the secret must be in another file".
If the purpose is only to output a "sufficiently random" string, then I
personally don't think we need to get into the business of supplying
software that does that, although we might supply documentation for UNIX
and for Windows that helps people do that using third-party tools. If
that turns out to be untenable for some reason, then I might reconsider
writing software that helps.
>
> If you approve, I would really recommend changing the docs at least in
> regards to a notice that the default behavior is at least not *that*
> secure.
- C
Domen Kožar
Sep 23, 2012, 12:03:40 PM9/23/12
to pylons...@googlegroups.com, vl...@haronmedia.com
Created pull request, changed the approach a bit: https://github.com/Pylons/pyramid/pull/695
- C
--
You received this message because you are subscribed to the Google Groups "pylons-devel" group.
Daniel Holth
Sep 24, 2012, 9:58:27 AM9/24/12
to pylons...@googlegroups.com, vl...@haronmedia.com
https://github.com/plone/plone.session/blob/master/plone/session/tktauth.py
Have you been able to use your patch with Apache's mod_auth_tkt?
Domen Kožar
Sep 24, 2012, 4:34:52 PM9/24/12
to pylons...@googlegroups.com, vl...@haronmedia.com
Unfortunately I stopped using apache as web server back in 2006, could someone else try that? Thanks!
To view this discussion on the web visit https://groups.google.com/d/msg/pylons-devel/-/M3T4DuD1fKwJ.
Reply all
Reply to author
Forward
0 new messages