Author: defan
Date: 2012-02-13 16:29:04 +0000 (Mon, 13 Feb 2012)
New Revision: 4478

Log:
Added disable_symlinks directive.

To completely disable symlinks (disable_symlinks on)
we use openat(O_NOFOLLOW) for each path component
to avoid races.

To allow symlinks with the same owner (disable_symlinks if_not_owner),
use openat() (followed by fstat()) and fstatat(AT_SYMLINK_NOFOLLOW),
and then compare uids between fstat() and fstatat().

As there is a race between openat() and fstatat() we don't
know if openat() in fact opened symlink or not. Therefore,
we have to compare uids even if fstatat() reports the opened
component isn't a symlink (as we don't know whether it was
symlink during openat() or not).

Default value is off, i.e. symlinks are allowed.


Modified:
trunk/src/core/ngx_core.h
trunk/src/core/ngx_open_file_cache.c
trunk/src/core/ngx_open_file_cache.h
trunk/src/http/ngx_http_core_module.c
trunk/src/http/ngx_http_core_module.h

Modified: trunk/src/core/ngx_core.h
===================================================================
--- trunk/src/core/ngx_core.h 2012-02-13 16:16:45 UTC (rev 4477)
+++ trunk/src/core/ngx_core.h 2012-02-13 16:29:04 UTC (rev 4478)
@@ -91,5 +91,10 @@

void ngx_cpuinfo(void);

+#if (NGX_HAVE_OPENAT)
+#define NGX_DISABLE_SYMLINKS_OFF 0
+#define NGX_DISABLE_SYMLINKS_ON 1
+#define NGX_DISABLE_SYMLINKS_NOTOWNER 2
+#endif

#endif /* _NGX_CORE_H_INCLUDED_ */

Modified: trunk/src/core/ngx_open_file_cache.c
===================================================================
--- trunk/src/core/ngx_open_file_cache.c 2012-02-13 16:16:45 UTC (rev 4477)
+++ trunk/src/core/ngx_open_file_cache.c 2012-02-13 16:29:04 UTC (rev 4478)
@@ -22,6 +22,15 @@


