Hi, Today marks the end of explicit database locks issued by Drupal core. Thanks to David Strauss who helped me getting rid of them by patching cache_set and variable_set in http://drupal.org/node/55516 . Now, do we want to keep db_lock_table ? In Drupal 6, likely yes. In Drupal 7? The question makes sense even if go over to PDO. Regards, NK
Quoting Karoly Negyesi <karoly@negyesi.net>:
Hi,
Today marks the end of explicit database locks issued by Drupal core. Thanks to David Strauss who helped me getting rid of them by patching cache_set and variable_set in http://drupal.org/node/55516 .
Now, do we want to keep db_lock_table ? In Drupal 6, likely yes. In Drupal 7? The question makes sense even if go over to PDO.
Yes, please and thank you. Locking tables are still something someone might want to do. Though issuing the sql through db_query is just as good, I suppose, having the API just seems more thorough. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On Nov 6, 2007 9:54 AM, Earnie Boyd <earnie@users.sourceforge.net> wrote:
Yes, please and thank you. Locking tables are still something someone might want to do. Though issuing the sql through db_query is just as good, I suppose, having the API just seems more thorough.
To provide some data on the subject...the list of files on the DRUPAL-5 branch in CVS which contain db_lock_table: buddylist/buddylist.module: chatroom/chatroom.module: commentapproval/commentapproval.module: community_tags/community_tags.module: exif/exif.module: i18n/i18n.module: localizer/system/includes/cache.inc: media_mover/media_mover_api.module: memcache/patches/DRUPAL-5-1-cache-serialize.patch: memcache/patches/DRUPAL-5-2-cache-serialize.patch: nodeapproval/nodeapproval.module: replication/bootstrap.inc.patch:+ reptag/reptag_helper.inc: reptag/reptag_module.inc: semantic_search/contrib/SONIA/ext/cache/ffcache.sonia.inc: trackback/trackback.module: xbview/xbview_parser.inc: Some of these are not concerns (e.g the two memcache patches include it is just in the context - not part of the patch). It's also possible that other branches contain more or fewer uses of the function, but this seems like a valid sample since it's the most common actively maintained branch. Among the 1409 modules on the DRUPAL-5 branch there are ~15 modules that use db_lock_table. It seems like a fairly infrequently used function. I imagine there are workarounds for most of them. Cheers, Greg PS full disclosure of my technique in case there's a flaw: CVS checkout: cvs -d :pserver@cvs.drupal.org:/cvs/drupal-contrib export -r DRUPAL-5 contributions/modules/ Number of modules: find ./ -name '*.module' | wc -l Files that use db_lock_table: grep -r db_lock_table * | gawk {'print $1'} | sort -u | wc -l -- Greg Knaddison Denver, CO | http://knaddison.com World Spanish Tour | http://wanderlusting.org/user/greg
Yup I did a similar grep, but I have not did my homework apparently. There are two typical kinds of locks: one, when doing a variable set which was a SELECT-INSERT. The solution to this is db_query("UPDATE {variable} SET value = '%s' WHERE name = '%s'", $serialized_value, $name); if (!db_affected_rows()) { @db_query("INSERT INTO {variable} (name, value) VALUES ('%s', '%s')", $name, $serialized_value); } http://drupal.org/node/55516#comment-389837 explains why locks are not necessary. Files that do stuff like this (I included those that already do an UPDATE-INSERT but wtih a lock which is unnecssary above):
buddylist/buddylist.module: commentapproval/commentapproval.module: i18n/i18n.module: localizer/system/includes/cache.inc: memcache/patches/DRUPAL-5-1-cache-serialize.patch: memcache/patches/DRUPAL-5-2-cache-serialize.patch: replication/bootstrap.inc.patch:+ nodeapproval/nodeapproval.module: semantic_search/contrib/SONIA/ext/cache/ffcache.sonia.inc: (this one creates the table if needed inside the lock, do this in the install hook instead) trackback/trackback.module:
And then we have a set of N-M saves, where you had N items in the database and now you will have M. You can always write a diff save -- drupal_write_record will save the legwork of writing a separate INSERT/UPDATE SQL.
community_tags/community_tags.module: exif/exif.module:
Special cases:
chatroom/chatroom.module: When this module goes over to Drupal-6 thus MySQL 4.1 it can revert http://drupal.org/node/70949 . "Prior to MySQL 4.0.14, the target table of the INSERT statement cannot appear in the FROM clause of the SELECT part of the query. This limitation is lifted in 4.0.14. "
media_mover/media_mover_api.module: Instead of a SELECT first and an UPDATE later you can db_query('UPDATE {media_mover_config_list} SET status = "running", last_start_time = %d, start_time = %d WHERE cid = %d AND status != 'running'', $config['start_time'], time(), $cid); -- I added AND status != 'running' here. Now, check for affected rows. If you have not affected any rows, then there is another running, if you affected, then you can run the SELECT.
Another UPDATE trick for long running scripts is the usage of an autoincrement field, if $id is the value of it, then UPDATE foo SET whatever = $id ; SELECT * FROM foo WHERE whatever = $id will very nicely take care of your script instance pwning one row without locking anything. This will let several instances of the same script running.
reptag/reptag_helper.inc: reptag/reptag_module.inc:
I was unable to get why this is using locks...
xbview/xbview_parser.inc: db_lock_table('xbookmark');db_query("DELETE FROM {xbookmark} ");db_unlock_tables();
I am not getting clearly why you need to lock for one single statement -- but here is a hint: TRUNCATE exists in pgsql and mysql both and will be a lot faster than DELETE FROM.
I think you mean the dusk of DB locks, chx. :-) In any case, yay. I would recommend leaving db_lock_table() in place, as we can't guarantee that there will never be a case where a lock is needed; just that we've found workarounds for all of core's use-cases and many contribs'. Instead, I'd modify the docs for it to say "this is bad for performance, there's usually other ways like A, B, C, don't use this unless you really have to." On Tuesday 06 November 2007, Karoly Negyesi wrote:
Hi,
Today marks the end of explicit database locks issued by Drupal core. Thanks to David Strauss who helped me getting rid of them by patching cache_set and variable_set in http://drupal.org/node/55516 .
Now, do we want to keep db_lock_table ? In Drupal 6, likely yes. In Drupal 7? The question makes sense even if go over to PDO.
Regards,
NK
-- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
On Nov 6, 2007 12:19 PM, Karoly Negyesi <karoly@negyesi.net> wrote:
Today marks the end of explicit database locks issued by Drupal core. Thanks to David Strauss who helped me getting rid of them by patching cache_set and variable_set in http://drupal.org/node/55516 .
Now, do we want to keep db_lock_table ? In Drupal 6, likely yes. In Drupal 7? The question makes sense even if go over to PDO.
Note that the installer still checks for LOCK permissions in the database. Now that we see some contrib modules use it, should we remove the LOCK check or should we keep it? Gaboe
If removed from the installer, it could also be removed from http://drupal.org/requirements On that note, CREATE TEMPORARY TABLES will only be used in one place once the search module patch is in - comment.install
On Nov 6, 2007 8:21 AM, catch libcom <catch56@googlemail.com> wrote:
If removed from the installer, it could also be removed from http://drupal.org/requirements
On that note, CREATE TEMPORARY TABLES will only be used in one place once the search module patch is in - comment.install
Used in one place still makes it a requirement. LOCK TABLES dropped from a core requirement, I think a special front page announcement on _just_ that one subject on the front page of d.o. in addition to the module update documentation would be appropriate. Something explaining it a bit from a technical perspective.
Steven Peck wrote:
Used in one place still makes it a requirement.
Of course, so here's an issue (no patch yet) to remove it: http://drupal.org/node/189832
In further news, social engineering still works. On Nov 6, 2007 8:39 AM, Walt Daniels <wdlists@optonline.net> wrote:
http://openid.net/pipermail/general/2007-January/001260.html
On Wednesday 07 November 2007 01:12, Steven Peck wrote:
In further news, social engineering still works.
What do you exactly mean, in this context? I don't understand what you are alluding to. (I mean: really, I am curious, especially since I have plans to use OpenId in the future). @Walt: Thank you for the information. Please, don't hijack a thread to post unrelated information. Start a new thread instead. (the message was posted in reply in the thread "The dawn of DB locks"). Thanks again for sharing the information. Blessings, Augustin.
On Nov 6, 2007, at 8:28 PM, Augustin (Beginner) wrote:
On Wednesday 07 November 2007 01:12, Steven Peck wrote:
In further news, social engineering still works.
What do you exactly mean, in this context?
Phishing is inherently about tricking humans into compromising their own security. OpenID isn't unique in this regard -- look at how well many phishing operations work, regardless of the underlying authentication mechanism. -Derek (dww)
On Wednesday 07 November 2007 14:28, Derek Wright wrote:
Phishing is inherently about tricking humans into compromising their own security. OpenID isn't unique in this regard -- look at how well many phishing operations work, regardless of the underlying authentication mechanism.
Ok, thanks. However, it seems to me that the OpenId protocol seems to make phishing easier. I agree that there is no perfect solution, but there is no need to actually make it easy to con the users. I followed some of the links, read the discussion: OpenID: Phishing Heaven http://www.links.org/?p=187 Solving the OpenID phishing problem http://simonwillison.net/2007/Jan/19/phishing/ I'll look if the later can be implemented for Drupal in a few months from now. Blessings, Augustin.
On Nov 6, 2007, at 10:58 PM, Augustin (Beginner) wrote:
... Solving the OpenID phishing problem http://simonwillison.net/2007/Jan/19/phishing/
I'll look if the later can be implemented for Drupal in a few months from now.
I just read through the original post and all the comments in that thread. What, exactly, are you looking into implementing? There are a wide range of different (conflicting) proposals in that thread, none of them clearly "solving" the problem. -Derek (dww)
Augustin (Beginner) wrote:
However, it seems to me that the OpenId protocol seems to make phishing easier.
In a sense it does, because as Derek says it can open up vectors by generally encouraging bad, phishable behaviour on the part of web users. But OpenID was never intended to solve the problem of reciprocal user--site-owner trust; instead, it's meant solely to centralize authentication. "This is not a trust system. Trust requires identity first." http://simonwillison.net/2007/Jan/10/account/ Most of the halfway-viable solutions on Simon's blog seem to point to the OpenID *providers* changing the way they process inbound requests e.g. require people to input extra URLs etc. when they land on the provider page. It's largely out of the hands of the e.g. Drupal sites that direct people to OpenID providers. Unless you're running your own OpenID *server* then this isn't an issue. Looking at the module page I don't think that's in 5.x yet, let alone core. In that case, if your Drupal site merely consumes OpenID - that is, it lets other people log in with OpenId - then I think the only way to expose your incoming users to phishing is for *you* to be the phisher! J-P -- J-P Stacey +44 (0)1608 811870 http://torchbox.com
On Wednesday 07 November 2007 17:58, J-P Stacey wrote:
Unless you're running your own OpenID *server* then this isn't an issue. Looking at the module page I don't think that's in 5.x yet, let alone core.
Thanks. I thought Drupal could act as a server.... Oh. I see what you mean. http://drupal.org/project/openid says the server code is in 4.7. http://drupal.org/node/144050 Here are some references for those interested: server module? (feature request) http://drupal.org/node/185272 Port 4.7-2.x to Drupal 5 http://drupal.org/node/126841 PHP-based OpenID Server code http://groups.drupal.org/node/1109 Augustin.
One thing that might help a little is to allow people to upload their verification picture. Then separate the userid and password to separate screens, or in the case of OpenID the proceed to the server page, with a new page where you show them their verification picture and the password box, or for OpenID a proceed button. Rather than allowing them to upload a verification picture, they could select from a large collection of supplied ones. One bank I use does approximately this and has a picture and a phrase under it that I supplied. -----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Augustin (Beginner) Sent: Wednesday, November 07, 2007 8:10 AM To: development@drupal.org Subject: Re: [development] OpenId open to phishing attacks. On Wednesday 07 November 2007 17:58, J-P Stacey wrote:
Unless you're running your own OpenID *server* then this isn't an issue. Looking at the module page I don't think that's in 5.x yet, let alone core.
Thanks. I thought Drupal could act as a server.... Oh. I see what you mean. http://drupal.org/project/openid says the server code is in 4.7. http://drupal.org/node/144050 Here are some references for those interested: server module? (feature request) http://drupal.org/node/185272 Port 4.7-2.x to Drupal 5 http://drupal.org/node/126841 PHP-based OpenID Server code http://groups.drupal.org/node/1109 Augustin. -- No virus found in this incoming message. Checked by AVG Free Edition. Version: 7.5.503 / Virus Database: 269.15.23/1113 - Release Date: 11/6/2007 10:04 AM
That wouldn't do anything to prevent man-in-the-middle attacks. The concern is that sites may intercept your password. However, a man-in- the-middle attack would not be possible if the OpenID server uses SSL encryption. We can provide security by ensuring that the OpenID server will not accept an insecure connection. On Nov 7, 2007, at 9:46 AM, Walt Daniels wrote:
One thing that might help a little is to allow people to upload their verification picture. Then separate the userid and password to separate screens, or in the case of OpenID the proceed to the server page, with a new page where you show them their verification picture and the password box, or for OpenID a proceed button. Rather than allowing them to upload a verification picture, they could select from a large collection of supplied ones. One bank I use does approximately this and has a picture and a phrase under it that I supplied.
I have no doubt that the hackers will find ways around almost anything we (or anybody else) does to prevent phishing. There is no possibility of overestimating the stupidity of our users in ignoring all the best that we can offer. My proposal is a simple to implement step in the right direction (supplemented by server side heavier duty security). It doesn't change the user behavior too much to be annoying. One can always make things more secure by introducing more and more complication. -----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Darren Oh Sent: Wednesday, November 07, 2007 9:58 AM To: development@drupal.org Subject: Re: [development] OpenId open to phishing attacks. That wouldn't do anything to prevent man-in-the-middle attacks. The concern is that sites may intercept your password. However, a man-in- the-middle attack would not be possible if the OpenID server uses SSL encryption. We can provide security by ensuring that the OpenID server will not accept an insecure connection. On Nov 7, 2007, at 9:46 AM, Walt Daniels wrote:
One thing that might help a little is to allow people to upload their verification picture. Then separate the userid and password to separate screens, or in the case of OpenID the proceed to the server page, with a new page where you show them their verification picture and the password box, or for OpenID a proceed button. Rather than allowing them to upload a verification picture, they could select from a large collection of supplied ones. One bank I use does approximately this and has a picture and a phrase under it that I supplied.
-- No virus found in this incoming message. Checked by AVG Free Edition. Version: 7.5.503 / Virus Database: 269.15.24/1115 - Release Date: 11/7/2007 9:21 AM
My point is that a picture and phrase would provide a false sense of security, since a phishing site could request it on the user's behalf, display it, collect the user's input, and post it to the OpenID server. By providing a false sense of security, the picture could be worse than doing nothing. The only possible defense is for the user to notice that the page has been decrypted. A simple warning not to submit the form unless the secure icon is shown in the browser would be the best security. On Nov 7, 2007, at 10:41 AM, Walt Daniels wrote:
I have no doubt that the hackers will find ways around almost anything we (or anybody else) does to prevent phishing. There is no possibility of overestimating the stupidity of our users in ignoring all the best that we can offer. My proposal is a simple to implement step in the right direction (supplemented by server side heavier duty security). It doesn't change the user behavior too much to be annoying. One can always make things more secure by introducing more and more complication.
Ok, I see your point, and I need to talk to my bank. There is a slight wrinkle on their part in that my userid is more like an additional password that is unique across customers. It is associated with my account number but never displayed anywhere so without a keylogger no one should be able to find it. It is on an SSL page. It has the usual restrictions on length and mixed character sets. -----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Darren Oh Sent: Wednesday, November 07, 2007 10:56 AM To: development@drupal.org Subject: Re: [development] OpenId open to phishing attacks. My point is that a picture and phrase would provide a false sense of security, since a phishing site could request it on the user's behalf, display it, collect the user's input, and post it to the OpenID server. By providing a false sense of security, the picture could be worse than doing nothing. The only possible defense is for the user to notice that the page has been decrypted. A simple warning not to submit the form unless the secure icon is shown in the browser would be the best security. On Nov 7, 2007, at 10:41 AM, Walt Daniels wrote:
I have no doubt that the hackers will find ways around almost anything we (or anybody else) does to prevent phishing. There is no possibility of overestimating the stupidity of our users in ignoring all the best that we can offer. My proposal is a simple to implement step in the right direction (supplemented by server side heavier duty security). It doesn't change the user behavior too much to be annoying. One can always make things more secure by introducing more and more complication.
-- No virus found in this incoming message. Checked by AVG Free Edition. Version: 7.5.503 / Virus Database: 269.15.24/1115 - Release Date: 11/7/2007 9:21 AM
I am absolutely NOT hijacking the thread. Someone forwarded a one line email consisting of a link. This then forced me to go read an email on another web site. My end conclusion was.... DUH. Nothing new. If the original poster had concerns _based_ on the email in question then they should have actually posted their thoughts _on_ the information and then _referenced_ the link for further information. In the absence of _any_ context, _anything_ posted in response was in fact relevant to the initial post. Thankyouverymuch! Steven On Nov 6, 2007 8:28 PM, Augustin (Beginner) <drupal.beginner@wechange.org> wrote:
On Wednesday 07 November 2007 01:12, Steven Peck wrote:
In further news, social engineering still works.
What do you exactly mean, in this context? I don't understand what you are alluding to. (I mean: really, I am curious, especially since I have plans to use OpenId in the future).
@Walt: Thank you for the information. Please, don't hijack a thread to post unrelated information. Start a new thread instead. (the message was posted in reply in the thread "The dawn of DB locks"). Thanks again for sharing the information.
Blessings,
Augustin.
Steven Peck wrote:
I am absolutely NOT hijacking the thread. FYI, I think he was talking to Walt when he said the hijacking part. @Walt: Thank you for the information. Please, don't hijack a thread to post unrelated information. -Rob
Ah, my bad then. On Nov 7, 2007 1:28 PM, Rob Barreca <rob@electronicinsight.com> wrote:
Steven Peck wrote:
I am absolutely NOT hijacking the thread. FYI, I think he was talking to Walt when he said the hijacking part.
@Walt: Thank you for the information. Please, don't hijack a thread to post unrelated information. -Rob
Quoting Gábor Hojtsy <gabor@hojtsy.hu>:
On Nov 6, 2007 12:19 PM, Karoly Negyesi <karoly@negyesi.net> wrote:
Today marks the end of explicit database locks issued by Drupal core. Thanks to David Strauss who helped me getting rid of them by patching cache_set and variable_set in http://drupal.org/node/55516 .
Now, do we want to keep db_lock_table ? In Drupal 6, likely yes. In Drupal 7? The question makes sense even if go over to PDO.
Note that the installer still checks for LOCK permissions in the database. Now that we see some contrib modules use it, should we remove the LOCK check or should we keep it?
IMO the contrib module shouldn't depend on core to make sure that its requirements are met. However, it would be nice for core to provide methods to check the requirement exists. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
Note that the installer still checks for LOCK permissions in the database. Now that we see some contrib modules use it
All of them do in error.
IMO the contrib module shouldn't depend on core to make sure that its requirements are met. However, it would be nice for core to provide methods to check the requirement exists.
Patch?
Note that the installer still checks for LOCK permissions in the database. Now that we see some contrib modules use it
All of them do in error.
IMO the contrib module shouldn't depend on core to make sure that its requirements are met. However, it would be nice for core to provide methods to check the requirement exists.
Patch?
On Nov 6, 2007 10:59 AM, Karoly Negyesi <karoly@negyesi.net> wrote:
Note that the installer still checks for LOCK permissions in the database. Now that we see some contrib modules use it
All of them do in error.
IMO the contrib module shouldn't depend on core to make sure that its requirements are met. However, it would be nice for core to provide methods to check the requirement exists.
Patch?
My point is, used in error or not, this is A) A significant change B) Something to celebrate C) Something to wake people up a bit to the upcoming changes and won't hurt to make them aware of.
My point is, used in error or not, this is A) A significant change B) Something to celebrate C) Something to wake people up a bit to the upcoming changes and won't hurt to make them aware of.
We kind of agree on A, B, C. What I meant to say is that because we saw no contribs that will need locks in Drupal 6, we can remove the requirement. I believe I do not want to change API. I will wait with the announcement until temporary tables are gone as well or until it's decided that we are not getting rid of them (which would be a turn of events I am not expecting).
participants (13)
-
Augustin (Beginner) -
catch libcom -
Darren Oh -
Derek Wright -
Earnie Boyd -
Greg Knaddison -
Gábor Hojtsy -
J-P Stacey -
Karoly Negyesi -
Larry Garfield -
Rob Barreca -
Steven Peck -
Walt Daniels