You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by David Shane Holden <dp...@yahoo.com> on 2002/07/09 20:40:45 UTC
[PATCH] mpm/winnt service permissions
This patch sets the calls to OpenSCManager and OpenService to use the
minimum required privileges.
Re: [PATCH] mpm/winnt service permissions
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
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
Posted by David Shane Holden <dp...@yahoo.com>.
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:dpejesh@yahoo.com]
>>Sent: Wednesday, July 10, 2002 2:28 AM
>>To: dev@httpd.apache.org
>>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
Posted by Mladen Turk <mt...@mappingsoft.com>.
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:dpejesh@yahoo.com]
> Sent: Wednesday, July 10, 2002 2:28 AM
> To: dev@httpd.apache.org
> 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
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
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
Posted by David Shane Holden <dp...@yahoo.com>.
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 -0000 1.56
>> +++ service.c 9 Jul 2002 18:02:38 -0000
>> @@ -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
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
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 -0000 1.56
>+++ service.c 9 Jul 2002 18:02:38 -0000
>@@ -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 */