Re: Small security patches
On Thu, Dec 12, 2002 at 11:34:02AM -0500, Pierre A. Humblet wrote:
Christopher Faylor wrote:
Actually, if you can get away without using a
constructor that would be best. Constructors are a noticeable part of
cygwin's startup cost.
- Is there a C++ way to initialize a constant class and have it in the .text
section, as const int i = 1; would be?
- If not, I can get the desired effect by using gcc Asm Labels, like
int foo asm (myfoo) = 2;
Would that be acceptable in Cygwin?
What about this idea:
Add a static method init() called from . Init() checks if it has been
called already before and returns immendiately if so. Otherwise it
initializes the external objects.
Shouldn't that be sufficient?
I created a patch (ignoring your patch so far) and it seem to work fine:
* dcrt0.cc (dll_crt0_1): Call well known SID initializer function.
* sec_helper.cc: Don't use constructor for well known SIDs.
(cygsid::init): New static method initializing well known SIDs.
* security.cc (class cygsid): Add static members for initializing
well known SIDs.
Index: dcrt0.cc
===
RCS file: /cvs/src/src/winsup/cygwin/dcrt0.cc,v
retrieving revision 1.165
diff -u -p -r1.165 dcrt0.cc
--- dcrt0.cc29 Nov 2002 07:05:25 - 1.165
+++ dcrt0.cc13 Dec 2002 11:48:05 -
@@ -549,6 +549,9 @@ dll_crt0_1 ()
/* Initialize SIGSEGV handling, etc. */
init_exceptions (cygwin_except_entry);
+ /* Init global well known SID objects */
+ cygsid::init ();
+
/* Set the os_being_run global. */
wincap.init ();
check_sanity_and_sync (user_data);
Index: sec_helper.cc
===
RCS file: /cvs/src/src/winsup/cygwin/sec_helper.cc,v
retrieving revision 1.30
diff -u -p -r1.30 sec_helper.cc
--- sec_helper.cc 10 Dec 2002 12:43:49 - 1.30
+++ sec_helper.cc 13 Dec 2002 11:48:05 -
@@ -48,18 +48,40 @@ SID_IDENTIFIER_AUTHORITY sid_auth[] = {
{SECURITY_NT_AUTHORITY}
};
-cygsid well_known_null_sid (S-1-0-0);
-cygsid well_known_world_sid (S-1-1-0);
-cygsid well_known_local_sid (S-1-2-0);
-cygsid well_known_creator_owner_sid (S-1-3-0);
-cygsid well_known_dialup_sid (S-1-5-1);
-cygsid well_known_network_sid (S-1-5-2);
-cygsid well_known_batch_sid (S-1-5-3);
-cygsid well_known_interactive_sid (S-1-5-4);
-cygsid well_known_service_sid (S-1-5-6);
-cygsid well_known_authenticated_users_sid (S-1-5-11);
-cygsid well_known_system_sid (S-1-5-18);
-cygsid well_known_admins_sid (S-1-5-32-544);
+cygsid well_known_null_sid;
+cygsid well_known_world_sid;
+cygsid well_known_local_sid;
+cygsid well_known_creator_owner_sid;
+cygsid well_known_dialup_sid;
+cygsid well_known_network_sid;
+cygsid well_known_batch_sid;
+cygsid well_known_interactive_sid;
+cygsid well_known_service_sid;
+cygsid well_known_authenticated_users_sid;
+cygsid well_known_system_sid;
+cygsid well_known_admins_sid;
+
+int cygsid::initialized = 0;
+
+void
+cygsid::init ()
+{
+ if (initialized)
+return;
+ well_known_null_sid = S-1-0-0;
+ well_known_world_sid = S-1-1-0;
+ well_known_local_sid = S-1-2-0;
+ well_known_creator_owner_sid = S-1-3-0;
+ well_known_dialup_sid = S-1-5-1;
+ well_known_network_sid = S-1-5-2;
+ well_known_batch_sid = S-1-5-3;
+ well_known_interactive_sid = S-1-5-4;
+ well_known_service_sid = S-1-5-6;
+ well_known_authenticated_users_sid = S-1-5-11;
+ well_known_system_sid = S-1-5-18;
+ well_known_admins_sid = S-1-5-32-544;
+ initialized = 1;
+}
char *
cygsid::string (char *nsidstr) const
Index: security.h
===
RCS file: /cvs/src/src/winsup/cygwin/security.h,v
retrieving revision 1.35
diff -u -p -r1.35 security.h
--- security.h 10 Dec 2002 12:43:49 - 1.35
+++ security.h 13 Dec 2002 11:48:05 -
@@ -23,6 +23,7 @@ details. */
class cygsid {
PSID psid;
char sbuf[MAX_SID_LEN];
+ static int initialized;
const PSID getfromstr (const char *nsidstr);
PSID get_sid (DWORD s, DWORD cnt, DWORD *r);
@@ -40,6 +41,7 @@ class cygsid {
}
public:
+ static void init();
inline operator const PSID () { return psid; }
inline const PSID operator= (cygsid nsid)
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Developermailto:[EMAIL PROTECTED]
Red Hat, Inc.
Re: Small security patches
Corinna Vinschen wrote: On Thu, Dec 12, 2002 at 11:34:02AM -0500, Pierre A. Humblet wrote: Christopher Faylor wrote: Actually, if you can get away without using a constructor that would be best. Constructors are a noticeable part of cygwin's startup cost. What about this idea: Add a static method init() called from . Init() checks if it has been called already before and returns immendiately if so. Otherwise it initializes the external objects. Shouldn't that be sufficient? That looks great. Can that be generalized? There must be other modules inside Cygwin that also need to initialize constant structures. Could all of these initializers be called from some central place, if needed, rather than having everybody maintain a separate isinitialized variable and add lines in the middle of dll_crt0_1 ()? In fact we may not even need a central isinitialized variable if the centralized routine is skipped only in case of forks. Pierre
Re: Small security patches
Corinna,
Until the initialization issue is settled, here is a patch covering
only the internationalization of security.cc
It should go in the next cygwin, and I always prefer when there
is a sufficiently long bake time.
Pierre
2002/12/13 Pierre Humblet [EMAIL PROTECTED]
* security.cc (get_user_local_groups): Use LookupAccountSid to find the
local equivalent of BUILTIN.
Index: security.cc
===
RCS file: /cvs/src/src/winsup/cygwin/security.cc,v
retrieving revision 1.128
diff -u -p -r1.128 security.cc
--- security.cc 10 Dec 2002 12:43:49 - 1.128
+++ security.cc 13 Dec 2002 02:35:32 -
@@ -389,16 +389,19 @@ get_user_local_groups (cygsidlist grp_l
return FALSE;
}
- char bgroup[sizeof (BUILTIN\\) + GNLEN] = BUILTIN\\;
+ char bgroup[INTERNET_MAX_HOST_NAME_LENGTH + GNLEN + 2];
char lgroup[INTERNET_MAX_HOST_NAME_LENGTH + GNLEN + 2];
- const DWORD blen = sizeof (BUILTIN\\) - 1;
- DWORD llen = INTERNET_MAX_HOST_NAME_LENGTH + 1;
- if (!GetComputerNameA (lgroup, llen))
+ DWORD blen, llen;
+ SID_NAME_USE use;
+
+ blen = llen = INTERNET_MAX_HOST_NAME_LENGTH + 1;
+ if (!LookupAccountSid (NULL, well_known_admins_sid, lgroup, llen, bgroup, blen,
+use)
+ || !GetComputerNameA (lgroup, (llen = INTERNET_MAX_HOST_NAME_LENGTH + 1)))
{
__seterrno ();
return FALSE;
}
- lgroup[llen++] = '\\';
+ bgroup[blen++] = lgroup[llen++] = '\\';
for (DWORD i = 0; i cnt; ++i)
if (is_group_member (buf[i].lgrpi0_name, pusersid, grp_list))
@@ -407,8 +410,8 @@ get_user_local_groups (cygsidlist grp_l
DWORD glen = sizeof (gsid);
char domain[INTERNET_MAX_HOST_NAME_LENGTH + 1];
DWORD dlen = sizeof (domain);
- SID_NAME_USE use = SidTypeInvalid;
+ use = SidTypeInvalid;
sys_wcstombs (bgroup + blen, buf[i].lgrpi0_name, GNLEN + 1);
if (!LookupAccountName (NULL, bgroup, gsid, glen, domain, dlen, use))
{
Re: Small security patches
On Fri, Dec 13, 2002 at 09:51:58AM -0500, Pierre A. Humblet wrote: Corinna Vinschen wrote: On Thu, Dec 12, 2002 at 11:34:02AM -0500, Pierre A. Humblet wrote: Christopher Faylor wrote: Actually, if you can get away without using a constructor that would be best. Constructors are a noticeable part of cygwin's startup cost. What about this idea: Add a static method init() called from . Init() checks if it has been called already before and returns immendiately if so. Otherwise it initializes the external objects. Shouldn't that be sufficient? That looks great. Can that be generalized? There must be other modules inside Cygwin that also need to initialize constant structures. Could all of these initializers be called from some central place, if needed, rather than having everybody maintain a separate isinitialized variable and add lines in the middle of dll_crt0_1 ()? That's what dll_crt0_1 is for. You're going to have to call the functions from someplace and dll_crt0_1 is designed for that. To answer your question, yes, you can tell if you've been either forked or execed. Rather than maintain an initialized variable, you can always check that. cgf
Re: Small security patches
Christopher Faylor wrote: Actually, if you can get away without using a constructor that would be best. Constructors are a noticeable part of cygwin's startup cost. - Is there a C++ way to initialize a constant class and have it in the .text section, as const int i = 1; would be? - If not, I can get the desired effect by using gcc Asm Labels, like int foo asm (myfoo) = 2; Would that be acceptable in Cygwin? Pierre
Re: Small security patches
On Wed, Dec 11, 2002 at 11:36:17AM -0500, Pierre A. Humblet wrote: Corinna, here is an internationalization bug fix, and some preliminary definitions for a future well_known_creator approach. Pierre 2002/12/11 Pierre Humblet [EMAIL PROTECTED] * security.h: Declare well_known_creator_group_sid. * sec_helper.cc: Declare and initialize well_known_creator_group_sid. * security.cc (get_user_local_groups): Use LookupAccountSid to find the local equivalent of BUILTIN. Shouldn't the global symbols be marked as NO_COPY? cgf
Re: Small security patches
Christopher Faylor wrote: Shouldn't the global symbols be marked as NO_COPY? I am not sure why things are as they are. These symbols are initialized in do_global_ctors and never change. Are the constructors running again after a fork? If so, NO_COPY is fine. It would seem more efficient to copy than to rerun the constructors, but I probably overlook some factors. Pierre
Re: Small security patches
On Wed, Dec 11, 2002 at 03:56:17PM -0500, Pierre A. Humblet wrote: Christopher Faylor wrote: Shouldn't the global symbols be marked as NO_COPY? I am not sure why things are as they are. These symbols are initialized in do_global_ctors and never change. Are the constructors running again after a fork? If so, NO_COPY is fine. It would seem more efficient to copy than to rerun the constructors, but I probably overlook some factors. Constructors are always run. If you use a global constructor without a NO_COPY then you just end up writing over the contents when the fork completes. So, if the constructor is setting things up correctly the global should be NO_COPY. Actually, if you can get away without using a constructor that would be best. Constructors are a noticeable part of cygwin's startup cost. cgf
Re: Small security patches
Christopher Faylor wrote: On Wed, Dec 11, 2002 at 03:56:17PM -0500, Pierre A. Humblet wrote: Christopher Faylor wrote: Shouldn't the global symbols be marked as NO_COPY? I am not sure why things are as they are. These symbols are initialized in do_global_ctors and never change. Are the constructors running again after a fork? If so, NO_COPY is fine. It would seem more efficient to copy than to rerun the constructors, but I probably overlook some factors. Constructors are always run. If you use a global constructor without a NO_COPY then you just end up writing over the contents when the fork completes. So, if the constructor is setting things up correctly the global should be NO_COPY. Actually, if you can get away without using a constructor that would be best. Constructors are a noticeable part of cygwin's startup cost. Thanks for the information. While we are at it, I was looking at the code and noticed that there were hooks to avoid running the constructors (things such as force and user_data-run_ctors_p). Are they ever used or are they history? In this case we could write a macro to initialize the cygsid structure without having a constructor. I will look into it. Pierre
