Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
patch for trunk: Index: htdbm.c === --- htdbm.c(revision 526861) +++ htdbm.c(working copy) @@ -69,7 +69,7 @@ #define ALG_APMD5 1 #define ALG_APSHA 2 -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) #define ALG_CRYPT 3 #endif @@ -311,12 +311,12 @@ case ALG_PLAIN: /* XXX this len limitation is not in sync with any HTTPd len. */ apr_cpystrn(cpw,htdbm-userpass,sizeof(cpw)); -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) fprintf(stderr, Warning: Plain text passwords aren't supported by the server on this platform!\n); #endif break; -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) case ALG_CRYPT: (void) srand((int) time((time_t *) NULL)); to64(salt[0], rand(), 8); @@ -347,7 +347,7 @@ static void htdbm_usage(void) { -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) #define CRYPT_OPTION d #else #define CRYPT_OPTION @@ -367,7 +367,7 @@ fprintf(stderr,-c Create a new database.\n); fprintf(stderr,-n Don't update database; display results on stdout.\n); fprintf(stderr,-m Force MD5 encryption of the password (default).\n); -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) fprintf(stderr,-d Force CRYPT encryption of the password (now deprecated).\n); #endif fprintf(stderr,-p Do not encrypt the password (plaintext).\n); @@ -474,7 +474,7 @@ case 's': h-alg = ALG_APSHA; break; -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) case 'd': h-alg = ALG_CRYPT; break; On 4/4/07, Jeff Trawick [EMAIL PROTECTED] wrote: On 4/4/07, Jeff Trawick [EMAIL PROTECTED] wrote: On 3/23/07, David Jones [EMAIL PROTECTED] wrote: ok here's the simple patch at the 2.0.x level that just checks platforms for htdbm.c Can you post a post to htdbm.c at trunk? whoops, make that Can you post a PATCH... htdbm.trunk.patch Description: Binary data
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
On 4/9/07, David Jones [EMAIL PROTECTED] wrote: patch for trunk: thanks; committed I'll propose for backport to 2.2.x shortly.
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
On 3/23/07, David Jones [EMAIL PROTECTED] wrote: ok here's the simple patch at the 2.0.x level that just checks platforms for htdbm.c Can you post a post to htdbm.c at trunk?
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
On 4/4/07, Jeff Trawick [EMAIL PROTECTED] wrote: On 3/23/07, David Jones [EMAIL PROTECTED] wrote: ok here's the simple patch at the 2.0.x level that just checks platforms for htdbm.c Can you post a post to htdbm.c at trunk? whoops, make that Can you post a PATCH...
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
ok here's the simple patch at the 2.0.x level that just checks platforms for htdbm.c Also appended is the semi-related patch for htpasswd.c that adds TPF to the platforms checked in 2 cases where its missed, which seems like an oversight. === --- htdbm.c(revision 521875) +++ htdbm.c(working copy) @@ -66,7 +66,7 @@ #define ALG_APMD5 1 #define ALG_APSHA 2 -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) #define ALG_CRYPT 3 #endif @@ -309,7 +309,7 @@ /* XXX this len limitation is not in sync with any HTTPd len. */ apr_cpystrn(cpw,htdbm-userpass,sizeof(cpw)); break; -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) case ALG_CRYPT: (void) srand((int) time((time_t *) NULL)); to64(salt[0], rand(), 8); @@ -340,7 +340,7 @@ static void htdbm_usage(void) { -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) #define CRYPT_OPTION d #else #define CRYPT_OPTION @@ -360,7 +360,7 @@ fprintf(stderr,-c Create a new database.\n); fprintf(stderr,-n Don't update database; display results on stdout.\n); fprintf(stderr,-m Force MD5 encryption of the password (default).\n); -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) fprintf(stderr,-d Force CRYPT encryption of the password (now deprecated).\n); #endif fprintf(stderr,-p Do not encrypt the password (plaintext).\n); @@ -467,7 +467,7 @@ case 's': h-alg = ALG_APSHA; break; -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) case 'd': h-alg = ALG_CRYPT; break; On 3/23/07, Jeff Trawick [EMAIL PROTECTED] wrote: On 3/20/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: Jeff Trawick wrote: On 3/20/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: httpd does not ;-) httpd the project (vs. apr, apr-util), not httpd the program (vs. htdbm, htpasswd) as in In httpd, we don't call crypt(), we call APR... So... what I suggest is; 1. use the same test from htpasswd to determine if crypt is used for htdbm from 2.0 - 2.2-branch. E.g. which platforms? 2. use the APR_HAVE_CRYPT_H just to decide to include crypt.h, and the UNISTD test for unistd.h. 3. for trunk (2.4) forwards, add a new macro to APR trunk (1.3.x) that would 'reveal' if apr_password_* API's include crypt() support. Does that sound sane? sounds sane to me... looking for crypt() (issue #1) is cuter, but as it isn't the perfect solution anyway (issue #4) then don't bother with something that has a remote potential of hiding crypt() from somebody who has it today htdbm.checkplatforms.patch Description: Binary data htpasswd.missingTPF.patch Description: Binary data
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
David Jones wrote: ok here's the simple patch at the 2.0.x level that just checks platforms for htdbm.c Also appended is the semi-related patch for htpasswd.c that adds TPF to the platforms checked in 2 cases where its missed, which seems like an oversight. +1 === --- htdbm.c(revision 521875) +++ htdbm.c(working copy) @@ -66,7 +66,7 @@ #define ALG_APMD5 1 #define ALG_APSHA 2 -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) #define ALG_CRYPT 3 #endif @@ -309,7 +309,7 @@ /* XXX this len limitation is not in sync with any HTTPd len. */ apr_cpystrn(cpw,htdbm-userpass,sizeof(cpw)); break; -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) case ALG_CRYPT: (void) srand((int) time((time_t *) NULL)); to64(salt[0], rand(), 8); @@ -340,7 +340,7 @@ static void htdbm_usage(void) { -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) #define CRYPT_OPTION d #else #define CRYPT_OPTION @@ -360,7 +360,7 @@ fprintf(stderr,-c Create a new database.\n); fprintf(stderr,-n Don't update database; display results on stdout.\n); fprintf(stderr,-m Force MD5 encryption of the password (default).\n); -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) fprintf(stderr,-d Force CRYPT encryption of the password (now deprecated).\n); #endif fprintf(stderr,-p Do not encrypt the password (plaintext).\n); @@ -467,7 +467,7 @@ case 's': h-alg = ALG_APSHA; break; -#if APR_HAVE_CRYPT_H +#if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) case 'd': h-alg = ALG_CRYPT; break;
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
Re: Jeff's last note, yes httpd does call crypt() directly, Any other questions that need discussion / resolution? David Jones -- [EMAIL PROTECTED] On 3/16/07, Jeff Trawick [EMAIL PROTECTED] wrote: On 3/16/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: Jeff Trawick wrote: APR doesn't pretend to figure out for APR apps exactly what the system provides, though there is currently a spotty set of APR_HAS_foo. Meanwhile, httpd goes and searches on its own for things APR doesn't tell anyone about. I'm curious about other opinions on whether or not it is APR's job to tell what is available. In httpd, we don't call crypt(), we call APR maybe this is the point of confusion... httpd does call crypt() $ grep crypt support/*c ... support/htdbm.c:apr_cpystrn(cpw, crypt(htdbm-userpass, salt), sizeof(cpw) - 1); ... support/htpasswd.c:apr_cpystrn(cpw, crypt(pw, salt), sizeof(cpw) - 1); ...
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
httpd does not ;-) htdbm/htpasswd et al do. Bill [httpd-2.2]$ grep -r crypt modules/aaa/* [httpd-2.2]$ grep -r apr_password modules/aaa/* modules/aaa/mod_auth_basic.c:#include apr_md5.h/* for apr_password_validate */ modules/aaa/mod_authn_dbd.c:rv = apr_password_validate(password, dbd_password); modules/aaa/mod_authn_dbm.c:#include apr_md5.h/* for apr_password_validate */ modules/aaa/mod_authn_dbm.c:rv = apr_password_validate(password, dbm_password); modules/aaa/mod_authn_file.c:#include apr_md5.h/* for apr_password_validate */ modules/aaa/mod_authn_file.c:status = apr_password_validate(password, file_password); modules/aaa/mod_authz_default.c:#include apr_md5.h/* for apr_password_validate */ David Jones wrote: Re: Jeff's last note, yes httpd does call crypt() directly, Any other questions that need discussion / resolution? David Jones -- [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] On 3/16/07, *Jeff Trawick* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: On 3/16/07, William A. Rowe, Jr. [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: Jeff Trawick wrote: APR doesn't pretend to figure out for APR apps exactly what the system provides, though there is currently a spotty set of APR_HAS_foo. Meanwhile, httpd goes and searches on its own for things APR doesn't tell anyone about. I'm curious about other opinions on whether or not it is APR's job to tell what is available. In httpd, we don't call crypt(), we call APR maybe this is the point of confusion... httpd does call crypt() $ grep crypt support/*c ... support/htdbm.c:apr_cpystrn(cpw, crypt(htdbm-userpass, salt), sizeof(cpw) - 1); ... support/htpasswd.c:apr_cpystrn(cpw, crypt(pw, salt), sizeof(cpw) - 1); ...
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
On 3/20/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: httpd does not ;-) httpd the project (vs. apr, apr-util), not httpd the program (vs. htdbm, htpasswd) as in In httpd, we don't call crypt(), we call APR...
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
Jeff Trawick wrote: On 3/20/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: httpd does not ;-) httpd the project (vs. apr, apr-util), not httpd the program (vs. htdbm, htpasswd) as in In httpd, we don't call crypt(), we call APR... So... what I suggest is; 1. use the same test from htpasswd to determine if crypt is used for htdbm from 2.0 - 2.2-branch. E.g. which platforms? 2. use the APR_HAVE_CRYPT_H just to decide to include crypt.h, and the UNISTD test for unistd.h. 3. for trunk (2.4) forwards, add a new macro to APR trunk (1.3.x) that would 'reveal' if apr_password_* API's include crypt() support. Does that sound sane?
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
William A. Rowe, Jr. wrote: Jeff Trawick wrote: On 3/20/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: httpd does not ;-) httpd the project (vs. apr, apr-util), not httpd the program (vs. htdbm, htpasswd) as in In httpd, we don't call crypt(), we call APR... So... what I suggest is; 1. use the same test from htpasswd to determine if crypt is used for htdbm from 2.0 - 2.2-branch. E.g. which platforms? 2. use the APR_HAVE_CRYPT_H just to decide to include crypt.h, and the UNISTD test for unistd.h. 3. for trunk (2.4) forwards, add a new macro to APR trunk (1.3.x) that would 'reveal' if apr_password_* API's include crypt() support. Missed one 4. on trunk, complete the apr_password_* API's such that crypt() isn't needed by an app. With apr[-util] drawing in openssl as of 1.3.x, I plan to pick up the openssl des_crypt implementation. Platforms other than unix'es will be able to handle crypt passwords, and we can pitch the ones with the crypled crypt() implementations :)
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
Nope - it won't. Where does z/OS define the crypt() prototype? The correct patch is to ask APR_HAS_CRYPT (which we need to provide by patching apr, if we don't already.) If APR didn't detect crypt, adding crypt() style p/w's to htdbm, htpasswd etc will still be a noop. Bill David Jones wrote: Support utilities should enable crypt() iff it is available. Using the presence of crypt.h does not reliably determine if crypt() is available. Specifically z/OS supports crypt, but does not have crypt.h , so it is broken when checking APR_HAVE_CRYPT_H. Added crypt to AC_CHECK_FUNCS in httpd's configure.in http://configure.in , this creates a HAVE_CRYPT define. Then changed the checks in htpasswd.c and htdbm.c to check HAVE_CRYPT. This will let htdbm.c determine crypt() support accurately (it currently checks APR_HAVE_CRYPT_H) and htpasswd.c use a more concise and consistent check (it currently checks if OS = WIN32 || TPF || NETWARE) Note: This also fixes a TPF bug as they need to switch from crypt to MD5 like the other systems who don't have crypt: Currently the check to automatically switch from using crypt to md5 is: #if !(defined(WIN32) || defined(NETWARE)) All other checks for not supporting crypt in htdbm.c are: #if (!(defined(WIN32) || defined(TPF) || defined(NETWARE))) From the man page for htpasswd: -d Use crypt() encryption for passwords. The default on all plat- forms but Windows, Netware and TPF. Though possibly supported by htpasswd on all platforms, it is not supported by the httpd server on Windows, Netware and TPF Index: configure.in http://configure.in === --- configure.in http://configure.in(revision 518254) +++ configure.in http://configure.in(working copy) @@ -389,6 +389,7 @@ dnl ## Check for library functions AC_SEARCH_LIBS(sqrt, m) +AC_SEARCH_LIBS(crypt, crypt ufc) dnl See Comment #Spoon @@ -399,6 +400,7 @@ bindprocessor \ prctl \ timegm \ +crypt ) dnl confirm that a void pointer is large enough to store a long integer Index: support/htdbm.c === --- support/htdbm.c(revision 494665) +++ support/htdbm.c(working copy) @@ -29,6 +29,7 @@ #include apr_file_info.h #include apr_pools.h #include apr_signal.h +#include ap_config.h #include apr_md5.h #include apr_sha1.h #include apr_dbm.h @@ -69,7 +70,7 @@ #define ALG_APMD5 1 #define ALG_APSHA 2 -#if APR_HAVE_CRYPT_H +#ifdef HAVE_CRYPT #define ALG_CRYPT 3 #endif @@ -311,12 +312,12 @@ case ALG_PLAIN: /* XXX this len limitation is not in sync with any HTTPd len. */ apr_cpystrn(cpw,htdbm-userpass,sizeof(cpw)); -#if APR_HAVE_CRYPT_H +#ifdef HAVE_CRYPT fprintf(stderr, Warning: Plain text passwords aren't supported by the server on this platform!\n); #endif break; -#if APR_HAVE_CRYPT_H +#ifdef HAVE_CRYPT case ALG_CRYPT: (void) srand((int) time((time_t *) NULL)); to64(salt[0], rand(), 8); @@ -347,7 +348,7 @@ static void htdbm_usage(void) { -#if APR_HAVE_CRYPT_H +#ifdef HAVE_CRYPT #define CRYPT_OPTION d #else #define CRYPT_OPTION @@ -367,7 +368,7 @@ fprintf(stderr,-c Create a new database.\n); fprintf(stderr,-n Don't update database; display results on stdout.\n); fprintf(stderr,-m Force MD5 encryption of the password (default).\n); -#if APR_HAVE_CRYPT_H +#ifdef HAVE_CRYPT fprintf(stderr,-d Force CRYPT encryption of the password (now deprecated).\n); #endif fprintf(stderr,-p Do not encrypt the password (plaintext).\n); @@ -474,7 +475,7 @@ case 's': h-alg = ALG_APSHA; break; -#if APR_HAVE_CRYPT_H +#ifdef HAVE_CRYPT case 'd': h-alg = ALG_CRYPT; break; Index: support/htpasswd.c === --- support/htpasswd.c(revision 494665) +++ support/htpasswd.c(working copy) @@ -45,6 +45,7 @@ #include apr_file_io.h #include apr_general.h #include apr_signal.h +#include ap_config.h #if APR_HAVE_STDIO_H #include stdio.h @@ -175,7 +176,7 @@ apr_cpystrn(cpw,pw,sizeof(cpw)); break; -#if !(defined(WIN32) || defined(NETWARE)) +#ifdef HAVE_CRYPT case ALG_CRYPT: default: (void) srand((int) time((time_t *) NULL)); @@ -215,12 +216,12 @@ apr_file_printf(errfile, -n Don't update file; display results on stdout. NL); apr_file_printf(errfile, -m Force MD5 encryption of the password -#if defined(WIN32) || defined(TPF) || defined(NETWARE)
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
On 3/16/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: Nope - it won't. Where does z/OS define the crypt() prototype? unistd.h is the common place, z/OS or not. http://www.opengroup.org/onlinepubs/007908799/xsh/crypt.html Apparently crypt_r() is often defined in crypt.h but not in unistd.h. The correct patch is to ask APR_HAS_CRYPT (which we need to provide by patching apr, if we don't already.) APR doesn't pretend to figure out for APR apps exactly what the system provides, though there is currently a spotty set of APR_HAS_foo. Meanwhile, httpd goes and searches on its own for things APR doesn't tell anyone about. I'm curious about other opinions on whether or not it is APR's job to tell what is available. (Sometimes I long for something like an apr-bootstrap component which owns the job of figuring out the characteristics of a system, and exports that information to all comers using its own namespace. APR or other apps/libraries then can use the information. So the very essence of this component is to make that type of information available, and the odd inconsistency of what APR exports or doesn't is no more.)
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
Jeff Trawick wrote: APR doesn't pretend to figure out for APR apps exactly what the system provides, though there is currently a spotty set of APR_HAS_foo. Meanwhile, httpd goes and searches on its own for things APR doesn't tell anyone about. I'm curious about other opinions on whether or not it is APR's job to tell what is available. In httpd, we don't call crypt(), we call APR, so it's really necessary for APR to give us a clue about what it found out. I take it z/OS etc already pick up on crypt() from APR? Bill
Re: PATCH: support utilities should enable crypt() , current htdbm checks broken
On 3/16/07, William A. Rowe, Jr. [EMAIL PROTECTED] wrote: Jeff Trawick wrote: APR doesn't pretend to figure out for APR apps exactly what the system provides, though there is currently a spotty set of APR_HAS_foo. Meanwhile, httpd goes and searches on its own for things APR doesn't tell anyone about. I'm curious about other opinions on whether or not it is APR's job to tell what is available. In httpd, we don't call crypt(), we call APR maybe this is the point of confusion... httpd does call crypt() $ grep crypt support/*c ... support/htdbm.c:apr_cpystrn(cpw, crypt(htdbm-userpass, salt), sizeof(cpw) - 1); ... support/htpasswd.c:apr_cpystrn(cpw, crypt(pw, salt), sizeof(cpw) - 1); ...