static void ngx_open_file_cache_cleanup(void *data);
+#if (NGX_HAVE_OPENAT)
+static ngx_fd_t ngx_openat_file_owner(ngx_fd_t at_fd, const u_char *name,
+ ngx_int_t mode, ngx_int_t create, ngx_int_t access);
+#endif
+static ngx_fd_t ngx_open_file_wrapper(ngx_str_t *name,
+ ngx_open_file_info_t *of, ngx_int_t mode, ngx_int_t create,
+ ngx_int_t access);
+static ngx_int_t ngx_file_info_wrapper(ngx_str_t *name,
+ ngx_open_file_info_t *of, ngx_file_info_t *fi);
static ngx_int_t ngx_open_and_stat_file(ngx_str_t *name,
ngx_open_file_info_t *of, ngx_log_t *log);
static void ngx_open_file_add_event(ngx_open_file_cache_t *cache,
@@ -147,9 +156,7 @@

if (of->test_only) {

- if (ngx_file_info(name->data, &fi) == NGX_FILE_ERROR) {
- of->err = ngx_errno;
- of->failed = ngx_file_info_n;
+ if (ngx_file_info_wrapper(name, of, &fi) == NGX_FILE_ERROR) {
return NGX_ERROR;
}

@@ -217,7 +224,11 @@
if (file->use_event
|| (file->event == NULL
&& (of->uniq == 0 || of->uniq == file->uniq)
- && now - file->created < of->valid))
+ && now - file->created < of->valid
+#if (NGX_HAVE_OPENAT)
+ && of->disable_symlinks == file->disable_symlinks
+#endif
+ ))
{
if (file->err == 0) {

@@ -239,7 +250,12 @@

} else {
of->err = file->err;
+#if (NGX_HAVE_OPENAT)
+ of->failed = file->disable_symlinks ? ngx_openat_file_n
+ : ngx_open_file_n;
+#else
of->failed = ngx_open_file_n;
+#endif
}

goto found;
@@ -375,6 +391,9 @@

file->fd = of->fd;
file->err = of->err;
+#if (NGX_HAVE_OPENAT)
+ file->disable_symlinks = of->disable_symlinks;
+#endif

if (of->err == 0) {
file->uniq = of->uniq;
@@ -459,7 +478,213 @@
}


+#if (NGX_HAVE_OPENAT)
+
+static ngx_fd_t
+ngx_openat_file_owner(ngx_fd_t at_fd, const u_char *name,
+ ngx_int_t mode, ngx_int_t create, ngx_int_t access)
+{
+ ngx_fd_t fd;
+ ngx_file_info_t fi, atfi;
+
+ /*
+ * To allow symlinks with the same owner, use openat() (followed
+ * by fstat()) and fstatat(AT_SYMLINK_NOFOLLOW), and then compare
+ * uids between fstat() and fstatat().
+ *
+ * As there is a race between openat() and fstatat() we don't
+ * know if openat() in fact opened symlink or not. Therefore,
+ * we have to compare uids even if fstatat() reports the opened
+ * component isn't a symlink (as we don't know whether it was
+ * symlink during openat() or not).
+ */
+
+ fd = ngx_openat_file(at_fd, name, mode, create, access);
+
+ if (fd == NGX_FILE_ERROR) {
+ return NGX_FILE_ERROR;
+ }
+
+ if (ngx_file_at_info(at_fd, name, &atfi, AT_SYMLINK_NOFOLLOW)
+ == NGX_FILE_ERROR)
+ {
+ ngx_close_file(fd);
+ return NGX_FILE_ERROR;
+ }
+
+ if (ngx_fd_info(fd, &fi) == NGX_FILE_ERROR) {
+ ngx_close_file(fd);
+ return NGX_FILE_ERROR;
+ }
+
+ if (fi.st_uid != atfi.st_uid) {
+ ngx_close_file(fd);
+ ngx_set_errno(NGX_ELOOP);
+ return NGX_FILE_ERROR;
+ }
+
+ return fd;
+}
+
+#endif
+
+
+static ngx_fd_t
+ngx_open_file_wrapper(ngx_str_t *name, ngx_open_file_info_t *of,
+ ngx_int_t mode, ngx_int_t create, ngx_int_t access)
+{
+ ngx_fd_t fd;
+
+#if !(NGX_HAVE_OPENAT)
+
+ fd = ngx_open_file(name->data, mode, create, access);
+
+ if (fd == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_open_file_n;
+ return NGX_FILE_ERROR;
+ }
+
+ return fd;
+
+#else
+
+ u_char *p, *cp, *end;
+ ngx_fd_t at_fd;
+
+ if (of->disable_symlinks == NGX_DISABLE_SYMLINKS_OFF) {
+ fd = ngx_open_file(name->data, mode, create, access);
+
+ if (fd == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_open_file_n;
+ return NGX_FILE_ERROR;
+ }
+
+ return fd;
+ }
+
+ at_fd = ngx_openat_file(AT_FDCWD, "/", NGX_FILE_RDONLY|NGX_FILE_NONBLOCK,
+ NGX_FILE_OPEN, 0);
+
+ if (at_fd == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_openat_file_n;
+ return NGX_FILE_ERROR;
+ }
+
+ end = name->data + name->len;
+ p = name->data + 1;
+
+ for ( ;; ) {
+ cp = ngx_strlchr(p, end, '/');
+ if (cp == NULL) {
+ break;
+ }
+
+ *cp = '\0';
+
+ if (of->disable_symlinks == NGX_DISABLE_SYMLINKS_NOTOWNER) {
+ fd = ngx_openat_file_owner(at_fd, p,
+ NGX_FILE_RDONLY|NGX_FILE_NONBLOCK,
+ NGX_FILE_OPEN, 0);
+
+ } else {
+ fd = ngx_openat_file(at_fd, p,
+ NGX_FILE_RDONLY|NGX_FILE_NONBLOCK|NGX_FILE_NOFOLLOW,
+ NGX_FILE_OPEN, 0);
+ }
+
+ *cp = '/';
+
+ ngx_close_file(at_fd);
+
+ if (fd == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_openat_file_n;
+ return NGX_FILE_ERROR;
+ }
+
+ p = cp + 1;
+ at_fd = fd;
+ }
+
+ if (of->disable_symlinks == NGX_DISABLE_SYMLINKS_NOTOWNER) {
+ fd = ngx_openat_file_owner(at_fd, p, mode, create, access);
+
+ } else {
+ fd = ngx_openat_file(at_fd, p, mode|NGX_FILE_NOFOLLOW, create, access);
+ }
+
+ if (fd == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_openat_file_n;
+ }
+
+ ngx_close_file(at_fd);
+
+ return fd;
+#endif
+}
+
+
static ngx_int_t
+ngx_file_info_wrapper(ngx_str_t *name, ngx_open_file_info_t *of,
+ ngx_file_info_t *fi)
+{
+ ngx_int_t rc;
+
+#if !(NGX_HAVE_OPENAT)
+
+ rc = ngx_file_info(name->data, fi);
+
+ if (rc == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_file_info_n;
+ return NGX_FILE_ERROR;
+ }
+
+ return rc;
+
+#else
+
+ ngx_fd_t fd;
+
+ if (of->disable_symlinks == NGX_DISABLE_SYMLINKS_OFF) {
+
+ rc = ngx_file_info(name->data, fi);
+
+ if (rc == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_file_info_n;
+ return NGX_FILE_ERROR;
+ }
+
+ return rc;
+ }
+
+ fd = ngx_open_file_wrapper(name, of, NGX_FILE_RDONLY|NGX_FILE_NONBLOCK,
+ NGX_FILE_OPEN, 0);
+
+ if (fd == NGX_FILE_ERROR) {
+ return NGX_FILE_ERROR;
+ }
+
+ if (ngx_fd_info(fd, fi) == NGX_FILE_ERROR) {
+ of->err = ngx_errno;
+ of->failed = ngx_fd_info_n;
+ ngx_close_file(fd);
+ return NGX_FILE_ERROR;
+ }
+
+ ngx_close_file(fd);
+
+ return NGX_OK;
+#endif
+}
+
+
+static ngx_int_t
ngx_open_and_stat_file(ngx_str_t *name, ngx_open_file_info_t *of,
ngx_log_t *log)
{
@@ -468,9 +693,9 @@

if (of->fd != NGX_INVALID_FILE) {

- if (ngx_file_info(name->data, &fi) == NGX_FILE_ERROR) {
- of->failed = ngx_file_info_n;
- goto failed;
+ if (ngx_file_info_wrapper(name, of, &fi) == NGX_FILE_ERROR) {
+ of->fd = NGX_INVALID_FILE;
+ return NGX_ERROR;
}

if (of->uniq == ngx_file_uniq(&fi)) {
@@ -479,9 +704,9 @@

} else if (of->test_dir) {

- if (ngx_file_info(name->data, &fi) == NGX_FILE_ERROR) {
- of->failed = ngx_file_info_n;
- goto failed;
+ if (ngx_file_info_wrapper(name, of, &fi) == NGX_FILE_ERROR) {
+ of->fd = NGX_INVALID_FILE;
+ return NGX_ERROR;
}

if (ngx_is_dir(&fi)) {
@@ -496,18 +721,18 @@
* This flag has no effect on a regular files.
*/

- fd = ngx_open_file(name->data, NGX_FILE_RDONLY|NGX_FILE_NONBLOCK,
- NGX_FILE_OPEN, 0);
+ fd = ngx_open_file_wrapper(name, of, NGX_FILE_RDONLY|NGX_FILE_NONBLOCK,
+ NGX_FILE_OPEN, 0);

} else {
- fd = ngx_open_file(name->data, NGX_FILE_APPEND,
- NGX_FILE_CREATE_OR_OPEN,
- NGX_FILE_DEFAULT_ACCESS);
+ fd = ngx_open_file_wrapper(name, of, NGX_FILE_APPEND,
+ NGX_FILE_CREATE_OR_OPEN,
+ NGX_FILE_DEFAULT_ACCESS);
}

if (fd == NGX_INVALID_FILE) {
- of->failed = ngx_open_file_n;
- goto failed;
+ of->fd = NGX_INVALID_FILE;
+ return NGX_ERROR;
}

if (ngx_fd_info(fd, &fi) == NGX_FILE_ERROR) {
@@ -565,13 +790,6 @@
of->is_exec = ngx_is_exec(&fi);

return NGX_OK;
-
-failed:
-
- of->fd = NGX_INVALID_FILE;
- of->err = ngx_errno;
-
- return NGX_ERROR;
}



Modified: trunk/src/core/ngx_open_file_cache.h
===================================================================
--- trunk/src/core/ngx_open_file_cache.h 2012-02-13 16:16:45 UTC (rev 4477)
+++ trunk/src/core/ngx_open_file_cache.h 2012-02-13 16:29:04 UTC (rev 4478)
@@ -32,6 +32,10 @@

ngx_uint_t min_uses;

+#if (NGX_HAVE_OPENAT)
+ unsigned disable_symlinks:2;
+#endif
+
unsigned test_dir:1;
unsigned test_only:1;
unsigned log:1;
@@ -64,6 +68,10 @@

uint32_t uses;

+#if (NGX_HAVE_OPENAT)
+ unsigned disable_symlinks:2;
+#endif
+
unsigned count:24;
unsigned close:1;
unsigned use_event:1;

Modified: trunk/src/http/ngx_http_core_module.c
===================================================================
--- trunk/src/http/ngx_http_core_module.c 2012-02-13 16:16:45 UTC (rev 4477)
+++ trunk/src/http/ngx_http_core_module.c 2012-02-13 16:29:04 UTC (rev 4478)
@@ -187,6 +187,18 @@
#endif


+#if (NGX_HAVE_OPENAT)
+
+static ngx_conf_enum_t ngx_http_core_disable_symlinks[] = {
+ { ngx_string("off"), NGX_DISABLE_SYMLINKS_OFF },
+ { ngx_string("if_not_owner"), NGX_DISABLE_SYMLINKS_NOTOWNER },
+ { ngx_string("on"), NGX_DISABLE_SYMLINKS_ON },
+ { ngx_null_string, 0 }
+};
+
+#endif
+
+
static ngx_command_t ngx_http_core_commands[] = {

{ ngx_string("variables_hash_max_size"),
@@ -764,6 +776,17 @@

#endif

+#if (NGX_HAVE_OPENAT)
+
+ { ngx_string("disable_symlinks"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
+ ngx_conf_set_enum_slot,
+ NGX_HTTP_LOC_CONF_OFFSET,
+ offsetof(ngx_http_core_loc_conf_t, disable_symlinks),
+ &ngx_http_core_disable_symlinks },
+
+#endif
+
ngx_null_command
};

@@ -1297,6 +1320,9 @@
of.test_only = 1;
of.errors = clcf->open_file_cache_errors;
of.events = clcf->open_file_cache_events;
+#if (NGX_HAVE_OPENAT)
+ of.disable_symlinks = clcf->disable_symlinks;
+#endif

if (ngx_open_cached_file(clcf->open_file_cache, &path, &of, r->pool)
!= NGX_OK)
@@ -3344,6 +3370,10 @@
#endif
#endif

+#if (NGX_HAVE_OPENAT)
+ clcf->disable_symlinks = NGX_CONF_UNSET_UINT;
+#endif
+
return clcf;
}

@@ -3623,6 +3653,11 @@
#endif
#endif

+#if (NGX_HAVE_OPENAT)
+ ngx_conf_merge_uint_value(conf->disable_symlinks, prev->disable_symlinks,
+ NGX_DISABLE_SYMLINKS_OFF);
+#endif
+
return NGX_CONF_OK;
}


Modified: trunk/src/http/ngx_http_core_module.h
===================================================================
--- trunk/src/http/ngx_http_core_module.h 2012-02-13 16:16:45 UTC (rev 4477)
+++ trunk/src/http/ngx_http_core_module.h 2012-02-13 16:29:04 UTC (rev 4478)
@@ -404,6 +404,10 @@
#endif
#endif

+#if (NGX_HAVE_OPENAT)
+ ngx_uint_t disable_symlinks; /* disable_symlinks */
+#endif
+
ngx_array_t *error_pages; /* error_page */
ngx_http_try_file_t *try_files; /* try_files */


_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel