update: secure include
CAE Samba Admin
caesmb at lab2.cc.wmich.edu
Fri Jun 11 20:04:35 GMT 1999
Hello,
I've added quite a bit more security to "secure include" as per
suggestions of a few people here (thanks).
This still isn't dynamic, sorry... But it does function as a nice
absolute security (ie, root and only root).
I've implemented verification of inodes, and I now perform the uid and
write permission checks across the whole path up to the include file.
I've included a diff against 2.0.4b below.
I am still interested in seeing something like this integrated into the
source trees, I'm also interested in cleaning this up for my own purposes.
Any suggestions anyone could offer are welcome.
I have a couple concerns as of now:
~ readlink() is called, but i don't think this function is POSIX.
does anyone know of a POSIX way to do this?
~ root is assumed to be uid==0.
is this true for all operating systems?
~ '/' is used in hacking apart the path
is there a way to determine the directory seperator
character for the OS?
Again, any help making this conform to samba source standards will be
appreciated.
Thanks,
Kevin Currie
-------------- next part --------------
diff -uNr samba-2.0.4b/source/param/loadparm.c samba-2.0.4b-diff/source/param/loadparm.c
--- samba-2.0.4b/source/param/loadparm.c Mon May 17 19:37:24 1999
+++ samba-2.0.4b-diff/source/param/loadparm.c Fri Jun 11 15:50:50 1999
@@ -261,6 +261,7 @@
char *szAdminUsers;
char *szCopy;
char *szInclude;
+ char *szSecureInclude;
char *szPreExec;
char *szPostExec;
char *szRootPreExec;
@@ -356,6 +357,7 @@
NULL, /* szAdminUsers */
NULL, /* szCopy */
NULL, /* szInclude */
+ NULL, /* szSecureInclude */
NULL, /* szPreExec */
NULL, /* szPostExec */
NULL, /* szRootPreExec */
@@ -452,6 +454,7 @@
/* prototypes for the special type handlers */
static BOOL handle_valid_chars(char *pszParmValue, char **ptr);
static BOOL handle_include(char *pszParmValue, char **ptr);
+static BOOL handle_secure_include(char *pszParmValue, char **ptr);
static BOOL handle_copy(char *pszParmValue, char **ptr);
static BOOL handle_character_set(char *pszParmValue,char **ptr);
static BOOL handle_coding_system(char *pszParmValue,char **ptr);
@@ -784,6 +787,7 @@
{"-valid", P_BOOL, P_LOCAL, &sDefault.valid, NULL, NULL, FLAG_HIDE},
{"copy", P_STRING, P_LOCAL, &sDefault.szCopy, handle_copy, NULL, FLAG_HIDE},
{"include", P_STRING, P_LOCAL, &sDefault.szInclude, handle_include, NULL, FLAG_HIDE},
+ {"secure include", P_STRING, P_LOCAL, &sDefault.szSecureInclude, handle_secure_include, NULL, FLAG_HIDE},
{"exec", P_STRING, P_LOCAL, &sDefault.szPreExec, NULL, NULL, FLAG_SHARE|FLAG_PRINT},
{"preexec", P_STRING, P_LOCAL, &sDefault.szPreExec, NULL, NULL, 0},
{"postexec", P_STRING, P_LOCAL, &sDefault.szPostExec, NULL, NULL, FLAG_SHARE|FLAG_PRINT},
@@ -1889,6 +1893,131 @@
}
+/***************************************************************************
+handle the secure include operation
+***************************************************************************/
+static BOOL handle_secure_include(char *pszParmValue,char **ptr)
+{
+ SMB_STRUCT_STAT init_stat, link_stat, test_stat;
+ int filedes;
+
+ pstring fname, path, front;
+ pstrcpy(fname,pszParmValue);
+
+ add_to_file_list(fname);
+
+ standard_sub_basic(fname);
+
+ string_set(ptr,fname);
+
+ if (!(file_exist(fname,NULL))) {
+ DEBUG(2,("Can't find include file %s\n",fname));
+ return(False);
+ }
+
+ // preserve fname since split_to_last_component is destructive
+
+ pstrcpy(front, fname);
+
+ // check security of full path of secure include file
+
+ do {
+
+ // Get file stat info on filename, IOW: get the inode ASAP
+
+ if (sys_stat(front, &init_stat)) {
+ DEBUG(2,("ERROR: sys_stat failed on secure include file/path %s\n",front));
+ return(False);
+ }
+
+ // Get link stat info on filename
+
+ if (sys_lstat(front, &init_stat)) {
+ DEBUG(2,("ERROR: sys_lstat failed on secure include file/path %s\n",front));
+ return(False);
+ }
+
+ // If filename is a symlink, get the name of the file it points to
+
+ if (S_ISLNK(link_stat.st_mode)) {
+ if (readlink(front, front, sizeof(pstring)) < 0) {
+ DEBUG(2,("ERROR: readlink failed on secure include file/path %s\n",front));
+ return(False);
+ }
+ }
+
+ // Make user the link isn't broken
+ // We don't use file_exist() here because we also process directories
+
+ if (sys_stat(front, &link_stat)) {
+ DEBUG(2,("ERROR: secure include file/path %s is a broken link\n",front));
+ return(False);
+ }
+
+ // Open the file while doing security checks so that softlinks
+ // cannot be redirected on a bogged down system before the
+ // checks can occur
+
+ filedes = sys_open(front, O_RDONLY, S_IRWXU);
+ if (filedes < 0) {
+ DEBUG(2,("ERROR: sys_open failed on secure include file/path %s\n",front));
+ return(False);
+ }
+
+ // Get stat info on the open file
+
+ if (sys_fstat(filedes, &test_stat)) {
+ DEBUG(2,("ERROR: sys_fstat failed on secure include file/path %s\n",front));
+ return(False);
+ }
+
+ // Now that we have the info, close the file
+
+ if (close(filedes)) {
+ DEBUG(2,("ERROR: close failed on secure include file/path %s\n",front));
+ return(False);
+ }
+
+ // Ensure we're at the same inode we started with
+
+ if (init_stat.st_ino != test_stat.st_ino) {
+ DEBUG(2,("ERROR: secure include file/path %s inode changed during security checks\n",front));
+ return(False);
+ }
+
+ // Ensure the file's uid == root
+
+ if (test_stat.st_uid) {
+ DEBUG(2,("ERROR: secure include file/path %s uid not root\n",front));
+ return(False);
+ }
+
+ // Make sure the file isn't group writable
+
+ if (test_stat.st_mode & S_IWGRP) {
+ DEBUG(2,("ERROR: secure include file/path %s has group write bit set\n",front));
+ return(False);
+ }
+
+ // Make sure the file isn't world writable
+
+ if (test_stat.st_mode & S_IWOTH) {
+ DEBUG(2,("ERROR: secure include file/path %s has world write bit set\n",front));
+ return(False);
+ }
+
+ pstrcpy(path, front);
+ split_at_last_component(path, front, '/', NULL);
+
+ } while (*front != '\0');
+
+ // Okay, we made it. Go ahead and process the file
+
+ return(pm_process(fname, do_section, do_parameter));
+
+}
+
+
/***************************************************************************
handle the interpretation of the copy parameter
***************************************************************************/
More information about the samba-technical
mailing list