Thread: stderr & win32 admin check
Per previous patch, win32 required the check for admin privs to be moved from main.c into postmaster.c, because elog was not available at this time. While working on fixing that all the way (moving the unix one as well), I realised this wasn't good, and did it this way instead: * Created function write_stderr(const char *fmt, ...), used before elog can be used. This function will write to stderr on unix and on win32 fconsole. It will write to the eventlog on win32 when running as a service. * Changed all (most? I think I got all) fprintf(stderr,...) to use this function instead. That way, we gain the ability to put all the other preivously-stderr-messages to the eventlog as well. * This also removes postmaster_error(), since it's mostly duplicate code with write_stderr(). This patch implements this, as well as adds the win32 check for admin privs to main.c. Attached file security.c goes in backend/port/win32. The patch makes the "if user is admin" check on win32 abort startup, similar to unix. I know several people have said they think this is wrong, since the last discussion. So if someone wants to re-open that discussion, go ahead :-) The behaviour can easily be adapted (just remove an exit(1) at a later date changing the message to a warning). //Magnus <<stderr.patch>>
Attachment
"Magnus Hagander" <mha@sollentuna.net> writes:
> * Created function write_stderr(const char *fmt, ...), used before elog
> can be used. This function will write to stderr on unix and on win32
> fconsole. It will write to the eventlog on win32 when running as a
> service.
> * Changed all (most? I think I got all) fprintf(stderr,...) to use this
> function instead. That way, we gain the ability to put all the other
> preivously-stderr-messages to the eventlog as well.
I'm not sure this is a good idea. The remaining uses of stderr were
that way for a reason, not because someone had forgot to change them
into elog calls. It would be a lot less invasive to just move the
privilege check as you originally intended.
regards, tom lane
>> * Created function write_stderr(const char *fmt, ...), used >before elog >> can be used. This function will write to stderr on unix and on win32 >> fconsole. It will write to the eventlog on win32 when running as a >> service. >> * Changed all (most? I think I got all) fprintf(stderr,...) >to use this >> function instead. That way, we gain the ability to put all the other >> preivously-stderr-messages to the eventlog as well. > >I'm not sure this is a good idea. The remaining uses of stderr were >that way for a reason, not because someone had forgot to change them >into elog calls. It would be a lot less invasive to just move the >privilege check as you originally intended. I figured as long as nothing "dangerous" (e.g. using memory allocations etc) is done in the function, it should be just as safe as fprintf. On Unix, it does nothing more than a simple fprintf anyway (one call deeper). The only difference in practice is that we can put them in the eventlog on win32 (again, only using calls that are safe in this context). If we do it the other way, we are going to lose these other messages when running as a service on win32 (since we specifically are not using ereport(), per what you say above). Also, this would remove the check so you could do initdb and other operations that are blocked today (that don't go through postmaster.c) when being root, I assumed that was not good either... //Magnus
> -----Original Message----- > From: pgsql-patches-owner@postgresql.org > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of > Magnus Hagander > Sent: 14 June 2004 21:49 > To: pgsql-patches@postgresql.org > Subject: [PATCHES] stderr & win32 admin check > > The patch makes the "if user is admin" check on win32 abort > startup, similar to unix. I know several people have said > they think this is wrong, since the last discussion. So if > someone wants to re-open that discussion, go ahead :-) OK, if you insist :-) This will prevent PostgreSQL being runable on NT4 by anyone with admin privileges, except as a service. NT4 doesn't have runas.exe (an su/sudo equivalent found on 2K/XP/2K3) thus preventing startup without logging in as a different user. I would guess that users with local admin privileges are far more common on Windows than users with root privileges on Unix systems, so this might affect more 'normal' users. Now I think of it, the default (and possibly only) user account on XP Home edition has admin privileges. I would suggest changing the error to a warning, or perhaps adding a command line switch to 'downgrade' the error to a warning, thus forcing an explicit action on the part of the user. Oh, and I notice the use of the PowerUsers group - iirc, there is no such group on NT4 domains, so the attempt to get the SID will fail. Regards, Dave.
<snip> > Oh, and I notice the use of the PowerUsers group - iirc, > there is no such group on NT4 domains, so the attempt to get > the SID will fail. That is one weird NT4.. :-) First of all, "Power Users" is not a domain group, it is a local group. It has nothing to do with your domain. As such, it will also work on non-domain-member servers/workstations. Second, the "Power Users" group have been included in the local groups since NT 3.1, IIRC (could be 3.5). It is most certainly there in NT4. This does not say anything about the other points you made - just this last one. //Magnus
> -----Original Message----- > From: Magnus Hagander [mailto:mha@sollentuna.net] > Sent: 15 June 2004 09:16 > To: Dave Page; pgsql-patches@postgresql.org > Subject: RE: [PATCHES] stderr & win32 admin check > > <snip> > > > > Oh, and I notice the use of the PowerUsers group - iirc, > there is no > > such group on NT4 domains, so the attempt to get the SID will fail. > > That is one weird NT4.. :-) > > First of all, "Power Users" is not a domain group, it is a > local group. > It has nothing to do with your domain. As such, it will also > work on non-domain-member servers/workstations. > > Second, the "Power Users" group have been included in the > local groups since NT 3.1, IIRC (could be 3.5). It is most > certainly there in NT4. Ack, you know I wrote that very late last night :-p You are of course correct that Power Users group is not a domain group. Wasn't aware that it was there in NT4 though - probably 'cos when I used to use NT4, it was on a small domain in which all servers were P/BDCs. Which don't have a power users group... /D
"Dave Page" <dpage@vale-housing.co.uk> writes:
> This will prevent PostgreSQL being runable on NT4 by anyone with admin
> privileges, except as a service.
Are we actually supporting NT4? I recall quite a bit of discussion long
ago about which versions of Windows were really reasonable to support,
but I don't recall if there was a definite outcome.
"Can't run Postgres securely" would be a more-than-sufficient reason
not to support NT4, IMHO.
> I would suggest changing the error to a warning, or perhaps adding a
> command line switch to 'downgrade' the error to a warning, thus forcing
> an explicit action on the part of the user.
See above. I don't want PG to become the next virus vector.
regards, tom lane
> > This will prevent PostgreSQL being runable on NT4 by anyone > with admin > > privileges, except as a service. > > Are we actually supporting NT4? I recall quite a bit of > discussion long ago about which versions of Windows were > really reasonable to support, but I don't recall if there was > a definite outcome. NT4 is on the list (for now). Nothing before it, and nothing from the 9x line. Meening NT4, 2000, XP and 2003 today. > "Can't run Postgres securely" would be a more-than-sufficient > reason not to support NT4, IMHO. It could still be run on NT4 under the following conditions: 1) Running as a service 2) Running if the user logged in is not an administrator. The difference is in 2000 and later you can log in with admin privs and then start postgres without it. //Magnus
"Magnus Hagander" <mha@sollentuna.net> writes:
>> "Can't run Postgres securely" would be a more-than-sufficient
>> reason not to support NT4, IMHO.
> It could still be run on NT4 under the following conditions:
> 1) Running as a service
> 2) Running if the user logged in is not an administrator.
Well, isn't "running as a service" sufficient? I thought that was the
only interesting case for non-hackers anyway.
As long as you get an error message that's reasonably clear about what
you can do instead, this hardly seems like a showstopper...
regards, tom lane
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > Sent: 15 June 2004 14:58 > To: Magnus Hagander > Cc: Dave Page; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] stderr & win32 admin check > > "Magnus Hagander" <mha@sollentuna.net> writes: > >> "Can't run Postgres securely" would be a > more-than-sufficient reason > >> not to support NT4, IMHO. > > > It could still be run on NT4 under the following conditions: > > 1) Running as a service > > 2) Running if the user logged in is not an administrator. > > Well, isn't "running as a service" sufficient? I thought > that was the only interesting case for non-hackers anyway. > > As long as you get an error message that's reasonably clear > about what you can do instead, this hardly seems like a showstopper... Well, that's kinda the point. If you are a hacker who has local admin privs (not exactly unusual on Windows networks - in some cases Power User group membership is required to run legacy software), you *cannot* run PostgreSQL except as a service, thus potentially making it a show stopper for those users. Personally I don't care as I use XP/2K3 anyway, but having been told my autovacuum service code needed to support NT4.... Regards, Dave
Dave Page wrote: >Personally I don't care as I use XP/2K3 anyway, but having been told my >autovacuum service code needed to support NT4.... > I have been working on integrating pg_autovacuum into the backend, and I have it working, I'm just trying to clean up some lose ends before I submit another patch. I think backend integration will eliminate the need for your autovacuum service patch no? Matthew
-----Original Message----- From: Matthew T. O'Connor [mailto:matthew@zeut.net] Sent: Tue 6/15/2004 4:06 PM To: Dave Page Cc: Tom Lane; Magnus Hagander; pgsql-patches@postgresql.org Subject: Re: [PATCHES] stderr & win32 admin check > I have been working on integrating pg_autovacuum into the backend, and I > have it working, I'm just trying to clean up some lose ends before I > submit another patch. I think backend integration will eliminate the > need for your autovacuum service patch no? Oh, of course. I wrote it 'just in case', and for a bit of experience before looking at the main server. Which Claudio thendid anyway! Regards, Dave
Dave Page wrote: >> >>>It could still be run on NT4 under the following conditions: >>>1) Running as a service >>>2) Running if the user logged in is not an administrator. >>> >>> >>Well, isn't "running as a service" sufficient? I thought >>that was the only interesting case for non-hackers anyway. >> >>As long as you get an error message that's reasonably clear >>about what you can do instead, this hardly seems like a showstopper... >> >> > >Well, that's kinda the point. If you are a hacker who has local admin >privs (not exactly unusual on Windows networks - in some cases Power >User group membership is required to run legacy software), > Not only legacy software... > you *cannot* >run PostgreSQL except as a service, thus potentially making it a show >stopper for those users. > > Actually I wouldn't expect a server to run as anything else but a service. Running a server from a command line is for debugging purposes only (in the win32 world). Thus I'd consider the non-admin check as acceptable, while quite irritating for most win32 users. Many win32 servers create an own account on installation or suggest to do so (instead of using the "Local System" account), this could make things more convenient for the average win32 user. Regards, Andreas Regards, Andreas
"Dave Page" <dpage@vale-housing.co.uk> writes:
> Well, that's kinda the point. If you are a hacker who has local admin
> privs (not exactly unusual on Windows networks - in some cases Power
> User group membership is required to run legacy software), you *cannot*
> run PostgreSQL except as a service, thus potentially making it a show
> stopper for those users.
So? I don't follow why "run it as a service" isn't a sufficient answer,
and indeed the preferred way to do it.
regards, tom lane
Tom Lane wrote: >"Dave Page" <dpage@vale-housing.co.uk> writes: > > >>Well, that's kinda the point. If you are a hacker who has local admin >>privs (not exactly unusual on Windows networks - in some cases Power >>User group membership is required to run legacy software), you *cannot* >>run PostgreSQL except as a service, thus potentially making it a show >>stopper for those users. >> >> > >So? I don't follow why "run it as a service" isn't a sufficient answer, >and indeed the preferred way to do it. > > > We don't know what the usage pattern is going to be on Windows - I think we need to keep it as flexible as possible consistent with good security. Bear in mind that Windows is effectively a one-console system - that's one reason why so many users just log on as Administrator - switching to another user is much more of a pain than it is on Unix. cheers andrew
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > Sent: 15 June 2004 19:11 > To: Dave Page > Cc: Magnus Hagander; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] stderr & win32 admin check > > So? I don't follow why "run it as a service" isn't a > sufficient answer, and indeed the preferred way to do it. It is the preferred method, however two reasons not to spring to mind: first, I bet you and most others on this list might not want to do that if forced to develop on a Windows box, and second, you can only run one instance as a service on a single machine. Like I said though, I run XP so it's not a problem for me - I'm just sticking up for the NT4 users it was decided we should support. Regards, Dave.
Andrew Dunstan <andrew@dunslane.net> writes:
> Tom Lane wrote:
>> So? I don't follow why "run it as a service" isn't a sufficient answer,
>> and indeed the preferred way to do it.
> We don't know what the usage pattern is going to be on Windows - I think
> we need to keep it as flexible as possible consistent with good
> security.
Sure, but I draw the line at running Postgres with admin privileges.
"Flexibility is more important than security" is exactly the mindset
that has gotten Microsoft into their current bed of nails.
The fact that there is a perfectly usable solution on NT4 (the oldest
Windows version we have any intention of supporting) seems enough to
me. There are more usable solutions on newer versions. Fine. But
nowhere in here do I see a sufficient reason to allow known-insecure
operating practices.
I might be more willing to listen to other opinions on this if I were
rejecting a somewhat smaller volume of Microsoft-security-hole-spawned
spam and viruses every day. But in the current environment I don't see
how any sane person can argue that allowing insecure operation of a
network-exposed service is acceptable behavior.
regards, tom lane
"Dave Page" <dpage@vale-housing.co.uk> writes:
>> So? I don't follow why "run it as a service" isn't a
>> sufficient answer, and indeed the preferred way to do it.
> It is the preferred method, however two reasons not to spring to mind:
> first, I bet you and most others on this list might not want to do that
> if forced to develop on a Windows box, and second, you can only run one
> instance as a service on a single machine.
If forced to develop on Windows, I certainly wouldn't hold still for
developing on NT4. This argument would have more force if it were
referring to a remotely current release, but that one is four Windows
revs back for pete's sake. It's not going to have every feature of
newer releases. This is one.
regards, tom lane
Dave Page wrote: > > > > you can only run one >instance as a service on a single machine. > If you mean only run one instance of postmaster as service, that's not true. If you like two pgsql servers (i.e. db clusters), you can install two services, both using the same binary with different cmd line arguments. Regards, Andreas
> -----Original Message----- > From: Andreas Pflug [mailto:pgadmin@pse-consulting.de] > Sent: 15 June 2004 22:28 > To: Dave Page > Cc: Tom Lane; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] stderr & win32 admin check > > Dave Page wrote: > > > > > > > > > you can only run one > >instance as a service on a single machine. > > > > If you mean only run one instance of postmaster as service, > that's not true. > If you like two pgsql servers (i.e. db clusters), you can > install two services, both using the same binary with > different cmd line arguments. In which case, what would 'net stop postgresql' do? What you suggest is correct if using something like the Cygwin service installer because you can specify unique service names etc. AFAIK there are no plans to allow such configurability in PostgreSQL - nor is that something you see in most other services. Regards, Dave.
>> If you mean only run one instance of postmaster as service, >> that's not true. >> If you like two pgsql servers (i.e. db clusters), you can >> install two services, both using the same binary with >> different cmd line arguments. > >In which case, what would 'net stop postgresql' do? What you suggest is >correct if using something like the Cygwin service installer >because you >can specify unique service names etc. AFAIK there are no plans to allow >such configurability in PostgreSQL - nor is that something you see in >most other services. 'net stop postgresql' would stop the instance named 'postgresql'. 'net stop someotherpostgresql' would stop the instance named 'someotherpostgresql'. It hasn't been discussed, but it would be fairly trivial to add this to the service installer. (A bit more work on the MSI installer, but we could do with that one just installing the default instance at least for starters). At least I don't htink it's in what Claudio has so far - Claudio? Lots of work to get into your framework? And FWIW, that's exactly what Microsoft SQL Server does when you install multiple instances on the same machine. //Magnus
>It hasn't been discussed, but it would be fairly trivial to >add this to the service installer. (A bit more work on the MSI >installer, but we could do with that one just installing the >default instance at least for starters). Correcting myself on this one - the MSI installer already supports instaling under any name you want. Only one, but you can pick the name (default being "postgresql <version>"). Since the service code is not in th ebackend yet, you can't actually install this, but the interface is there and ready. //Magnus
Magnus Hagander said: >>It hasn't been discussed, but it would be fairly trivial to >>add this to the service installer. (A bit more work on the MSI >>installer, but we could do with that one just installing the >>default instance at least for starters). > > Correcting myself on this one - the MSI installer already supports > instaling under any name you want. Only one, but you can pick the name > (default being "postgresql <version>"). > Since the service code is not in th ebackend yet, you can't actually > install this, but the interface is there and ready. > I have often seen multiple instances of a service under different names (think of srvany from the Resources Toolkit). I agree that the serice name should be configurable. Will the installer have a set of options? e.g. . install binaries . initdb . install/configure service? If so, maybe you could just run it again, missing out the install binaries step, to create another service instance. cheers andrew
Tom Lane said: > Andrew Dunstan <andrew@dunslane.net> writes: >> Tom Lane wrote: >>> So? I don't follow why "run it as a service" isn't a sufficient >>> answer, and indeed the preferred way to do it. > >> We don't know what the usage pattern is going to be on Windows - I >> think we need to keep it as flexible as possible consistent with good >> security. > > Sure, but I draw the line at running Postgres with admin privileges. > "Flexibility is more important than security" is exactly the mindset > that has gotten Microsoft into their current bed of nails. > That would be the reason for my qualifying phrase "consistent with good security". :-) cheers andrew
>>>It hasn't been discussed, but it would be fairly trivial to >>>add this to the service installer. (A bit more work on the MSI >>>installer, but we could do with that one just installing the >>>default instance at least for starters). >> >> Correcting myself on this one - the MSI installer already supports >> instaling under any name you want. Only one, but you can >pick the name >> (default being "postgresql <version>"). >> Since the service code is not in th ebackend yet, you can't actually >> install this, but the interface is there and ready. >> > >I have often seen multiple instances of a service under different names >(think of srvany from the Resources Toolkit). > >I agree that the serice name should be configurable. > >Will the installer have a set of options? e.g. >. install binaries >. initdb >. install/configure service? Yes. >If so, maybe you could just run it again, missing out the >install binaries >step, to create another service instance. Well, ATM it will detect that it's already installed, and offer to remove the program. But that can be changed to allow just this, yes. What it will not allow (per MSI specs AFAIK) is to install the same version in different places. But it can still install multiple services calling the same postmaster.exe with different arguments. I think :-) //Magnus
Dave Page wrote: > > -----Original Message----- > From: Matthew T. O'Connor [mailto:matthew@zeut.net] > Sent: Tue 6/15/2004 4:06 PM > To: Dave Page > Cc: Tom Lane; Magnus Hagander; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] stderr & win32 admin check > > > I have been working on integrating pg_autovacuum into the backend, and I > > have it working, I'm just trying to clean up some lose ends before I > > submit another patch. I think backend integration will eliminate the > > need for your autovacuum service patch no? > > Oh, of course. I wrote it 'just in case', and for a bit of experience before looking at the main server. Which Claudiothen did anyway! OK, I have removed your patches on this from my mailbox. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > Tom Lane wrote: > >> So? I don't follow why "run it as a service" isn't a sufficient answer, > >> and indeed the preferred way to do it. > > > We don't know what the usage pattern is going to be on Windows - I think > > we need to keep it as flexible as possible consistent with good > > security. > > Sure, but I draw the line at running Postgres with admin privileges. > "Flexibility is more important than security" is exactly the mindset > that has gotten Microsoft into their current bed of nails. Easy of use is also one of the reaons MS is so popular and on so many computers. I am not advocating we allow unsecure usage, but I am pointing out that security and easy of use are important. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
> At least I don't htink it's in what Claudio has so far - Claudio? Lots > of work to get into your framework? The original patch I submitted actually *required* a service name, allowing any number of postgres installations. I have no intention of removing that. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see <a href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em ailpolicy.html</a>
Magnus, where are we on this refactoring process. --------------------------------------------------------------------------- Magnus Hagander wrote: > >> * Created function write_stderr(const char *fmt, ...), used > >before elog > >> can be used. This function will write to stderr on unix and on win32 > >> fconsole. It will write to the eventlog on win32 when running as a > >> service. > >> * Changed all (most? I think I got all) fprintf(stderr,...) > >to use this > >> function instead. That way, we gain the ability to put all the other > >> preivously-stderr-messages to the eventlog as well. > > > >I'm not sure this is a good idea. The remaining uses of stderr were > >that way for a reason, not because someone had forgot to change them > >into elog calls. It would be a lot less invasive to just move the > >privilege check as you originally intended. > > > I figured as long as nothing "dangerous" (e.g. using memory allocations > etc) is done in the function, it should be just as safe as fprintf. On > Unix, it does nothing more than a simple fprintf anyway (one call > deeper). The only difference in practice is that we can put them in the > eventlog on win32 (again, only using calls that are safe in this > context). If we do it the other way, we are going to lose these other > messages when running as a service on win32 (since we specifically are > not using ereport(), per what you say above). > > Also, this would remove the check so you could do initdb and other > operations that are blocked today (that don't go through postmaster.c) > when being root, I assumed that was not good either... > > //Magnus > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
I plan to resubmit this patch shortly (hopefully during the weekend) including supprot for detecting if running as a service (and thus pick eventlog support). From what I can tell, the rest should be Ok to go, so expect a new one shortly. //Magnus >-----Original Message----- >From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > > > >Magnus, where are we on this refactoring process. > >--------------------------------------------------------------- >------------ > >Magnus Hagander wrote: >> >> * Created function write_stderr(const char *fmt, ...), used >> >before elog >> >> can be used. This function will write to stderr on unix >and on win32 >> >> fconsole. It will write to the eventlog on win32 when running as a >> >> service. >> >> * Changed all (most? I think I got all) fprintf(stderr,...) >> >to use this >> >> function instead. That way, we gain the ability to put >all the other >> >> preivously-stderr-messages to the eventlog as well. >> > >> >I'm not sure this is a good idea. The remaining uses of stderr were >> >that way for a reason, not because someone had forgot to change them >> >into elog calls. It would be a lot less invasive to just move the >> >privilege check as you originally intended. >> >> >> I figured as long as nothing "dangerous" (e.g. using memory >allocations >> etc) is done in the function, it should be just as safe as >fprintf. On >> Unix, it does nothing more than a simple fprintf anyway (one call >> deeper). The only difference in practice is that we can put >them in the >> eventlog on win32 (again, only using calls that are safe in this >> context). If we do it the other way, we are going to lose these other >> messages when running as a service on win32 (since we >specifically are >> not using ereport(), per what you say above). >> >> Also, this would remove the check so you could do initdb and other >> operations that are blocked today (that don't go through >postmaster.c) >> when being root, I assumed that was not good either... >> >> //Magnus >> >> ---------------------------(end of >broadcast)--------------------------- >> TIP 1: subscribe and unsubscribe commands go to >majordomo@postgresql.org >> > >-- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, >Pennsylvania 19073 >
Attached is the updated version of this patch, which now includes proper testing for win32 service running. This is tested and verified with Claudios service wrapper pg_ctl patch (including the parts I added and sent in a short while ago). security.c goes in backend/port/win32/ //Magnus >-----Original Message----- >From: Magnus Hagander >Sent: den 19 juni 2004 13:55 >To: Bruce Momjian >Cc: Tom Lane; pgsql-patches@postgresql.org >Subject: Re: [PATCHES] stderr & win32 admin check > > >I plan to resubmit this patch shortly (hopefully during the weekend) >including supprot for detecting if running as a service (and thus pick >eventlog support). From what I can tell, the rest should be Ok >to go, so >expect a new one shortly. > >//Magnus > >>-----Original Message----- >>From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] >> >> >> >>Magnus, where are we on this refactoring process. >> >>--------------------------------------------------------------- >>------------ >> >>Magnus Hagander wrote: >>> >> * Created function write_stderr(const char *fmt, ...), used >>> >before elog >>> >> can be used. This function will write to stderr on unix >>and on win32 >>> >> fconsole. It will write to the eventlog on win32 when >running as a >>> >> service. >>> >> * Changed all (most? I think I got all) fprintf(stderr,...) >>> >to use this >>> >> function instead. That way, we gain the ability to put >>all the other >>> >> preivously-stderr-messages to the eventlog as well. >>> > >>> >I'm not sure this is a good idea. The remaining uses of >stderr were >>> >that way for a reason, not because someone had forgot to >change them >>> >into elog calls. It would be a lot less invasive to just move the >>> >privilege check as you originally intended. >>> >>> >>> I figured as long as nothing "dangerous" (e.g. using memory >>allocations >>> etc) is done in the function, it should be just as safe as >>fprintf. On >>> Unix, it does nothing more than a simple fprintf anyway (one call >>> deeper). The only difference in practice is that we can put >>them in the >>> eventlog on win32 (again, only using calls that are safe in this >>> context). If we do it the other way, we are going to lose >these other >>> messages when running as a service on win32 (since we >>specifically are >>> not using ereport(), per what you say above). >>> >>> Also, this would remove the check so you could do initdb and other >>> operations that are blocked today (that don't go through >>postmaster.c) >>> when being root, I assumed that was not good either... >>> >>> //Magnus >>> >>> ---------------------------(end of >>broadcast)--------------------------- >>> TIP 1: subscribe and unsubscribe commands go to >>majordomo@postgresql.org >>> >> >>-- >> Bruce Momjian | http://candle.pha.pa.us >> pgman@candle.pha.pa.us | (610) 359-1001 >> + If your life is a hard drive, | 13 Roberts Road >> + Christ can be your backup. | Newtown Square, >>Pennsylvania 19073 >> > >---------------------------(end of >broadcast)--------------------------- >TIP 8: explain analyze is your friend >
Attachment
You probably would've been a lot less confused if I had actually included the *patch* along with the C file.. Sorry! //Magnus >-----Original Message----- >From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] >Sent: den 20 juni 2004 20:27 >To: Magnus Hagander >Cc: Tom Lane; pgsql-patches@postgresql.org >Subject: Re: [PATCHES] stderr & win32 admin check > > > >I am confused. There are no hooks to call this function right now. Is >it called by Claudio's patch? > >--------------------------------------------------------------- >------------ > >Magnus Hagander wrote: >> Attached is the updated version of this patch, which now >includes proper >> testing for win32 service running. This is tested and verified with >> Claudios service wrapper pg_ctl patch (including the parts I >added and >> sent in a short while ago). >> >> security.c goes in backend/port/win32/ >> >> //Magnus >> >> >> >-----Original Message----- >> >From: Magnus Hagander >> >Sent: den 19 juni 2004 13:55 >> >To: Bruce Momjian >> >Cc: Tom Lane; pgsql-patches@postgresql.org >> >Subject: Re: [PATCHES] stderr & win32 admin check >> > >> > >> >I plan to resubmit this patch shortly (hopefully during the weekend) >> >including supprot for detecting if running as a service >(and thus pick >> >eventlog support). From what I can tell, the rest should be Ok >> >to go, so >> >expect a new one shortly. >> > >> >//Magnus >> > >> >>-----Original Message----- >> >>From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] >> >> >> >> >> >> >> >>Magnus, where are we on this refactoring process. >> >> >> >>--------------------------------------------------------------- >> >>------------ >> >> >> >>Magnus Hagander wrote: >> >>> >> * Created function write_stderr(const char *fmt, ...), used >> >>> >before elog >> >>> >> can be used. This function will write to stderr on unix >> >>and on win32 >> >>> >> fconsole. It will write to the eventlog on win32 when >> >running as a >> >>> >> service. >> >>> >> * Changed all (most? I think I got all) fprintf(stderr,...) >> >>> >to use this >> >>> >> function instead. That way, we gain the ability to put >> >>all the other >> >>> >> preivously-stderr-messages to the eventlog as well. >> >>> > >> >>> >I'm not sure this is a good idea. The remaining uses of >> >stderr were >> >>> >that way for a reason, not because someone had forgot to >> >change them >> >>> >into elog calls. It would be a lot less invasive to >just move the >> >>> >privilege check as you originally intended. >> >>> >> >>> >> >>> I figured as long as nothing "dangerous" (e.g. using memory >> >>allocations >> >>> etc) is done in the function, it should be just as safe as >> >>fprintf. On >> >>> Unix, it does nothing more than a simple fprintf anyway (one call >> >>> deeper). The only difference in practice is that we can put >> >>them in the >> >>> eventlog on win32 (again, only using calls that are safe in this >> >>> context). If we do it the other way, we are going to lose >> >these other >> >>> messages when running as a service on win32 (since we >> >>specifically are >> >>> not using ereport(), per what you say above). >> >>> >> >>> Also, this would remove the check so you could do initdb >and other >> >>> operations that are blocked today (that don't go through >> >>postmaster.c) >> >>> when being root, I assumed that was not good either... >> >>> >> >>> //Magnus >> >>> >> >>> ---------------------------(end of >> >>broadcast)--------------------------- >> >>> TIP 1: subscribe and unsubscribe commands go to >> >>majordomo@postgresql.org >> >>> >> >> >> >>-- >> >> Bruce Momjian | http://candle.pha.pa.us >> >> pgman@candle.pha.pa.us | (610) 359-1001 >> >> + If your life is a hard drive, | 13 Roberts Road >> >> + Christ can be your backup. | Newtown Square, >> >>Pennsylvania 19073 >> >> >> > >> >---------------------------(end of >> >broadcast)--------------------------- >> >TIP 8: explain analyze is your friend >> > > >Content-Description: security.c > >[ Attachment, skipping... ] > >-- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, >Pennsylvania 19073 >
Attachment
I am confused. There are no hooks to call this function right now. Is it called by Claudio's patch? --------------------------------------------------------------------------- Magnus Hagander wrote: > Attached is the updated version of this patch, which now includes proper > testing for win32 service running. This is tested and verified with > Claudios service wrapper pg_ctl patch (including the parts I added and > sent in a short while ago). > > security.c goes in backend/port/win32/ > > //Magnus > > > >-----Original Message----- > >From: Magnus Hagander > >Sent: den 19 juni 2004 13:55 > >To: Bruce Momjian > >Cc: Tom Lane; pgsql-patches@postgresql.org > >Subject: Re: [PATCHES] stderr & win32 admin check > > > > > >I plan to resubmit this patch shortly (hopefully during the weekend) > >including supprot for detecting if running as a service (and thus pick > >eventlog support). From what I can tell, the rest should be Ok > >to go, so > >expect a new one shortly. > > > >//Magnus > > > >>-----Original Message----- > >>From: Bruce Momjian [mailto:pgman@candle.pha.pa.us] > >> > >> > >> > >>Magnus, where are we on this refactoring process. > >> > >>--------------------------------------------------------------- > >>------------ > >> > >>Magnus Hagander wrote: > >>> >> * Created function write_stderr(const char *fmt, ...), used > >>> >before elog > >>> >> can be used. This function will write to stderr on unix > >>and on win32 > >>> >> fconsole. It will write to the eventlog on win32 when > >running as a > >>> >> service. > >>> >> * Changed all (most? I think I got all) fprintf(stderr,...) > >>> >to use this > >>> >> function instead. That way, we gain the ability to put > >>all the other > >>> >> preivously-stderr-messages to the eventlog as well. > >>> > > >>> >I'm not sure this is a good idea. The remaining uses of > >stderr were > >>> >that way for a reason, not because someone had forgot to > >change them > >>> >into elog calls. It would be a lot less invasive to just move the > >>> >privilege check as you originally intended. > >>> > >>> > >>> I figured as long as nothing "dangerous" (e.g. using memory > >>allocations > >>> etc) is done in the function, it should be just as safe as > >>fprintf. On > >>> Unix, it does nothing more than a simple fprintf anyway (one call > >>> deeper). The only difference in practice is that we can put > >>them in the > >>> eventlog on win32 (again, only using calls that are safe in this > >>> context). If we do it the other way, we are going to lose > >these other > >>> messages when running as a service on win32 (since we > >>specifically are > >>> not using ereport(), per what you say above). > >>> > >>> Also, this would remove the check so you could do initdb and other > >>> operations that are blocked today (that don't go through > >>postmaster.c) > >>> when being root, I assumed that was not good either... > >>> > >>> //Magnus > >>> > >>> ---------------------------(end of > >>broadcast)--------------------------- > >>> TIP 1: subscribe and unsubscribe commands go to > >>majordomo@postgresql.org > >>> > >> > >>-- > >> Bruce Momjian | http://candle.pha.pa.us > >> pgman@candle.pha.pa.us | (610) 359-1001 > >> + If your life is a hard drive, | 13 Roberts Road > >> + Christ can be your backup. | Newtown Square, > >>Pennsylvania 19073 > >> > > > >---------------------------(end of > >broadcast)--------------------------- > >TIP 8: explain analyze is your friend > > Content-Description: security.c [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
"Magnus Hagander" <mha@sollentuna.net> writes:
> Attached is the updated version of this patch, which now includes proper
> testing for win32 service running. This is tested and verified with
> Claudios service wrapper pg_ctl patch (including the parts I added and
> sent in a short while ago).
Applied.
regards, tom lane