Re: [PATCH] mpm/winnt service permissions
At 12:54 PM 7/10/2002, you wrote: > That's the responsibility of Windows. By forcing admin privileges to call >apache -k * isn't creating any kind of security. Anybody could create a >simple >five like program or open up services from the control panel to control apache >if their account has the rights to do so. Just because apache.exe and AM >forces >admin requirements, the system does not. > >But I think I see what you're saying and to enforce that we'd need to add >account >checking to the startup code, not the service control code. We aren't enforcing anything. What we've tried to do is to assure that AM and apache -k foo will do what they are -allowed- to do under the user's current permissions, crippling only the features that the user is -denied-. So if the Apache service is set up to run as effectively nobody, it won't be fixing the service 'Description' to the server string. Big deal. That shouldn't mean it fails, only that the one feature can't be supported [and we should continue.] Apache can and does only does what it's allowed to do. Bill
Re: [PATCH] mpm/winnt service permissions
That's the responsibility of Windows. By forcing admin privileges to call apache -k * isn't creating any kind of security. Anybody could create a simple five like program or open up services from the control panel to control apache if their account has the rights to do so. Just because apache.exe and AM forces admin requirements, the system does not. But I think I see what you're saying and to enforce that we'd need to add account checking to the startup code, not the service control code. Shane Mladen Turk wrote: >Just one thought :-) > >I think that at least Administrator privileges are needed to start the >services. >The ApacheMonitor will definitely need that once when async behavior >will be used, so that calls for starting services gets serialized with >LockServiceDatabase that needs Admin privileges. >So I'm for the GENERIC_READ/GENERIC_WRITE/GENERIC_EXECUTE generic access >types, and not for finding security holes. Neither AM nor Apache >shouldn't brake that allowing starting or stopping something that cannot >be done through Service Manager itself, and should report that as access >violation errors. > >MT. > > > >>-Original Message- >>From: David Shane Holden [mailto:[EMAIL PROTECTED]] >>Sent: Wednesday, July 10, 2002 2:28 AM >>To: [EMAIL PROTECTED] >>Subject: Re: [PATCH] mpm/winnt service permissions >> >> >>Correct me if I'm wrong, but it sounds like you think this is for >>ApacheMonitor. This is for the winnt mpm itself. >>I thought your patch this morning was for the mpm just as I >>believe you >>think this is for the monitor. >> >>Shane >> >> >>William A. Rowe, Jr. wrote: >> >> >> >>>At 01:40 PM 7/9/2002, you wrote: >>> >>> >>> >>>>This patch sets the calls to OpenSCManager and OpenService >>>> >>>> >>to use the >> >> >>>>minimum required privileges. >>>> >>>> >>>Cool. Could you cvs up to grab the latest version with Mladen's >>>patch, compare your suggested changes to his latest changes for >>>requested privileges, and provide an updated patch to discuss? >>> >>>Bill >>> >>> >>> > > > >>>>- SC_MANAGER_ALL_ACCESS); >>>>+ SC_MANAGER_CONNECT); >>>> if (!schSCManager) { >>>> rv = apr_get_os_error(); >>>> ap_log_error(APLOG_MARK, APLOG_ERR | >>>> >>>> >>APLOG_STARTUP, rv, >> >> >>>>NULL, >>>>@@ -1265,7 +1262,7 @@ >>>> SC_HANDLE schSCManager; >>>> >>>> schSCManager = OpenSCManager(NULL, NULL, // >>>> >>>> >>default machine >> >> >>>>& database >>>>- SC_MANAGER_ALL_ACCESS); >>>>+ SC_MANAGER_CONNECT); >>>> >>>> if (!schSCManager) { >>>> ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, >>>>apr_get_os_error(), NULL, >>>>@@ -1275,7 +1272,8 @@ >>>> >>>> /* ###: utf-ize */ >>>> schService = OpenService(schSCManager, mpm_service_name, >>>>- SERVICE_ALL_ACCESS); >>>>+ SERVICE_INTERROGATE | >>>>SERVICE_QUERY_STATUS | >>>>+ SERVICE_START | SERVICE_STOP); >>>> >>>> if (schService == NULL) { >>>> /* Could not open the service */ >>>> >>>> >>> >>> >> >> > > >
RE: [PATCH] mpm/winnt service permissions
Just one thought :-) I think that at least Administrator privileges are needed to start the services. The ApacheMonitor will definitely need that once when async behavior will be used, so that calls for starting services gets serialized with LockServiceDatabase that needs Admin privileges. So I'm for the GENERIC_READ/GENERIC_WRITE/GENERIC_EXECUTE generic access types, and not for finding security holes. Neither AM nor Apache shouldn't brake that allowing starting or stopping something that cannot be done through Service Manager itself, and should report that as access violation errors. MT. > -Original Message- > From: David Shane Holden [mailto:[EMAIL PROTECTED]] > Sent: Wednesday, July 10, 2002 2:28 AM > To: [EMAIL PROTECTED] > Subject: Re: [PATCH] mpm/winnt service permissions > > > Correct me if I'm wrong, but it sounds like you think this is for > ApacheMonitor. This is for the winnt mpm itself. > I thought your patch this morning was for the mpm just as I > believe you > think this is for the monitor. > > Shane > > > William A. Rowe, Jr. wrote: > > > At 01:40 PM 7/9/2002, you wrote: > > > >> This patch sets the calls to OpenSCManager and OpenService > to use the > >> minimum required privileges. > > > > > > Cool. Could you cvs up to grab the latest version with Mladen's > > patch, compare your suggested changes to his latest changes for > > requested privileges, and provide an updated patch to discuss? > > > > Bill > > > >> - SC_MANAGER_ALL_ACCESS); > >> + SC_MANAGER_CONNECT); > >> if (!schSCManager) { > >> rv = apr_get_os_error(); > >> ap_log_error(APLOG_MARK, APLOG_ERR | > APLOG_STARTUP, rv, > >> NULL, > >> @@ -1265,7 +1262,7 @@ > >> SC_HANDLE schSCManager; > >> > >> schSCManager = OpenSCManager(NULL, NULL, // > default machine > >> & database > >> - SC_MANAGER_ALL_ACCESS); > >> + SC_MANAGER_CONNECT); > >> > >> if (!schSCManager) { > >> ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, > >> apr_get_os_error(), NULL, > >> @@ -1275,7 +1272,8 @@ > >> > >> /* ###: utf-ize */ > >> schService = OpenService(schSCManager, mpm_service_name, > >> - SERVICE_ALL_ACCESS); > >> + SERVICE_INTERROGATE | > >> SERVICE_QUERY_STATUS | > >> + SERVICE_START | SERVICE_STOP); > >> > >> if (schService == NULL) { > >> /* Could not open the service */ > > > > > > > > > >
Re: [PATCH] mpm/winnt service permissions
At 07:27 PM 7/9/2002, you wrote: >Correct me if I'm wrong, but it sounds like you think this is for >ApacheMonitor. This is for the winnt mpm itself. >I thought your patch this morning was for the mpm just as I believe you >think this is for the monitor. Yup... That would be it. Thanks for the patch, I'll review. Bill
Re: [PATCH] mpm/winnt service permissions
Correct me if I'm wrong, but it sounds like you think this is for ApacheMonitor. This is for the winnt mpm itself. I thought your patch this morning was for the mpm just as I believe you think this is for the monitor. Shane William A. Rowe, Jr. wrote: > At 01:40 PM 7/9/2002, you wrote: > >> This patch sets the calls to OpenSCManager and OpenService to use the >> minimum required privileges. > > > Cool. Could you cvs up to grab the latest version with Mladen's patch, > compare your suggested changes to his latest changes for requested > privileges, and provide an updated patch to discuss? > > Bill > > >> Index: service.c >> === >> RCS file: /home/cvspublic/httpd-2.0/server/mpm/winnt/service.c,v >> retrieving revision 1.56 >> diff -u -3 -r1.56 service.c >> --- service.c 2 Jul 2002 19:03:15 - 1.56 >> +++ service.c 9 Jul 2002 18:02:38 - >> @@ -483,10 +483,10 @@ >> if ((osver.dwPlatformId == VER_PLATFORM_WIN32_NT) >>&& (osver.dwMajorVersion > 4) >>&& (ChangeServiceConfig2) >> - && (schSCManager = OpenSCManager(NULL, NULL, >> SC_MANAGER_ALL_ACCESS))) >> + && (schSCManager = OpenSCManager(NULL, NULL, >> SC_MANAGER_CONNECT))) >> { >> SC_HANDLE schService = OpenService(schSCManager, >> mpm_service_name, >> - SERVICE_ALL_ACCESS); >> + SERVICE_CHANGE_CONFIG); >> if (schService) { >> /* Cast is necessary, ChangeServiceConfig2 handles multiple >> * object types, some volatile, some not. >> @@ -854,10 +854,9 @@ >> { >> SC_HANDLE schService; >> SC_HANDLE schSCManager; >> - >> -// TODO: Determine the minimum permissions required for >> security >> + >> schSCManager = OpenSCManager(NULL, NULL, /* local, default >> database */ >> - SC_MANAGER_ALL_ACCESS); >> + SC_MANAGER_CREATE_SERVICE); >> if (!schSCManager) { >> rv = apr_get_os_error(); >> ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, >> NULL, >> @@ -870,7 +869,7 @@ >> if (reconfig) { >> /* ###: utf-ize */ >> schService = OpenService(schSCManager, mpm_service_name, >> - SERVICE_ALL_ACCESS); >> + SERVICE_CHANGE_CONFIG); >> if (!schService) { >> ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_ERR, >> apr_get_os_error(), NULL, >> @@ -1008,9 +1007,8 @@ >> >> fprintf(stderr,"Removing the %s service\n", mpm_display_name); >> >> -// TODO: Determine the minimum permissions required for >> security >> schSCManager = OpenSCManager(NULL, NULL, /* local, default >> database */ >> - SC_MANAGER_ALL_ACCESS); >> + SC_MANAGER_CONNECT); >> if (!schSCManager) { >> rv = apr_get_os_error(); >> ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, >> NULL, >> @@ -1019,7 +1017,7 @@ >> } >> >> /* ###: utf-ize */ >> -schService = OpenService(schSCManager, mpm_service_name, >> SERVICE_ALL_ACCESS); >> +schService = OpenService(schSCManager, mpm_service_name, >> DELETE); >> >> if (!schService) { >> rv = apr_get_os_error(); >> @@ -1123,9 +1121,8 @@ >> SC_HANDLE schService; >> SC_HANDLE schSCManager; >> >> -// TODO: Determine the minimum permissions required for >> security >> schSCManager = OpenSCManager(NULL, NULL, /* local, default >> database */ >> - SC_MANAGER_ALL_ACCESS); >> + SC_MANAGER_CONNECT); >> if (!schSCManager) { >> rv = apr_get_os_error(); >> ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, >> NULL, >> @@ -1265,7 +1262,7 @@ >> SC_HANDLE schSCManager; >> >> schSCManager = OpenSCManager(NULL, NULL, // default machine >> & database >> - SC_MANAGER_ALL_ACCESS); >> + SC_MANAGER_CONNECT); >> >> if (!schSCManager) { >> ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, >> apr_get_os_error(), NULL, >> @@ -1275,7 +1272,8 @@ >> >> /* ###: utf-ize */ >> schService = OpenService(schSCManager, mpm_service_name, >> - SERVICE_ALL_ACCESS); >> + SERVICE_INTERROGATE | >> SERVICE_QUERY_STATUS | >> + SERVICE_START | SERVICE_STOP); >> >> if (schService == NULL) { >> /* Could not open the service */ > > >
Re: [PATCH] mpm/winnt service permissions
At 01:40 PM 7/9/2002, you wrote: >This patch sets the calls to OpenSCManager and OpenService to use the >minimum required privileges. Cool. Could you cvs up to grab the latest version with Mladen's patch, compare your suggested changes to his latest changes for requested privileges, and provide an updated patch to discuss? Bill >Index: service.c >=== >RCS file: /home/cvspublic/httpd-2.0/server/mpm/winnt/service.c,v >retrieving revision 1.56 >diff -u -3 -r1.56 service.c >--- service.c 2 Jul 2002 19:03:15 - 1.56 >+++ service.c 9 Jul 2002 18:02:38 - >@@ -483,10 +483,10 @@ > if ((osver.dwPlatformId == VER_PLATFORM_WIN32_NT) >&& (osver.dwMajorVersion > 4) >&& (ChangeServiceConfig2) >- && (schSCManager = OpenSCManager(NULL, NULL, >SC_MANAGER_ALL_ACCESS))) >+ && (schSCManager = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT))) > { > SC_HANDLE schService = OpenService(schSCManager, mpm_service_name, >- SERVICE_ALL_ACCESS); >+ SERVICE_CHANGE_CONFIG); > if (schService) { > /* Cast is necessary, ChangeServiceConfig2 handles multiple > * object types, some volatile, some not. >@@ -854,10 +854,9 @@ > { > SC_HANDLE schService; > SC_HANDLE schSCManager; >- >-// TODO: Determine the minimum permissions required for security >+ > schSCManager = OpenSCManager(NULL, NULL, /* local, default > database */ >- SC_MANAGER_ALL_ACCESS); >+ SC_MANAGER_CREATE_SERVICE); > if (!schSCManager) { > rv = apr_get_os_error(); > ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, NULL, >@@ -870,7 +869,7 @@ > if (reconfig) { > /* ###: utf-ize */ > schService = OpenService(schSCManager, mpm_service_name, >- SERVICE_ALL_ACCESS); >+ SERVICE_CHANGE_CONFIG); > if (!schService) { > ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_ERR, > apr_get_os_error(), NULL, >@@ -1008,9 +1007,8 @@ > > fprintf(stderr,"Removing the %s service\n", mpm_display_name); > >-// TODO: Determine the minimum permissions required for security > schSCManager = OpenSCManager(NULL, NULL, /* local, default > database */ >- SC_MANAGER_ALL_ACCESS); >+ SC_MANAGER_CONNECT); > if (!schSCManager) { > rv = apr_get_os_error(); > ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, NULL, >@@ -1019,7 +1017,7 @@ > } > > /* ###: utf-ize */ >-schService = OpenService(schSCManager, mpm_service_name, >SERVICE_ALL_ACCESS); >+schService = OpenService(schSCManager, mpm_service_name, DELETE); > > if (!schService) { > rv = apr_get_os_error(); >@@ -1123,9 +1121,8 @@ > SC_HANDLE schService; > SC_HANDLE schSCManager; > >-// TODO: Determine the minimum permissions required for security > schSCManager = OpenSCManager(NULL, NULL, /* local, default > database */ >- SC_MANAGER_ALL_ACCESS); >+ SC_MANAGER_CONNECT); > if (!schSCManager) { > rv = apr_get_os_error(); > ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, NULL, >@@ -1265,7 +1262,7 @@ > SC_HANDLE schSCManager; > > schSCManager = OpenSCManager(NULL, NULL, // default machine & > database >- SC_MANAGER_ALL_ACCESS); >+ SC_MANAGER_CONNECT); > > if (!schSCManager) { > ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, > apr_get_os_error(), NULL, >@@ -1275,7 +1272,8 @@ > > /* ###: utf-ize */ > schService = OpenService(schSCManager, mpm_service_name, >- SERVICE_ALL_ACCESS); >+ SERVICE_INTERROGATE | >SERVICE_QUERY_STATUS | >+ SERVICE_START | SERVICE_STOP); > > if (schService == NULL) { > /* Could not open the service */
[PATCH] mpm/winnt service permissions
This patch sets the calls to OpenSCManager and OpenService to use the minimum required privileges. Index: service.c === RCS file: /home/cvspublic/httpd-2.0/server/mpm/winnt/service.c,v retrieving revision 1.56 diff -u -3 -r1.56 service.c --- service.c 2 Jul 2002 19:03:15 - 1.56 +++ service.c 9 Jul 2002 18:02:38 - @@ -483,10 +483,10 @@ if ((osver.dwPlatformId == VER_PLATFORM_WIN32_NT) && (osver.dwMajorVersion > 4) && (ChangeServiceConfig2) - && (schSCManager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS))) + && (schSCManager = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT))) { SC_HANDLE schService = OpenService(schSCManager, mpm_service_name, - SERVICE_ALL_ACCESS); + SERVICE_CHANGE_CONFIG); if (schService) { /* Cast is necessary, ChangeServiceConfig2 handles multiple * object types, some volatile, some not. @@ -854,10 +854,9 @@ { SC_HANDLE schService; SC_HANDLE schSCManager; - -// TODO: Determine the minimum permissions required for security + schSCManager = OpenSCManager(NULL, NULL, /* local, default database */ - SC_MANAGER_ALL_ACCESS); + SC_MANAGER_CREATE_SERVICE); if (!schSCManager) { rv = apr_get_os_error(); ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, NULL, @@ -870,7 +869,7 @@ if (reconfig) { /* ###: utf-ize */ schService = OpenService(schSCManager, mpm_service_name, - SERVICE_ALL_ACCESS); + SERVICE_CHANGE_CONFIG); if (!schService) { ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_ERR, apr_get_os_error(), NULL, @@ -1008,9 +1007,8 @@ fprintf(stderr,"Removing the %s service\n", mpm_display_name); -// TODO: Determine the minimum permissions required for security schSCManager = OpenSCManager(NULL, NULL, /* local, default database */ - SC_MANAGER_ALL_ACCESS); + SC_MANAGER_CONNECT); if (!schSCManager) { rv = apr_get_os_error(); ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, NULL, @@ -1019,7 +1017,7 @@ } /* ###: utf-ize */ -schService = OpenService(schSCManager, mpm_service_name, SERVICE_ALL_ACCESS); +schService = OpenService(schSCManager, mpm_service_name, DELETE); if (!schService) { rv = apr_get_os_error(); @@ -1123,9 +1121,8 @@ SC_HANDLE schService; SC_HANDLE schSCManager; -// TODO: Determine the minimum permissions required for security schSCManager = OpenSCManager(NULL, NULL, /* local, default database */ - SC_MANAGER_ALL_ACCESS); + SC_MANAGER_CONNECT); if (!schSCManager) { rv = apr_get_os_error(); ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, rv, NULL, @@ -1265,7 +1262,7 @@ SC_HANDLE schSCManager; schSCManager = OpenSCManager(NULL, NULL, // default machine & database - SC_MANAGER_ALL_ACCESS); + SC_MANAGER_CONNECT); if (!schSCManager) { ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_STARTUP, apr_get_os_error(), NULL, @@ -1275,7 +1272,8 @@ /* ###: utf-ize */ schService = OpenService(schSCManager, mpm_service_name, - SERVICE_ALL_ACCESS); + SERVICE_INTERROGATE | SERVICE_QUERY_STATUS | + SERVICE_START | SERVICE_STOP); if (schService == NULL) { /* Could not open the service */