Re: [PATCH] Implement ACL module architecture and sample MySQL ACL module
Shawn Pearce writes:
> But seeing this, yes, that is a bad idea. Better to treat that like a
> hook, where exit status 0 allows the connection to continue, and exit
> status non-zero causes the connection to be closed. Maybe with an
> error printed to stderr (if any) being echoed first to the client if
> possible using the ERR formatting notation.
Yeah, note that we can only give a single "ERR " line, though.
Something like this? Totally untested, of course ;-)
daemon.c | 79
1 file changed, 79 insertions(+)
diff --git a/daemon.c b/daemon.c
index ab21e66..41a9679 100644
--- a/daemon.c
+++ b/daemon.c
@@ -30,6 +30,7 @@ static const char daemon_usage[] =
" [--interpolated-path=]\n"
" [--reuseaddr] [--pid-file=]\n"
" [--(enable|disable|allow-override|forbid-override)=]\n"
+" [--access-hook=]\n"
" [--inetd | [--listen=] [--port=]\n"
" [--detach] [--user= [--group=]]\n"
" [...]";
@@ -256,6 +257,73 @@ static int daemon_error(const char *dir, const char *msg)
return -1;
}
+static char *access_hook;
+
+static int run_access_hook(struct daemon_service *service, const char *dir,
const char *path)
+{
+ struct child_process child;
+ struct strbuf buf = STRBUF_INIT;
+ const char *argv[8];
+ const char **arg = argv;
+ char *eol;
+ int seen_errors = 0;
+
+#define STRARG(x) ((x) ? (x) : "")
+ *arg++ = access_hook;
+ *arg++ = service->name;
+ *arg++ = path;
+ *arg++ = STRARG(hostname);
+ *arg++ = STRARG(canon_hostname);
+ *arg++ = STRARG(ip_address);
+ *arg++ = STRARG(tcp_port);
+ *arg = NULL;
+#undef STRARG
+
+ memset(&child, 0, sizeof(child));
+ child.use_shell = 1;
+ child.argv = argv;
+ child.no_stdin = 1;
+ child.no_stderr = 1;
+ child.out = -1;
+ if (start_command(&child)) {
+ logerror("daemon access hook '%s' failed to start",
+access_hook);
+ goto error_return;
+ }
+ if (strbuf_read(&buf, child.out, 0) < 0) {
+ logerror("failed to read from pipe to daemon access hook '%s'",
+access_hook);
+ strbuf_reset(&buf);
+ seen_errors = 1;
+ }
+ if (close(child.out) < 0) {
+ logerror("failed to close pipe to daemon access hook '%s'",
+access_hook);
+ seen_errors = 1;
+ }
+ if (finish_command(&child) < 0) {
+ logerror("failed to finish-command daemon access hook '%s'",
+access_hook);
+ seen_errors = 1;
+ }
+ if (!seen_errors) {
+ strbuf_release(&buf);
+ return 0;
+ }
+
+error_return:
+ strbuf_ltrim(&buf);
+ if (!buf.len)
+ strbuf_addstr(&buf, "service rejected");
+ eol = strchr(buf.buf, '\n');
+ if (eol)
+ *eol = '\0';
+ errno = EACCES;
+ daemon_error(dir, buf.buf);
+ strbuf_release(&buf);
+ return -1;
+}
+
static int run_service(char *dir, struct daemon_service *service)
{
const char *path;
@@ -304,6 +372,13 @@ static int run_service(char *dir, struct daemon_service
*service)
}
/*
+* Optionally, a hook can choose to deny access to the
+* repository depending on the phase of the moon.
+*/
+ if (access_hook && run_access_hook(service, dir, path))
+ return -1;
+
+ /*
* We'll ignore SIGTERM from now on, we have a
* good client.
*/
@@ -1142,6 +1217,10 @@ int main(int argc, char **argv)
export_all_trees = 1;
continue;
}
+ if (!prefixcmp(arg, "--access-hook=")) {
+ access_hook = arg + 14;
+ continue;
+ }
if (!prefixcmp(arg, "--timeout=")) {
timeout = atoi(arg+10);
continue;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Implement ACL module architecture and sample MySQL ACL module
On Tue, Aug 14, 2012 at 10:06 AM, Junio C Hamano wrote: > Shawn Pearce writes: > >> Parsing the request line of git-daemon is easy. But we could make it >> easier. An alternative arrangement would be to add a new command line >> flag to git daemon like --command-filter that names an executable >> git-daemon will invoke after parsing the request line. It can pass >> along the client IP address, command request, repository name, and >> resolved repository path, and tie stdin/stdout to the client. This >> binary can decide to exec the proper git binary for the named command, >> or just exit to disconnect the client and refuse service. This makes >> it simple for a tool like gitolite to plug into the git-daemon >> authorization path, without needing to be the network daemon itself, >> worry about number of active connection slots, etc. > > I think that is a good direction to go in, except that I am unsure > what kind of conversation do you want to allow between the "command > filter" helper and the client by exposing standard input and output > stream to to the helper. Sorry, I was thinking the helper would exec the git command, and thus pass along the stdin/stdout socket. > If the client side has a matching "pre > negotiate command" helper support, then presumably the helpers can > discuss what Git protocol proper does not care about before deciding > to allow the connection go through, but until that happens, opening > the stdio streams up to the helper sounds like an accident waiting > to happen to me (e.g. "fetch-pack" connects, the server side helper > reads the first pkt-line from the client, says "OK, you may proceed" > to the daemon, then the daemon spawns the "upload-pack", which will > obviously see a corrupt request stream from "fetch-pack"). But seeing this, yes, that is a bad idea. Better to treat that like a hook, where exit status 0 allows the connection to continue, and exit status non-zero causes the connection to be closed. Maybe with an error printed to stderr (if any) being echoed first to the client if possible using the ERR formatting notation. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Implement ACL module architecture and sample MySQL ACL module
Shawn Pearce writes: > Parsing the request line of git-daemon is easy. But we could make it > easier. An alternative arrangement would be to add a new command line > flag to git daemon like --command-filter that names an executable > git-daemon will invoke after parsing the request line. It can pass > along the client IP address, command request, repository name, and > resolved repository path, and tie stdin/stdout to the client. This > binary can decide to exec the proper git binary for the named command, > or just exit to disconnect the client and refuse service. This makes > it simple for a tool like gitolite to plug into the git-daemon > authorization path, without needing to be the network daemon itself, > worry about number of active connection slots, etc. I think that is a good direction to go in, except that I am unsure what kind of conversation do you want to allow between the "command filter" helper and the client by exposing standard input and output stream to to the helper. If the client side has a matching "pre negotiate command" helper support, then presumably the helpers can discuss what Git protocol proper does not care about before deciding to allow the connection go through, but until that happens, opening the stdio streams up to the helper sounds like an accident waiting to happen to me (e.g. "fetch-pack" connects, the server side helper reads the first pkt-line from the client, says "OK, you may proceed" to the daemon, then the daemon spawns the "upload-pack", which will obviously see a corrupt request stream from "fetch-pack"). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Implement ACL module architecture and sample MySQL ACL module
On Tue, Aug 14, 2012 at 9:12 AM, Junio C Hamano wrote: > Michal Novotny writes: > >> Hi, >> this is the patch to introduce the ACL module architecture into git >> versioning system. > > No, it doesn't. It adds something only to "git daemon", but does > not affect any other uses of Git. Yes, this part of the commit message also confused me until I read through the patch further. :-( > Side note: I am not saying other uses of Git must be ACL > controlled by MySQL database. They shouldn't be. I am only > saying that the proposed commit log message must match what the > change does. > > Please familiarize yourself with Documentation/SubmittingPatches > first, and then imitate the style in existing commits in the history > and posted patches by the "good" developers (you can tell who they > are by observing the list traffic for a few weeks), by the way. > > As "git daemon" already has a mechanism to specify what repositories > are served with whitelist or blacklist, I am not sure if this patch > adds enough value to the system to make us want to add further > complexity only to carry more code to be audited for security. > > Opinions? Traditionally Git has been about providing the plumbing to handle the protocol and storage, and other tools that wrap git manage access controls, e.g. UNIX filesystem or gitolite. I would strongly prefer to keep that arrangement. Parsing the request line of git-daemon is easy. But we could make it easier. An alternative arrangement would be to add a new command line flag to git daemon like --command-filter that names an executable git-daemon will invoke after parsing the request line. It can pass along the client IP address, command request, repository name, and resolved repository path, and tie stdin/stdout to the client. This binary can decide to exec the proper git binary for the named command, or just exit to disconnect the client and refuse service. This makes it simple for a tool like gitolite to plug into the git-daemon authorization path, without needing to be the network daemon itself, worry about number of active connection slots, etc. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Implement ACL module architecture and sample MySQL ACL module
Michal Novotny writes: > Hi, > this is the patch to introduce the ACL module architecture into git > versioning system. No, it doesn't. It adds something only to "git daemon", but does not affect any other uses of Git. Side note: I am not saying other uses of Git must be ACL controlled by MySQL database. They shouldn't be. I am only saying that the proposed commit log message must match what the change does. Please familiarize yourself with Documentation/SubmittingPatches first, and then imitate the style in existing commits in the history and posted patches by the "good" developers (you can tell who they are by observing the list traffic for a few weeks), by the way. As "git daemon" already has a mechanism to specify what repositories are served with whitelist or blacklist, I am not sure if this patch adds enough value to the system to make us want to add further complexity only to carry more code to be audited for security. Opinions? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Implement ACL module architecture and sample MySQL ACL module
Hi,
this is the patch to introduce the ACL module architecture into git
versioning system. The variable of GIT_BASE_DIR is being used to seek
for the modules if available. If variable is unset then daemon looks
for the /etc/git-daemon.conf file existence and reads the
'base_path' key if it exists to determine the root path for
the git-daemon. This will be referred to as to $GIT_BASE_DIR
directory and the ACL modules subdirectory should reside in path of
$GIT_BASE_DIR/modules/acl. For MySQL connection you have to
alter the modules/config/modgitacl_mysql.cfg file to be able
to connect to the MySQL server using your credentials.
For MySQL database connection 2 new tables will get created
on your server and in the desired database. One holds the
ACLs for IP addresses and repositories and the second is
optional logging to the history table. This is by default
disabled however it could be enabled by setting the "log_history"
column of the git_access_acl entry to 1.
The patch also introduces the global_access_rule key to the config
which defines the default access policy if address and repository
key combination doesn't exist in the database. This automatically
creates the default access rule for the repository (which is
basically an entry with addr field set to '%' as all possible
entries in this column) based on the key settings.
The search algorithm for the entries is as follows:
1) Try to find the default entry for the repository
- If default entry doesn't exist then create it based on the
global_access_rule key settings (if set and valid)
2) Try to find entry for repository and IP address to override
default settings
3) Log entries access if appropriate
If the new default access rule is being created then logging is
enabled for 'deny' global_access_rule and disabled for 'allow'
global_access_rule settings.
The MySQL module is just the working example of how to use the
ACL architecture so it's not being compiled by default and user
has to compile it manually. Basically this module is showing how
git ACL infrastructure works and it's good point to start writing
such modules.
The patch has been tested using following command line on one terminal:
$ make && (cd modules/acl && make && cd -) && GIT_BASE_DIR=`pwd` ./git-daemon
--export-all
and cloning the existing repository on the second terminal with testing
all possible options in the database (using the sample MySQL module)
and everything was working fine.
Thanks,
Michal
Signed-off-by: Michal Novotny
---
Makefile | 1 +
daemon.c | 123 +
modules/acl/Makefile | 6 ++
modules/acl/modgitacl_mysql.c | 211 +
modules/config/modgitacl_mysql.cfg | 12 +++
modules/modules.h | 9 ++
6 files changed, 362 insertions(+)
create mode 100644 modules/acl/Makefile
create mode 100644 modules/acl/modgitacl_mysql.c
create mode 100644 modules/config/modgitacl_mysql.cfg
create mode 100644 modules/modules.h
diff --git a/Makefile b/Makefile
index 6b0c961..6fe32a7 100644
--- a/Makefile
+++ b/Makefile
@@ -932,6 +932,7 @@ ifeq ($(uname_S),OSF1)
NO_NSEC = YesPlease
endif
ifeq ($(uname_S),Linux)
+ BASIC_CFLAGS += -ldl -rdynamic
NO_STRLCPY = YesPlease
NO_MKSTEMPS = YesPlease
HAVE_PATHS_H = YesPlease
diff --git a/daemon.c b/daemon.c
index ab21e66..8a06c05 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1,9 +1,14 @@
+#defineGIT_DAEMON_CONFIG_FILE "/etc/git-daemon.conf"
+#defineGIT_DAEMON_CONFIG_FILE_PATH_KEY "base_path"
+#defineBUFSIZE 1 << 12
+
#include "cache.h"
#include "pkt-line.h"
#include "exec_cmd.h"
#include "run-command.h"
#include "strbuf.h"
#include "string-list.h"
+#include "modules/modules.h"
#ifndef HOST_NAME_MAX
#define HOST_NAME_MAX 256
@@ -256,6 +261,117 @@ static int daemon_error(const char *dir, const char *msg)
return -1;
}
+static char* daemon_read_config(const char *filename, char *key)
+{
+ FILE *fp;
+ char line[BUFSIZE];
+
+ fp = fopen(filename, "r");
+ if (fp == NULL)
+ return NULL;
+
+ while (!feof(fp)) {
+ fgets(line, sizeof(line), fp);
+
+ if (strncmp(line, key, strlen(key)) == 0) {
+ return strdup( line + strlen(key) + 3 );
+ }
+ }
+ fclose(fp);
+
+ return NULL;
+}
+
+static int check_access_addrdir(char *libname, char *base_path, char *addr,
const char *dir)
+{
+ int ret = -EPERM;
+ void *lib = NULL;
+ void *pCheck = NULL;
+ typedef int (*tCheckFunc) (char *base_path, char *addr, const char
*dir);
+
+ lib = dlopen(libname, RTLD_LAZY);
+ if (lib == NULL) {
+ logerror("%s: Cannot load ACL library '%s'", __FUNCTION__,
libname);
+ goto cleanup;
+ }
+
+ pChe
