# faster & public domain MD5 implementation - Thread: https://externals.io/message/33878 - Original archive: https://news-web.php.net/php.internals/33878 - Started by Solar Designer on 2007-12-09T01:05:52+00:00 - Messages: 36 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/33878.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #33878 — Solar Designer — 2007-12-09T01:05:52+00:00 URL: https://externals.io/message/33878#33878 Hi, Attached is a quick patch for PHP 5.2.5 that replaces RSA's copyrighted implementation of MD5 with my public domain one: http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ This also results in faster and slightly smaller code (both source and binary). On a Pentium 3 that I've tested this on, the speedup is 12% for the portable hashes as implemented in phpass: http://www.openwall.com/phpass/ Given that most CPU time is spent on various overhead of the PHP interpreter rather than on MD5 hashing itself (and gprof confirms this), the 12% speedup seen on the PHP script as a whole means that the speedup for the MD5 implementation alone is much higher than that. The speedup should be similar on other little-endian CPUs (other x86 CPUs, x86-64, Alpha), but smaller on big-endian. The code can be made cleaner by taking my md5.[ch] files as they are and introducing two files more for PHP's added functions. I did not do it for this patch in order for my changes to be less invasive. I also did not similarly replace the MD5 implementation in hash_md.c, which obviously will need to be done (once again, I'd prefer that separate md5.[ch] files are used - and perhaps only one instance of them included in the PHP distribution). I also wrote a similar public domain implementation of MD4, which I can provide for inclusion in hash_md.c if there's any interest. Finally, it'd be nice if PHP could optionally link against OpenSSL libcrypto to take advantage of the architecture-specific implementations of these hashes. My implementations of MD5 and MD4 are function prototype compatible with OpenSSL's. I'd appreciate being CC'ed on any follow-ups as I am not on the list. Thanks, -- Alexander Peslyak GPG key ID: 5B341F15 fp: B3FB 63F4 D7A3 BCCC 6F6E FC55 A2FC 027C 5B34 1F15 http://www.openwall.com - bringing security into open computing environments --- ## #33881 — Peter Brodersen — 2007-12-09T11:53:56+00:00 URL: https://externals.io/message/33878#33881 · In reply to: #33878 On Sun, 9 Dec 2007 04:05:52 +0300, in php.internals solar@openwall.com (Solar Designer) wrote: >Attached is a quick patch for PHP 5.2.5 that replaces RSA's copyrighted >implementation of MD5 with my public domain one: > > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ A bit on a side note regarding the php function `md5()`: In general it is possible to supply an arbitrary number of bits as input to MD5. However, the implementation of `md5()` only takes a sting with octets as the smallest fragments. Would it somehow be possible to supply an input where the number of bits does not add up to a number divisible by eight? Or is this feature of md5 simply not relevant to anybody? -- - Peter Brodersen --- ## #33884 — Solar Designer — 2007-12-09T17:49:02+00:00 URL: https://externals.io/message/33878#33884 · In reply to: #33881 > A bit on a side note regarding the php function `md5()`: In general it > is possible to supply an arbitrary number of bits as input to MD5. > However, the implementation of `md5()` only takes a sting with octets as > the smallest fragments. > > Would it somehow be possible to supply an input where the number of > bits does not add up to a number divisible by eight? Or is this > feature of md5 simply not relevant to anybody? I don't think there's any demand for this. Even the reference implementation only works on whole octets, so there are no protocol specs or anything (that I am aware of) that would require computation of MD5 on a bitstream of arbitrary length. Alexander --- ## #34030 — Solar Designer — 2007-12-14T04:32:06+00:00 URL: https://externals.io/message/33878#34030 · In reply to: #33878 A few days ago, I wrote: > Attached is a quick patch for PHP 5.2.5 that replaces RSA's copyrighted > implementation of MD5 with my public domain one: > > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ > > This also results in faster and slightly smaller code (both source and > binary). On a Pentium 3 that I've tested this on, the speedup is 12% > for the portable hashes as implemented in phpass: > > http://www.openwall.com/phpass/ The code from phpass is now being used by phpBB3 (3.0.0 release) and WordPress (development). It is being considered for Drupal 7. Wouldn't it be nice for PHP to be more efficient when running those popular apps, and at the same time have cleaner and smaller code? Of course, it'd be great if PHP would also integrate the `CRYPT_BLOWFISH` code from Suhosin, but that can be discussed separately (if it needs a discussion). > Given that most CPU time is spent on various overhead of the PHP > interpreter rather than on MD5 hashing itself (and gprof confirms this), > the 12% speedup seen on the PHP script as a whole means that the speedup > for the MD5 implementation alone is much higher than that. The speedup > should be similar on other little-endian CPUs (other x86 CPUs, x86-64, > Alpha), but smaller on big-endian. A correction: actually, Alpha falls into the same category with big-endian CPUs here (a smaller speedup - but nevertheless a speedup) because it lacks hardware support for unaligned accesses. I must have been too tired when I wrote the above. I am not quoting the rest of my posting, although I'd appreciate comments on the suggestions I had made there. Thanks, Alexander --- ## #34189 — Stanislav Malyshev — 2007-12-21T00:27:30+00:00 URL: https://externals.io/message/33878#34189 · In reply to: #33878 > Attached is a quick patch for PHP 5.2.5 that replaces RSA's copyrighted > implementation of MD5 with my public domain one: > > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ Tried that one and it is about 30% faster indeed (on md5-only benchmark, 32-bit Linux on AMD Opteron. Anybody objects to accepting this? -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #34190 — Martin Alterisio — 2007-12-21T02:15:20+00:00 URL: https://externals.io/message/33878#34190 · In reply to: #34189 2007/12/20, Stanislav Malyshev : > > > […] > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ > > Tried that one and it is about 30% faster indeed (on md5-only benchmark, > 32-bit Linux on AMD Opteron. Anybody objects to accepting this? > > Just one, but is a mere formality. I didn't see any copyright notice on the code nor a licensing document attached. I have not much of the legal mumbo-jumbo comprehension, so, correct me if I'm wrong, shouldn't external code that's to be included in the php codebase meet these legal formalities so it doesn't become a liability? That's all. Best Regards, Martin Alterisio PS: Does anyone knows if using a nickname for authorship is considered legally valid? I believe it might be valid, think about writers that use such pseudonyms, but I'm not sure... --- ## #34196 — Antony Dovgal — 2007-12-21T09:16:08+00:00 URL: https://externals.io/message/33878#34196 · In reply to: #34190 > > […] > Just one, but is a mere formality. I didn't see any copyright notice on the > code nor a licensing document attached. I have not much of the legal > mumbo-jumbo comprehension, so, correct me if I'm wrong, shouldn't external > code that's to be included in the php codebase meet these legal formalities > so it doesn't become a liability? Good point, the license should be explicitly specified. I'm going to test the patch on Linux/64bit today. -- Wbr, Antony Dovgal --- ## #34197 — Alexey Zakhlestin — 2007-12-21T09:55:33+00:00 URL: https://externals.io/message/33878#34197 · In reply to: #34190 isn't "public domain" specific enough? > 2007/12/20, Stanislav Malyshev : > > […] > Just one, but is a mere formality. I didn't see any copyright notice on the > code nor a licensing document attached. I have not much of the legal > mumbo-jumbo comprehension, so, correct me if I'm wrong, shouldn't external > code that's to be included in the php codebase meet these legal formalities > so it doesn't become a liability? > > That's all. > > Best Regards, > > Martin Alterisio > > PS: Does anyone knows if using a nickname for authorship is considered > legally valid? I believe it might be valid, think about writers that use > such pseudonyms, but I'm not sure... > -- Alexey Zakhlestin http://blog.milkfarmsoft.com/ --- ## #34199 — Marcus Boerger — 2007-12-21T10:30:21+00:00 URL: https://externals.io/message/33878#34199 · In reply to: #33878 Hello Solar, thanks again for offering this. What we require is to have the PHP License in the code. If you are fine with this we will gladly replace the existing code with yours. You can of course put more header information in there to stress out further that it is your code and where it can be found. The key is that we indeed try to get rid of anything that is not under the PHP License. marcus Sunday, December 9, 2007, 2:05:52 AM, you wrote: > Hi, > Attached is a quick patch for PHP 5.2.5 that replaces RSA's copyrighted > implementation of MD5 with my public domain one: > > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ > This also results in faster and slightly smaller code (both source and > binary). On a Pentium 3 that I've tested this on, the speedup is 12% > for the portable hashes as implemented in phpass: > http://www.openwall.com/phpass/ > Given that most CPU time is spent on various overhead of the PHP > interpreter rather than on MD5 hashing itself (and gprof confirms this), > the 12% speedup seen on the PHP script as a whole means that the speedup > for the MD5 implementation alone is much higher than that. The speedup > should be similar on other little-endian CPUs (other x86 CPUs, x86-64, > Alpha), but smaller on big-endian. > The code can be made cleaner by taking my md5.[ch] files as they are and > introducing two files more for PHP's added functions. I did not do it > for this patch in order for my changes to be less invasive. > I also did not similarly replace the MD5 implementation in hash_md.c, > which obviously will need to be done (once again, I'd prefer that > separate md5.[ch] files are used - and perhaps only one instance of them > included in the PHP distribution). > I also wrote a similar public domain implementation of MD4, which I can > provide for inclusion in hash_md.c if there's any interest. > Finally, it'd be nice if PHP could optionally link against OpenSSL > libcrypto to take advantage of the architecture-specific implementations > of these hashes. My implementations of MD5 and MD4 are function > prototype compatible with OpenSSL's. > I'd appreciate being CC'ed on any follow-ups as I am not on the list. > Thanks, Best regards, Marcus --- ## #34200 — William A. Rowe — 2007-12-21T10:41:10+00:00 URL: https://externals.io/message/33878#34200 · In reply to: #34197 Alexey Zakhlestin wrote: > isn't "public domain" specific enough? Public Domain actually isn't a universal concept, and it isn't recognized everywhere in law. A very open BSD style license first, followed by granting it to the public domain, should cover all bases. Even somewhere that doesn't recognize public domain should fail over to accept your BSD license. --- ## #34202 — Martin Alterisio — 2007-12-21T13:29:35+00:00 URL: https://externals.io/message/33878#34202 · In reply to: #34197 No, public domain isn't recognized by copyright law as a copyright notice. An extra line should be added that says something like "no copyright is claimed", and attach a year range where the copyright is in effect (since copyright expires, if I remember correctly). Anyway, the keyword here is "copyright", which is recognized internationally (which means you should use this word even if writing the copyright notice in another language). Without a copyright notice, the code isn't apt for distribution. The author could claim that he did not give his consent for public distribution and that the code was an in-house development. Anyway, it's too complicate and I understand very little. To get the real facts here, you should talk to your lawyer. 2007/12/21, Alexey Zakhlestin : > > isn't "public domain" specific enough? > > > > […] > copyrighted > > […] > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ > > […] > benchmark, > > […] > the > > […] > external > > […] > formalities > > […] > > > -- > Alexey Zakhlestin > http://blog.milkfarmsoft.com/ --- ## #34204 — dz@bitxtender.com — 2007-12-21T16:32:36+00:00 URL: https://externals.io/message/33878#34204 · In reply to: #34202 Not entirely correct. As I pointed out in the other thread, not all countries have the concept of transferrable copyright. Therefor, a note should be added that explicitly states that everyone is free to use it without permission, fees etc. Much like a BSD or MIT license, but without the additional conditions of preserving copyright notices. David Am 21.12.2007 um 14:29 schrieb Martin Alterisio: > No, public domain isn't recognized by copyright law as a copyright > notice. > An extra line should be added that says something like "no copyright > is > claimed", and attach a year range where the copyright is in effect > (since > copyright expires, if I remember correctly). Anyway, the keyword > here is > "copyright", which is recognized internationally (which means you > should use > this word even if writing the copyright notice in another language). > > Without a copyright notice, the code isn't apt for distribution. The > author > could claim that he did not give his consent for public distribution > and > that the code was an in-house development. Anyway, it's too > complicate and I > understand very little. To get the real facts here, you should talk > to your > lawyer. > > 2007/12/21, Alexey Zakhlestin : > > […] --- ## #34206 — Martin Alterisio — 2007-12-21T17:30:14+00:00 URL: https://externals.io/message/33878#34206 · In reply to: #34204 Thanks! I didn't know about that. 2007/12/21, David Zülke : > > Not entirely correct. As I pointed out in the other thread, not all > countries have the concept of transferrable copyright. Therefor, a > note should be added that explicitly states that everyone is free to > use it without permission, fees etc. Much like a BSD or MIT license, > but without the additional conditions of preserving copyright notices. > > > > David > > > > Am 21.12.2007 um 14:29 schrieb Martin Alterisio: > > > […] > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ > > […] --- ## #34243 — Sara Golemon — 2007-12-25T06:30:16+00:00 URL: https://externals.io/message/33878#34243 · In reply to: #33878 > I also wrote a similar public domain implementation of MD4, which I can > provide for inclusion in hash_md.c if there's any interest. > No objections from me on replacing the implementations in ext/hash. Those were written with functional correctness in mind, not speed. So long as the test vectors in ext/hash/tests continue to pass, I don't see why faster algorithms wouldn't be quite welcome... -Sara --- ## #35180 — Dmitry Stogov — 2008-02-05T11:34:40+00:00 URL: https://externals.io/message/33878#35180 · In reply to: #33878 Hi, We are going to include your `md5()` implementation into php-5.3.0. I confirm at least 25% `md5()` speedup on my Core2 3GHz, however license issues are not clear. We are going to distribute files under standard PHP license including your original copyright notes. The files which are going to be committed are attached. Please confirm your agreement. Thanks. Dmitry. Solar Designer wrote: > Hi, > > Attached is a quick patch for PHP 5.2.5 that replaces RSA's copyrighted > implementation of MD5 with my public domain one: > > http://cvsweb.openwall.com/cgi/cvsweb.cgi/Owl/packages/popa3d/popa3d/md5/ > > This also results in faster and slightly smaller code (both source and > binary). On a Pentium 3 that I've tested this on, the speedup is 12% > for the portable hashes as implemented in phpass: > > http://www.openwall.com/phpass/ > > Given that most CPU time is spent on various overhead of the PHP > interpreter rather than on MD5 hashing itself (and gprof confirms this), > the 12% speedup seen on the PHP script as a whole means that the speedup > for the MD5 implementation alone is much higher than that. The speedup > should be similar on other little-endian CPUs (other x86 CPUs, x86-64, > Alpha), but smaller on big-endian. > > The code can be made cleaner by taking my md5.[ch] files as they are and > introducing two files more for PHP's added functions. I did not do it > for this patch in order for my changes to be less invasive. > > I also did not similarly replace the MD5 implementation in hash_md.c, > which obviously will need to be done (once again, I'd prefer that > separate md5.[ch] files are used - and perhaps only one instance of them > included in the PHP distribution). > > I also wrote a similar public domain implementation of MD4, which I can > provide for inclusion in hash_md.c if there's any interest. > > Finally, it'd be nice if PHP could optionally link against OpenSSL > libcrypto to take advantage of the architecture-specific implementations > of these hashes. My implementations of MD5 and MD4 are function > prototype compatible with OpenSSL's. > > I'd appreciate being CC'ed on any follow-ups as I am not on the list. > > Thanks, --- ## #35186 — johannes@php.net — 2008-02-05T18:09:18+00:00 URL: https://externals.io/message/33878#35186 · In reply to: #35180 Dmitry, > /* > * * This is an OpenSSL-compatible implementation of the RSA Data Security, > * * Inc. MD5 Message-Digest Algorithm (RFC 1321). > * * [...] looks like your editor has gone wild on that comment in md5.c ;-) johannes --- ## #35211 — Solar Designer — 2008-02-05T23:50:55+00:00 URL: https://externals.io/message/33878#35211 · In reply to: #35180 Hi Dmitry and all, First of all, please accept my apologies for failing to find the time to participate in the licensing issues discussion in December. It is a topic that I would like to discuss and arrive at a conclusion as I often happen to write code that I'd like to release to the public under the most relaxed terms possible. I thought that not claiming copyright (or even disclaiming copyright) and placing the code into the public domain would be it, but apparently in many (most? all?) jurisdictions there's no explicitly specified way for someone to place their works into the public domain (although the concept of public domain does exist - and stuff "falls" there as old copyrights expire) and now it has also been mentioned that some jurisdictions don't even recognize public domain at all (I have not yet seen/heard a lawyer state that, though). A possible solution could be to simultaneously try to place stuff in the public domain with a statement to that extent and license it to the public under very liberal terms. One issue with it is that I have to not claim or disclaim copyright in order to place a work of mine into the public domain, yet I have to be the copyright holder in order to license that work. Maybe this can be taken care of with a severability clause, making either the public domain or the license work in any given jurisdiction. But I'd rather see/hear a lawyer comment on that before I possibly go that route. That said, a lot of software that we use has been placed in the public domain by its authors. This includes some software by D. J. Bernstein, perhaps best known as the author of qmail, who is also known for the Bernstein vs. United States litigation - http://cr.yp.to/export.html - so perhaps he should know the law. Then, public domain is officially recognized as being compatible with GNU GPL by the FSF - http://www.fsf.org/licensing/licenses/ - and is apparently recognized by the OSI - http://opensource.org/node/239 > We are going to include your `md5()` implementation into php-5.3.0. Great! > I confirm at least 25% `md5()` speedup on my Core2 3GHz, however license > issues are not clear. > We are going to distribute files under standard PHP license including > your original copyright notes. > The files which are going to be committed are attached. > > Please confirm your agreement. Confirmed. Please note, however, that there were no "copyright notes" on my original files; instead, there was an authorship note and a public domain statement. I also have some comments on the modified files: > | Copyright (c) 1997-2008 The PHP Group | ... > | Author: Solar Designer | So you claim copyright to a modified version of my code, that I had placed in the public domain. This is fine by me. I do not formally require it (in fact, I can't), but maybe the "Author" line could be changed to either: | Original author: Solar Designer | or: | Authors: Solar Designer with further | | modifications by others. | (or you can make it more explicit, e.g. "... by The PHP Group" if that is appropriate - or whatever). > /* MD5 context. */ > typedef struct { > php_uint32 lo, hi; > php_uint32 a, b, c, d; > unsigned char buffer[64]; > php_uint32 block[16]; > } PHP_MD5_CTX; Maybe it would be better to do: typedef php_uint32 MD5_u32plus; and use the latter type. This would reduce the number of changes between my version of the code and yours, making it easier for you to sync to any newer versions of the code that I might make. > | Author: Solar Designer | If you do choose to change this in the .h file, then do the same in the .c, obviously. > #if (defined(__APPLE__) || defined(__APPLE_CC__)) && (defined(__BIG_ENDIAN__) || defined(__LITTLE_ENDIAN__)) > # if defined(__LITTLE_ENDIAN__) > # undef WORDS_BIGENDIAN > # else > # if defined(__BIG_ENDIAN__) > # define WORDS_BIGENDIAN > # endif > # endif > #endif This looks wrong to me. One of the specific properties of my implementation is that it does not strictly depend on the endianness being correctly specified at compile-time (and at all, for that matter). However, if you do happen to use the (little-endian and unaligned-OK) optimized code on a system that is not in fact little-endian or does not in fact tolerate unaligned accesses, then problems will arise! So any #if's you use must assume (might-be-big-endian and might-disallow-unaligned) by default. In fact, I am only aware of three widespread and general-purpose architectures that satisfy the criteria for the optimized code: #if defined(__i386__) || defined(__x86_64__) || defined(__vax__) Thus, I suggest that you leave the above #if intact, the way it was in the patch that I submitted. Do not explicitly check for any endianness macros - this is bound to cause problems. > /* > * * SET reads 4 input bytes in little-endian byte order and stores them > * * in a properly aligned word in host byte order. > * * > * * The check for little-endian architectures that tolerate unaligned > * * memory accesses is just an optimization. Nothing will break if it > * * doesn't work. > * */ > #ifndef WORDS_BIGENDIAN > # define SET(n) \ > (*(php_uint32 *)&ptr[(n) * 4]) > # define GET(n) \ > SET(n) > #else ... As explained above, I strongly recommend that you revert your "#ifndef WORDS_BIGENDIAN" to my "#if ..." What if an architecture is big-endian, but WORDS_BIGENDIAN just happens to not be specified? You'll have incorrect results (not MD5), whereas with my version of the code, everything will be just fine. Similarly, regardless of endianness, if WORDS_BIGENDIAN is not specified (maybe because the architecture is in fact little-endian), but the architecture does not tolerate unaligned accesses (at all or supports them with kernel emulation), things will go wrong (SIGBUS or very poor performance and a flood of kernel messages). This issue can't occur with my original #if that only lists specific known-safe architectures. > data = body(ctx, data, size & ~(unsigned long)0x3f); If you change all of my unsigned long's to size_t, you should change this one as well. When on a 64-bit system (userland pointer size), your size_t better be 64-bit as well (I have not checked whether this is necessarily the case; I hope so). > PHPAPI void PHP_MD5Final(unsigned char *result, PHP_MD5_CTX *ctx) > { > unsigned long used, free; Here's another one. Thanks, Alexander Peslyak GPG key ID: 5B341F15 fp: B3FB 63F4 D7A3 BCCC 6F6E FC55 A2FC 027C 5B34 1F15 http://www.openwall.com - bringing security into open computing environments --- ## #35217 — Steph Fox — 2008-02-06T00:18:37+00:00 URL: https://externals.io/message/33878#35217 · In reply to: #35211 Hi Alexander, Stupid question maybe, but why can't you use your given name for this? Caveat: please don't let this email stand in the way of your giving your gift. - Steph ----- Original Message ----- From: "Solar Designer" To: "Dmitry Stogov" Cc: "Stanislav Malyshev" ; "Andi Gutmans" ; "PHP Internals List" Sent: Tuesday, February 05, 2008 11:50 PM Subject: Re: [PHP-DEV] faster & public domain MD5 implementation > Hi Dmitry and all, > > First of all, please accept my apologies for failing to find the time to > participate in the licensing issues discussion in December. It is a > topic that I would like to discuss and arrive at a conclusion as I often > happen to write code that I'd like to release to the public under the most > relaxed terms possible. I thought that not claiming copyright (or even > disclaiming copyright) and placing the code into the public domain would > be it, but apparently in many (most? all?) jurisdictions there's no > explicitly specified way for someone to place their works into the > public domain (although the concept of public domain does exist - and > stuff "falls" there as old copyrights expire) and now it has also been > mentioned that some jurisdictions don't even recognize public domain at > all (I have not yet seen/heard a lawyer state that, though). > > A possible solution could be to simultaneously try to place stuff in the > public domain with a statement to that extent and license it to the > public under very liberal terms. One issue with it is that I have to > not claim or disclaim copyright in order to place a work of mine into > the public domain, yet I have to be the copyright holder in order to > license that work. Maybe this can be taken care of with a severability > clause, making either the public domain or the license work in any given > jurisdiction. But I'd rather see/hear a lawyer comment on that before I > possibly go that route. > > That said, a lot of software that we use has been placed in the public > domain by its authors. This includes some software by D. J. Bernstein, > perhaps best known as the author of qmail, who is also known for the > Bernstein vs. United States litigation - http://cr.yp.to/export.html - > so perhaps he should know the law. Then, public domain is officially > recognized as being compatible with GNU GPL by the FSF - > http://www.fsf.org/licensing/licenses/ - and is apparently recognized by > the OSI - http://opensource.org/node/239 > > > > […] > > Great! > > > […] > > Confirmed. Please note, however, that there were no "copyright notes" > on my original files; instead, there was an authorship note and a public > domain statement. > > I also have some comments on the modified files: > > > […] > ... > > […] > > So you claim copyright to a modified version of my code, that I had > placed in the public domain. This is fine by me. > > I do not formally require it (in fact, I can't), but maybe the "Author" > line could be changed to either: > > | Original author: Solar Designer | > > or: > > | Authors: Solar Designer with further | > | modifications by others. | > > (or you can make it more explicit, e.g. "... by The PHP Group" if that > is appropriate - or whatever). > > > […] > > Maybe it would be better to do: > > typedef php_uint32 MD5_u32plus; > > and use the latter type. This would reduce the number of changes > between my version of the code and yours, making it easier for you to > sync to any newer versions of the code that I might make. > > > […] > > If you do choose to change this in the .h file, then do the same in the > .c, obviously. > > > […] > > This looks wrong to me. One of the specific properties of my > implementation is that it does not strictly depend on the endianness > being correctly specified at compile-time (and at all, for that matter). > However, if you do happen to use the (little-endian and unaligned-OK) > optimized code on a system that is not in fact little-endian or does not > in fact tolerate unaligned accesses, then problems will arise! So any > #if's you use must assume (might-be-big-endian and > might-disallow-unaligned) > by default. > > In fact, I am only aware of three widespread and general-purpose > architectures that satisfy the criteria for the optimized code: > > #if defined(__i386__) || defined(__x86_64__) || defined(__vax__) > > Thus, I suggest that you leave the above #if intact, the way it was in > the patch that I submitted. Do not explicitly check for any endianness > macros - this is bound to cause problems. > > > […] > ... > > As explained above, I strongly recommend that you revert your "#ifndef > WORDS_BIGENDIAN" to my "#if ..." > > What if an architecture is big-endian, but WORDS_BIGENDIAN just happens > to not be specified? You'll have incorrect results (not MD5), whereas > with my version of the code, everything will be just fine. > > Similarly, regardless of endianness, if WORDS_BIGENDIAN is not specified > (maybe because the architecture is in fact little-endian), but the > architecture does not tolerate unaligned accesses (at all or supports > them with kernel emulation), things will go wrong (SIGBUS or very poor > performance and a flood of kernel messages). This issue can't occur > with my original #if that only lists specific known-safe architectures. > > > […] > > If you change all of my unsigned long's to size_t, you should change > this one as well. > > When on a 64-bit system (userland pointer size), your size_t better be > 64-bit as well (I have not checked whether this is necessarily the case; > I hope so). > > > […] > > Here's another one. > > Thanks, > > Alexander Peslyak > GPG key ID: 5B341F15 fp: B3FB 63F4 D7A3 BCCC 6F6E FC55 A2FC 027C 5B34 > 1F15 > http://www.openwall.com - bringing security into open computing > environments --- ## #35221 — Solar Designer — 2008-02-06T02:36:59+00:00 URL: https://externals.io/message/33878#35221 · In reply to: #35217 Hi Steph, > Stupid question maybe, but why can't you use your given name for this? I can, and I don't mind you replacing all occurrences of "Solar Designer" with "Alexander Peslyak" in these two files - or would you like me to do that myself? I don't care much about this. I am simply better known in the "security community" as Solar Designer, although this is changing and the e-mail address probably gives me away despite of your attempt to hide me under my real name. ;-) > Caveat: please don't let this email stand in the way of your giving your > gift. Your question is just fine, and I might have more "gifts" for PHP (also related to cryptographic hashes) if this one goes in well. Thanks, Alexander --- ## #35285 — Dmitry Stogov — 2008-02-07T10:11:38+00:00 URL: https://externals.io/message/33878#35285 · In reply to: #35211 Hi Alexander, I've updated the files according to your notes, except php_uint32/MD5_u32plus. However new version breaks ext/hash md4. (ext/hash/tests/md4.phpt is broken). Sara, did you have a solution for this issue? Or could you look into it please? Thanks. Dmitry. Solar Designer wrote: > Hi Dmitry and all, > > First of all, please accept my apologies for failing to find the time to > participate in the licensing issues discussion in December. It is a > topic that I would like to discuss and arrive at a conclusion as I often > happen to write code that I'd like to release to the public under the most > relaxed terms possible. I thought that not claiming copyright (or even > disclaiming copyright) and placing the code into the public domain would > be it, but apparently in many (most? all?) jurisdictions there's no > explicitly specified way for someone to place their works into the > public domain (although the concept of public domain does exist - and > stuff "falls" there as old copyrights expire) and now it has also been > mentioned that some jurisdictions don't even recognize public domain at > all (I have not yet seen/heard a lawyer state that, though). > > A possible solution could be to simultaneously try to place stuff in the > public domain with a statement to that extent and license it to the > public under very liberal terms. One issue with it is that I have to > not claim or disclaim copyright in order to place a work of mine into > the public domain, yet I have to be the copyright holder in order to > license that work. Maybe this can be taken care of with a severability > clause, making either the public domain or the license work in any given > jurisdiction. But I'd rather see/hear a lawyer comment on that before I > possibly go that route. > > That said, a lot of software that we use has been placed in the public > domain by its authors. This includes some software by D. J. Bernstein, > perhaps best known as the author of qmail, who is also known for the > Bernstein vs. United States litigation - http://cr.yp.to/export.html - > so perhaps he should know the law. Then, public domain is officially > recognized as being compatible with GNU GPL by the FSF - > http://www.fsf.org/licensing/licenses/ - and is apparently recognized by > the OSI - http://opensource.org/node/239 > > > > […] > > Great! > > > […] > > Confirmed. Please note, however, that there were no "copyright notes" > on my original files; instead, there was an authorship note and a public > domain statement. > > I also have some comments on the modified files: > > > […] > ... > > […] > > So you claim copyright to a modified version of my code, that I had > placed in the public domain. This is fine by me. > > I do not formally require it (in fact, I can't), but maybe the "Author" > line could be changed to either: > > | Original author: Solar Designer | > > or: > > | Authors: Solar Designer with further | > | modifications by others. | > > (or you can make it more explicit, e.g. "... by The PHP Group" if that > is appropriate - or whatever). > > > […] > > Maybe it would be better to do: > > typedef php_uint32 MD5_u32plus; > > and use the latter type. This would reduce the number of changes > between my version of the code and yours, making it easier for you to > sync to any newer versions of the code that I might make. > > > […] > > If you do choose to change this in the .h file, then do the same in the > .c, obviously. > > > […] > > This looks wrong to me. One of the specific properties of my > implementation is that it does not strictly depend on the endianness > being correctly specified at compile-time (and at all, for that matter). > However, if you do happen to use the (little-endian and unaligned-OK) > optimized code on a system that is not in fact little-endian or does not > in fact tolerate unaligned accesses, then problems will arise! So any > #if's you use must assume (might-be-big-endian and might-disallow-unaligned) > by default. > > In fact, I am only aware of three widespread and general-purpose > architectures that satisfy the criteria for the optimized code: > > #if defined(__i386__) || defined(__x86_64__) || defined(__vax__) > > Thus, I suggest that you leave the above #if intact, the way it was in > the patch that I submitted. Do not explicitly check for any endianness > macros - this is bound to cause problems. > > > […] > ... > > As explained above, I strongly recommend that you revert your "#ifndef > WORDS_BIGENDIAN" to my "#if ..." > > What if an architecture is big-endian, but WORDS_BIGENDIAN just happens > to not be specified? You'll have incorrect results (not MD5), whereas > with my version of the code, everything will be just fine. > > Similarly, regardless of endianness, if WORDS_BIGENDIAN is not specified > (maybe because the architecture is in fact little-endian), but the > architecture does not tolerate unaligned accesses (at all or supports > them with kernel emulation), things will go wrong (SIGBUS or very poor > performance and a flood of kernel messages). This issue can't occur > with my original #if that only lists specific known-safe architectures. > > > […] > > If you change all of my unsigned long's to size_t, you should change > this one as well. > > When on a 64-bit system (userland pointer size), your size_t better be > 64-bit as well (I have not checked whether this is necessarily the case; > I hope so). > > > […] > > Here's another one. > > Thanks, > > Alexander Peslyak > GPG key ID: 5B341F15 fp: B3FB 63F4 D7A3 BCCC 6F6E FC55 A2FC 027C 5B34 1F15 > http://www.openwall.com - bringing security into open computing environments --- ## #35288 — Solar Designer — 2008-02-07T10:35:58+00:00 URL: https://externals.io/message/33878#35288 · In reply to: #35285 Hi Dmitry, > I've updated the files according to your notes, except > php_uint32/MD5_u32plus. ...and except for adding a note that I'm not the only author (not of the modified code). That's fine, if it's your preference, I don't care much either way. > However new version breaks ext/hash md4. (ext/hash/tests/md4.phpt is > broken). That's weird. Maybe you meant ext/hash/tests/md5.phpt, not md4? That would be somewhat weird, too, as ext/hash/hash_md.c contains its own copy of the MD5 code (to be replaced later) - but I suppose there could be a symbol clash. Did you check that PHP `md5()` still works correctly after your changes? (The patch I had submitted was fine in this respect, passing my tests.) Some more comments on the files: > * * This is an OpenSSL-compatible implementation of the RSA Data Security, > * * Inc. MD5 Message-Digest Algorithm (RFC 1321). > * * > * * Written by Solar Designer in 2001, and placed > * * in the public domain. There's absolutely no warranty. > * * > * * This differs from Colin Plumb's older public domain implementation in ... > * * and avoid compile-time configuration. Why are you re-formatting these comments in such a weird way, making lines exceed 80 chars for long comments? > PHPAPI void PHP_MD5Final(unsigned char *result, PHP_MD5_CTX *ctx) > { > php_uint32 used, free; These should be size_t, not php_uint32. > PHPAPI void make_digest_ex(char *md5str, const unsigned char *digest, int len); You could want to change this to "size_t len" as well, although this function existed before and used "int". Obviously, if you do change it, you need to do so in both files and perhaps also in some callers. Maybe this should be done with a separate commit (a change not directly related to replacing the MD5 implementation). Thanks, -- Alexander Peslyak GPG key ID: 5B341F15 fp: B3FB 63F4 D7A3 BCCC 6F6E FC55 A2FC 027C 5B34 1F15 http://www.openwall.com - bringing security into open computing environments --- ## #35291 — Solar Designer — 2008-02-07T11:16:25+00:00 URL: https://externals.io/message/33878#35291 · In reply to: #35288 A followup on my own posting: > ...and except for adding a note that I'm not the only author (not of the > modified code). That's fine, if it's your preference, I don't care much > either way. Actually, I am also not the author of some of the code that you're leaving intact in PHP's md5.c - that's functions make_digest(), make_digest_ex(), php_if_md5(), and php_if_md5_file(). My contribution is limited to replacing RSA's implementation of MD5 itself; I did not contribute replacements for these wrapper functions that are specific to PHP. The original PHP's md5.h had "Author: Rasmus Lerdorf" on it, but I am not sure what that applied to (the header file is small and is hardly subject to copyright). The original PHP's md5.c had: | Author: Lachlan Roche ... * md5.c - Copyright 1997 Lachlan Roche * `md5_file()` added by Alessandro Astarita I am not sure who the code that remains after our changes should be attributed to. Maybe you could check the commits history to figure this out, if it matters. One thing I'm sure of is that it's OK to drop RSA's copyright and license notice, because we're replacing that code in its entirety - and you already did. > > […] > > That's weird. Maybe you meant ext/hash/tests/md5.phpt, not md4? That > would be somewhat weird, too, as ext/hash/hash_md.c contains its own > copy of the MD5 code (to be replaced later) - but I suppose there could > be a symbol clash. I've recalled that the MD5 code in hash_md.c is within: #ifdef PHP_HASH_MD5_NOT_IN_CORE ... #endif /* PHP_HASH_MD5_NOT_IN_CORE */ So if our changes broke the MD5 implementation somehow, it is no surprise that ext/hash/tests/md5.phpt would fail. However, I've diff'ed the latest md5.[ch] that you posted against those I had submitted (and tested) and I failed to notice any obvious bugs (worse than the php_uint32 vs. size_t issue in PHP_MD5Final(), which should not cause those tests on short strings to fail). So this needs further testing on a clean build and, if necessary, debugging... maybe there's some issue specific to the program at large that is not obvious when looking just at the changes. Of course, I've been assuming that php_uint32 is in fact an unsigned 32-bit integer data type. Alexander --- ## #35292 — Solar Designer — 2008-02-07T11:21:09+00:00 URL: https://externals.io/message/33878#35292 · In reply to: #35285 > However new version breaks ext/hash md4. (ext/hash/tests/md4.phpt is > broken). Oh, I am able to reproduce this with my original patch: PASS hmac-md5 algorithm [ext/hash/tests/hmac-md5.phpt] PASS md2 algorithm [ext/hash/tests/md2.phpt] FAIL md4 algorithm [ext/hash/tests/md4.phpt] PASS md5 algorithm [ext/hash/tests/md5.phpt] PASS ripemd128 algorithm [ext/hash/tests/ripemd128.phpt] It didn't occur to me to pay attention to the MD4 test after patching MD5. Really weird. I'll look into it. Alexander --- ## #35294 — Solar Designer — 2008-02-07T11:50:27+00:00 URL: https://externals.io/message/33878#35294 · In reply to: #35292 I wrote: > Oh, I am able to reproduce this with my original patch: > > PASS hmac-md5 algorithm [ext/hash/tests/hmac-md5.phpt] > PASS md2 algorithm [ext/hash/tests/md2.phpt] > FAIL md4 algorithm [ext/hash/tests/md4.phpt] > PASS md5 algorithm [ext/hash/tests/md5.phpt] > PASS ripemd128 algorithm [ext/hash/tests/ripemd128.phpt] > > It didn't occur to me to pay attention to the MD4 test after patching > MD5. Really weird. I'll look into it. OK, I think I found it. ext/hash/php_hash_md.h has this: #define PHP_MD4Init PHP_MD5Init which breaks when the two implementations are not that similar anymore. Replacing the MD4 implementation with mine as well would fix this (or hide the bug, depending on your point of view), but for now I think the right fix would be to define a PHP_MD4Init() function explicitly. Alexander --- ## #35295 — Solar Designer — 2008-02-07T12:13:53+00:00 URL: https://externals.io/message/33878#35295 · In reply to: #35294 I wrote: > OK, I think I found it. ext/hash/php_hash_md.h has this: > > #define PHP_MD4Init PHP_MD5Init > > which breaks when the two implementations are not that similar anymore. > Replacing the MD4 implementation with mine as well would fix this (or > hide the bug, depending on your point of view), but for now I think the > right fix would be to define a PHP_MD4Init() function explicitly. The patch is attached. It contains two other tiny changes: 1. Replaces two of MD4's basic functions with more optimal versions (faster and smaller code). 2. Corrects a typo in a nearby comment. This patch may be applied independently and before the MD5 replacement patch - but it is required for the MD5 replacement patch. Alexander --- Next page: https://externals.io/message/33878.md?page=2