[Git][debian-proftpd-team/proftpd][master] Patch for issue Issue #1830 (Closes: #1082326).
Hilmar Preuße (@hilmar)
gitlab at salsa.debian.org
Thu Nov 14 11:28:29 GMT 2024
Hilmar Preuße pushed to branch master at Debian ProFTPD Team / proftpd
Commits:
62a91062 by Hilmar Preuße at 2024-11-14T12:28:14+01:00
Patch for issue Issue #1830 (Closes: #1082326).
- - - - -
3 changed files:
- debian/changelog
- + debian/patches/5031d498a71c493b9659e2b5ccafde58b0897e30.diff
- debian/patches/series
Changes:
=====================================
debian/changelog
=====================================
@@ -1,3 +1,10 @@
+proftpd-dfsg (1.3.8.b+dfsg-4) UNRELEASED; urgency=high
+
+ * Patch for issue Issue #1830 (Closes: #1082326).
+ Supplemental Group Inheritance Grants Unintended Access to GID 0
+
+ -- Hilmar Preuße <hille42 at debian.org> Thu, 14 Nov 2024 11:28:01 +0100
+
proftpd-dfsg (1.3.8.b+dfsg-3) unstable; urgency=medium
* Patches:
=====================================
debian/patches/5031d498a71c493b9659e2b5ccafde58b0897e30.diff
=====================================
@@ -0,0 +1,337 @@
+From 5031d498a71c493b9659e2b5ccafde58b0897e30 Mon Sep 17 00:00:00 2001
+From: TJ Saunders <tj at castaglia.org>
+Date: Wed, 13 Nov 2024 06:33:35 -0800
+Subject: [PATCH] Issue #1830: When no supplemental groups are provided by the
+ underlying authentication providers, fall back to using the primary
+ group/GID. (#1835)
+
+This prevents surprise due to inheritance of the parent processes' supplemental group membership, which might inadvertently provided undesired access.
+---
+ NEWS | 2 +
+ contrib/mod_sftp/auth.c | 14 +-
+ modules/mod_auth.c | 21 ++-
+ src/auth.c | 16 +-
+ .../ProFTPD/Tests/Modules/mod_sql_sqlite.pm | 174 ++++++++++++++++++
+ 5 files changed, 213 insertions(+), 14 deletions(-)
+
+diff --git a/NEWS b/NEWS
+index 2cd427e8d..495d74853 100644
+diff --git a/contrib/mod_sftp/auth.c b/contrib/mod_sftp/auth.c
+index c7a694e04..6196fec4a 100644
+--- a/contrib/mod_sftp/auth.c
++++ b/contrib/mod_sftp/auth.c
+@@ -388,8 +388,20 @@ static int setup_env(pool *p, const char *user) {
+ session.groups == NULL) {
+ res = pr_auth_getgroups(p, pw->pw_name, &session.gids, &session.groups);
+ if (res < 1) {
++ /* If no supplemental groups are provided, default to using the process
++ * primary GID as the supplemental group. This prevents access
++ * regressions as seen in Issue #1830.
++ */
+ (void) pr_log_writefile(sftp_logfd, MOD_SFTP_VERSION,
+- "no supplemental groups found for user '%s'", pw->pw_name);
++ "no supplemental groups found for user '%s', "
++ "using primary group %s (GID %lu)", pw->pw_name, session.group,
++ (unsigned long) session.login_gid);
++
++ session.gids = make_array(p, 2, sizeof(gid_t));
++ session.groups = make_array(p, 2, sizeof(char *));
++
++ *((gid_t *) push_array(session.gids)) = session.login_gid;
++ *((char **) push_array(session.groups)) = pstrdup(p, session.group);
+ }
+ }
+
+diff --git a/modules/mod_auth.c b/modules/mod_auth.c
+index a85be0675..4daa927d7 100644
+--- a/modules/mod_auth.c
++++ b/modules/mod_auth.c
+@@ -2,7 +2,7 @@
+ * ProFTPD - FTP server daemon
+ * Copyright (c) 1997, 1998 Public Flood Software
+ * Copyright (c) 1999, 2000 MacGyver aka Habeeb J. Dihu <macgyver at tos.net>
+- * Copyright (c) 2001-2022 The ProFTPD Project team
++ * Copyright (c) 2001-2024 The ProFTPD Project team
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+@@ -1113,8 +1113,8 @@ static int setup_env(pool *p, cmd_rec *cmd, const char *user, char *pass) {
+ session.groups = NULL;
+ }
+
+- if (!session.gids &&
+- !session.groups) {
++ if (session.gids == NULL &&
++ session.groups == NULL) {
+ /* Get the supplemental groups. Note that we only look up the
+ * supplemental group credentials if we have not cached the group
+ * credentials before, in session.gids and session.groups.
+@@ -1124,8 +1124,19 @@ static int setup_env(pool *p, cmd_rec *cmd, const char *user, char *pass) {
+ */
+ res = pr_auth_getgroups(p, pw->pw_name, &session.gids, &session.groups);
+ if (res < 1) {
+- pr_log_debug(DEBUG5, "no supplemental groups found for user '%s'",
+- pw->pw_name);
++ /* If no supplemental groups are provided, default to using the process
++ * primary GID as the supplemental group. This prevents access
++ * regressions as seen in Issue #1830.
++ */
++ pr_log_debug(DEBUG5, "no supplemental groups found for user '%s', "
++ "using primary group %s (GID %lu)", pw->pw_name, session.group,
++ (unsigned long) session.login_gid);
++
++ session.gids = make_array(p, 2, sizeof(gid_t));
++ session.groups = make_array(p, 2, sizeof(char *));
++
++ *((gid_t *) push_array(session.gids)) = session.login_gid;
++ *((char **) push_array(session.groups)) = pstrdup(p, session.group);
+ }
+ }
+
+diff --git a/src/auth.c b/src/auth.c
+index b90fe4162..378f99744 100644
+--- a/src/auth.c
++++ b/src/auth.c
+@@ -2,7 +2,7 @@
+ * ProFTPD - FTP server daemon
+ * Copyright (c) 1997, 1998 Public Flood Software
+ * Copyright (c) 1999, 2000 MacGyver aka Habeeb J. Dihu <macgyver at tos.net>
+- * Copyright (c) 2001-2022 The ProFTPD Project team
++ * Copyright (c) 2001-2024 The ProFTPD Project team
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+@@ -1471,12 +1471,12 @@ int pr_auth_getgroups(pool *p, const char *name, array_header **group_ids,
+ }
+
+ /* Allocate memory for the array_headers of GIDs and group names. */
+- if (group_ids) {
+- *group_ids = make_array(permanent_pool, 2, sizeof(gid_t));
++ if (group_ids != NULL) {
++ *group_ids = make_array(p, 2, sizeof(gid_t));
+ }
+
+- if (group_names) {
+- *group_names = make_array(permanent_pool, 2, sizeof(char *));
++ if (group_names != NULL) {
++ *group_names = make_array(p, 2, sizeof(char *));
+ }
+
+ cmd = make_cmd(p, 3, name, group_ids ? *group_ids : NULL,
+@@ -1495,7 +1495,7 @@ int pr_auth_getgroups(pool *p, const char *name, array_header **group_ids,
+ * for the benefit of auth_getgroup() implementors.
+ */
+
+- if (group_ids) {
++ if (group_ids != NULL) {
+ register unsigned int i;
+ char *strgids = "";
+ gid_t *gids = (*group_ids)->elts;
+@@ -1511,7 +1511,7 @@ int pr_auth_getgroups(pool *p, const char *name, array_header **group_ids,
+ *strgids ? strgids : "(None; corrupted group file?)");
+ }
+
+- if (group_names) {
++ if (group_names != NULL) {
+ register unsigned int i;
+ char *strgroups = "";
+ char **groups = (*group_names)->elts;
+@@ -1527,7 +1527,7 @@ int pr_auth_getgroups(pool *p, const char *name, array_header **group_ids,
+ }
+ }
+
+- if (cmd->tmp_pool) {
++ if (cmd->tmp_pool != NULL) {
+ destroy_pool(cmd->tmp_pool);
+ cmd->tmp_pool = NULL;
+ }
+diff --git a/tests/t/lib/ProFTPD/Tests/Modules/mod_sql_sqlite.pm b/tests/t/lib/ProFTPD/Tests/Modules/mod_sql_sqlite.pm
+index 08c1542aa..42ba9678b 100644
+--- a/tests/t/lib/ProFTPD/Tests/Modules/mod_sql_sqlite.pm
++++ b/tests/t/lib/ProFTPD/Tests/Modules/mod_sql_sqlite.pm
+@@ -467,6 +467,11 @@ my $TESTS = {
+ order => ++$order,
+ test_class => [qw(forking bug mod_tls)],
+ },
++
++ sql_user_info_no_suppl_groups_issue1830 => {
++ order => ++$order,
++ test_class => [qw(forking bug rootprivs)],
++ },
+ };
+
+ sub new {
+@@ -15764,4 +15769,173 @@ EOC
+ test_cleanup($setup->{log_file}, $ex);
+ }
+
++sub sql_user_info_no_suppl_groups_issue1830 {
++ my $self = shift;
++ my $tmpdir = $self->{tmpdir};
++ my $setup = test_setup($tmpdir, 'sqlite');
++
++ my $db_file = File::Spec->rel2abs("$tmpdir/proftpd.db");
++
++ # Build up sqlite3 command to create users, groups tables and populate them
++ my $db_script = File::Spec->rel2abs("$tmpdir/proftpd.sql");
++
++ if (open(my $fh, "> $db_script")) {
++ print $fh <<EOS;
++CREATE TABLE users (
++ userid TEXT,
++ passwd TEXT,
++ uid INTEGER,
++ gid INTEGER,
++ homedir TEXT,
++ shell TEXT
++);
++INSERT INTO users (userid, passwd, uid, gid, homedir, shell) VALUES ('$setup->{user}', '$setup->{passwd}', $setup->{uid}, $setup->{gid}, '$setup->{home_dir}', '/bin/bash');
++
++CREATE TABLE groups (
++ groupname TEXT,
++ gid INTEGER,
++ members TEXT
++);
++INSERT INTO groups (groupname, gid, members) VALUES ('$setup->{group}', $setup->{gid}, '$setup->{user}');
++EOS
++
++ unless (close($fh)) {
++ die("Can't write $db_script: $!");
++ }
++
++ } else {
++ die("Can't open $db_script: $!");
++ }
++
++ my $cmd = "sqlite3 $db_file < $db_script";
++ build_db($cmd, $db_script);
++
++ # Make sure that, if we're running as root, the database file has
++ # the permissions/privs set for use by proftpd
++ if ($< == 0) {
++ unless (chmod(0666, $db_file)) {
++ die("Can't set perms on $db_file to 0666: $!");
++ }
++ }
++
++ my $config = {
++ PidFile => $setup->{pid_file},
++ ScoreboardFile => $setup->{scoreboard_file},
++ SystemLog => $setup->{log_file},
++ TraceLog => $setup->{log_file},
++ Trace => 'auth:20 sql:20',
++
++ # Required for logging the expected message
++ DebugLevel => 5,
++
++ IfModules => {
++ 'mod_delay.c' => {
++ DelayEngine => 'off',
++ },
++
++ 'mod_sql.c' => {
++ AuthOrder => 'mod_sql.c',
++
++ SQLAuthenticate => 'users',
++ SQLAuthTypes => 'plaintext',
++ SQLBackend => 'sqlite3',
++ SQLConnectInfo => $db_file,
++ SQLLogFile => $setup->{log_file},
++
++ # Set these, so that our lower UID/GID will be used
++ SQLMinUserUID => 100,
++ SQLMinUserGID => 100,
++ },
++ },
++ };
++
++ my ($port, $config_user, $config_group) = config_write($setup->{config_file},
++ $config);
++
++ # Open pipes, for use between the parent and child processes. Specifically,
++ # the child will indicate when it's done with its test by writing a message
++ # to the parent.
++ my ($rfh, $wfh);
++ unless (pipe($rfh, $wfh)) {
++ die("Can't open pipe: $!");
++ }
++
++ my $ex;
++
++ # Fork child
++ $self->handle_sigchld();
++ defined(my $pid = fork()) or die("Can't fork: $!");
++ if ($pid) {
++ eval {
++ sleep(2);
++
++ my $client = ProFTPD::TestSuite::FTP->new('127.0.0.1', $port);
++ $client->login($setup->{user}, $setup->{passwd});
++
++ my $resp_msgs = $client->response_msgs();
++ my $nmsgs = scalar(@$resp_msgs);
++
++ my $expected = 1;
++ $self->assert($expected == $nmsgs,
++ test_msg("Expected $expected, got $nmsgs"));
++
++ $expected = "User $setup->{user} logged in";
++ $self->assert($expected eq $resp_msgs->[0],
++ test_msg("Expected response '$expected', got '$resp_msgs->[0]'"));
++
++ $client->quit();
++ };
++ if ($@) {
++ $ex = $@;
++ }
++
++ $wfh->print("done\n");
++ $wfh->flush();
++
++ } else {
++ eval { server_wait($setup->{config_file}, $rfh) };
++ if ($@) {
++ warn($@);
++ exit 1;
++ }
++
++ exit 0;
++ }
++
++ # Stop server
++ server_stop($setup->{pid_file});
++ $self->assert_child_ok($pid);
++
++ eval {
++ if (open(my $fh, "< $setup->{log_file}")) {
++ my $ok = 0;
++
++ while (my $line = <$fh>) {
++ chomp($line);
++
++ if ($ENV{TEST_VERBOSE}) {
++ print STDERR "# $line\n";
++ }
++
++ if ($line =~ /no supplemental groups found for user '$setup->{user}', using primary group/) {
++ $ok = 1;
++ last;
++ }
++ }
++
++ close($fh);
++
++ $self->assert($ok, test_msg("Did not see expected log message"));
++
++ } else {
++ die("Can't read $setup->{log_file}: $!");
++ }
++ };
++ if ($@) {
++ $ex = $@ unless $ex;
++ }
++
++ test_cleanup($setup->{log_file}, $ex);
++}
++
+ 1;
=====================================
debian/patches/series
=====================================
@@ -15,3 +15,4 @@ odbc
# https://github.com/proftpd/proftpd/issues/1831
#03_disable_failing_coding_tests.diff
784a8e28332059cd6f41e7bcfbdc9b0142fe2c13.diff
+5031d498a71c493b9659e2b5ccafde58b0897e30.diff
View it on GitLab: https://salsa.debian.org/debian-proftpd-team/proftpd/-/commit/62a9106217dae98ef203a5237528c94f1fb0c4ea
--
View it on GitLab: https://salsa.debian.org/debian-proftpd-team/proftpd/-/commit/62a9106217dae98ef203a5237528c94f1fb0c4ea
You're receiving this email because of your account on salsa.debian.org.
More information about the Pkg-proftpd-maintainers
mailing list