diff --git a/bin/admin/check-consistency.pl b/bin/admin/check-consistency.pl index 75c23f8..a8d6f36 100755 --- a/bin/admin/check-consistency.pl +++ b/bin/admin/check-consistency.pl @@ -4,12 +4,15 @@ use common::sense; use Data::Dumper; use Term::ANSIColor; use Digest::MD5 (); - use File::Basename; + +use lib dirname(__FILE__) . '/../../lib/perl'; +use OVH::Bastion; + my $BASEDIR = dirname(__FILE__) . '/../..'; -my $MIN_KEYGROUP_GID = 2000; -my $MAX_KEYGROUP_GID = 99999; +my $MIN_KEYGROUP_GID = OVH::Bastion::config("groupGidMin")->value; +my $MAX_KEYGROUP_GID = $MIN_KEYGROUP_GID + 99999; my @KEY_GROUPS_IGNORE = qw{ keeper reader }; my $HOME_SUBDIRS_IGNORE_RE = qr{^^}; diff --git a/bin/admin/fix-group-gid.sh b/bin/admin/fix-group-gid.sh index 4c1de70..fbca242 100755 --- a/bin/admin/fix-group-gid.sh +++ b/bin/admin/fix-group-gid.sh @@ -1,12 +1,16 @@ #! /usr/bin/env bash # vim: set filetype=sh ts=4 sw=4 sts=4 et: +# This script ensures that GIDs of bastion groups (including their group system +# roles) are higher than the configured groupGidMin value. +# It shifts group GIDs from the low range (where they may collide with reserved +# account UIDs) to the high range defined by groupGidMin. set -e basedir=$(readlink -f "$(dirname "$0")"/../..) # shellcheck source=lib/shell/functions.inc . "$basedir"/lib/shell/functions.inc -MINGID=10000 +MINGID=$(perl -I"$basedir/lib/perl" -MOVH::Bastion -e 'print OVH::Bastion::config("groupGidMin")->value') if [ -n "$2" ] || [ -z "$1" ] ; then echo "Usage: $0 " @@ -34,13 +38,12 @@ _run() fi } +next_available_gid=$((MINGID + 1)) find_next_available_gid() { - nextgid=$((MINGID + 1)) - while getent group "$nextgid" >/dev/null; do - nextgid=$((nextgid + 1)) + while getent group "$next_available_gid" >/dev/null; do + next_available_gid=$((next_available_gid + 1)) done - echo $nextgid } change_gid() @@ -63,7 +66,8 @@ change_gid() [ "$oldgid" -ge "$MINGID" ] && return - newgid=$(find_next_available_gid) + find_next_available_gid + newgid=$next_available_gid _run group_change_gid_compat "$group" "$newgid" tocheck="" @@ -106,13 +110,15 @@ main() return fi - echo - echo "$group: OK to proceed? (CTRL+C to abort). You'll still have to validate each commands I'm going to run" - # shellcheck disable=SC2034 - read -r ___ - really_run_commands=1 - batchrun - echo "$group: done." + if [ -z "${DRY_RUN:-}" ] || [ "$DRY_RUN" = 0 ]; then + echo + echo "$group: OK to proceed? (CTRL+C to abort). You'll still have to validate each commands I'm going to run" + # shellcheck disable=SC2034 + read -r ___ + really_run_commands=1 + batchrun + echo "$group: done." + fi } if [ "$1" = "ALL" ]; then diff --git a/bin/helper/osh-groupCreate b/bin/helper/osh-groupCreate index d4c8ff9..b2f5435 100755 --- a/bin/helper/osh-groupCreate +++ b/bin/helper/osh-groupCreate @@ -153,11 +153,19 @@ if ($encrypted) { # First create group osh_info("Creating groups"); -foreach my $tocreate ($group, "$group-aclkeeper", "$group-gatekeeper", "$group-owner") { - $fnret = OVH::Bastion::sys_groupadd(group => $tocreate, noisy_stderr => 1); + +# Create all 4 groups with allocated GIDs to avoid collisions with account UIDs +my @groups_to_create = ($group, "$group-aclkeeper", "$group-gatekeeper", "$group-owner"); +foreach my $tocreate (@groups_to_create) { + $fnret = OVH::Bastion::get_next_available_gid(); + $fnret or HEXIT('ERR_GID_ALLOCATION_FAILED', msg => "Failed to allocate GID for group $tocreate: " . $fnret->msg); + my $gid = $fnret->value; + + $fnret = OVH::Bastion::sys_groupadd(group => $tocreate, gid => $gid, noisy_stderr => 1); $fnret->err eq 'OK' - or - HEXIT('ERR_GROUPADD_FAILED', msg => "Error while running groupadd command for $tocreate (" . $fnret->msg . ")"); + or HEXIT('ERR_GROUPADD_FAILED', + msg => "Error while running groupadd command for $tocreate with GID $gid (" . $fnret->msg . ")"); + osh_debug("Created group $tocreate with GID $gid"); } osh_debug("Creating directory"); diff --git a/doc/sphinx/administration/configuration/bastion_conf.rst b/doc/sphinx/administration/configuration/bastion_conf.rst index 540da34..5295852 100644 --- a/doc/sphinx/administration/configuration/bastion_conf.rst +++ b/doc/sphinx/administration/configuration/bastion_conf.rst @@ -142,6 +142,7 @@ These options are either discouraged (in which case this is explained in the des - `accountUidMin`_ - `accountUidMax`_ +- `groupGidMin`_ - `ttyrecGroupIdOffset`_ - `documentationURL`_ - `debug`_ @@ -1007,6 +1008,17 @@ accountUidMax Maximum allowed UID for accounts on this bastion. +.. _groupGidMin: + +groupGidMin +*********** + +:Type: ``int >= 10000`` + +:Default: ``500000`` + +Minimum GID for bastion groups. Should be set high enough (>> accountUidMax) to avoid collision with account UIDs/GIDs. + .. _ttyrecGroupIdOffset: ttyrecGroupIdOffset diff --git a/etc/bastion/bastion.conf.dist b/etc/bastion/bastion.conf.dist index 0961fe4..b008c6f 100644 --- a/etc/bastion/bastion.conf.dist +++ b/etc/bastion/bastion.conf.dist @@ -472,6 +472,11 @@ # DEFAULT: 99999 "accountUidMax": 99999, # +# groupGidMin (int >= 10000) +# DESC: Minimum GID for bastion groups. Should be set high enough (>> accountUidMax) to avoid collision with account UIDs/GIDs. +# DEFAULT: 500000 +"groupGidMin": 500000, +# # ttyrecGroupIdOffset (int > 0) # DESC: Offset to apply on user group uid to create its ``-tty`` group, should be > ``accountUidMax - accountUidMin`` to ensure there is no overlap. # DEFAULT: 100000 diff --git a/lib/perl/OVH/Bastion/allowkeeper.inc b/lib/perl/OVH/Bastion/allowkeeper.inc index 2fb0a0c..fda6058 100644 --- a/lib/perl/OVH/Bastion/allowkeeper.inc +++ b/lib/perl/OVH/Bastion/allowkeeper.inc @@ -153,6 +153,30 @@ sub get_next_available_uid { return R('ERR_UID_COLLISION', msg => "No available UID in the allowed range"); } +sub get_next_available_gid { + my %params = @_; + + # Get the minimum GID from configuration (should be >> accountUidMax to avoid collisions) + my $groupGidMin = OVH::Bastion::config('groupGidMin')->value(); + + # Start from groupGidMin and find the next available GID + my $next = $groupGidMin; + my $maxAttempts = 100_000; # safety limit + my $attempts = 0; + + while ($attempts < $maxAttempts) { + # Check if this GID is available + if (!scalar(getgrgid($next))) { + # Found an available GID + return R('OK', value => $next); + } + $next++; + $attempts++; + } + + return R('ERR_GID_COLLISION', msg => "No available GID found in the allowed range starting from $groupGidMin"); +} + sub is_bastion_account_valid_and_existing { my %params = @_; my $fnret = OVH::Bastion::is_account_valid(%params); diff --git a/lib/perl/OVH/Bastion/configuration.inc b/lib/perl/OVH/Bastion/configuration.inc index 4696527..cab1d0d 100644 --- a/lib/perl/OVH/Bastion/configuration.inc +++ b/lib/perl/OVH/Bastion/configuration.inc @@ -190,28 +190,29 @@ sub load_configuration { # 2/6) Options that must be numbers, between min and max. foreach my $o ( - {name => 'accountUidMin', min => 100, max => 999_999_999, default => 2000}, - {name => 'accountUidMax', min => 100, max => 999_999_999, default => 99999}, - {name => 'ttyrecGroupIdOffset', min => 1, max => 999_999_999, default => 100_000}, - {name => 'minimumIngressRsaKeySize', min => 1024, max => 16384, default => 2048}, - {name => 'minimumEgressRsaKeySize', min => 1024, max => 16384, default => 2048}, - {name => 'maximumIngressRsaKeySize', min => 1024, max => 32768, default => 8192}, - {name => 'maximumEgressRsaKeySize', min => 1024, max => 32768, default => 8192}, - {name => 'moshTimeoutNetwork', min => 0, max => 86400 * 365, default => 86400}, - {name => 'moshTimeoutSignal', min => 0, max => 86400 * 365, default => 30}, - {name => 'idleLockTimeout', min => 0, max => 86400 * 365, default => 0}, - {name => 'idleKillTimeout', min => 0, max => 86400 * 365, default => 0}, - {name => 'warnBeforeLockSeconds', min => 0, max => 86400 * 365, default => 0}, - {name => 'warnBeforeKillSeconds', min => 0, max => 86400 * 365, default => 0}, - {name => 'MFAPasswordInactiveDays', min => -1, max => 365 * 5, default => -1}, - {name => 'MFAPasswordMinDays', min => 0, max => 365 * 5, default => 0}, - {name => 'MFAPasswordMaxDays', min => 0, max => 365 * 5, default => 90}, - {name => 'MFAPasswordWarnDays', min => 0, max => 365 * 5, default => 15}, - {name => 'sshClientDebugLevel', min => 0, max => 3, default => 0}, - {name => 'accountMaxInactiveDays', min => 0, max => 365 * 5, default => 0}, - {name => 'interactiveModeTimeout', min => 0, max => 86400 * 365, default => 15}, - {name => 'interactiveModeProactiveMFAexpiration', min => 0, max => 86400, default => 900}, - {name => 'dnsSupportLevel', min => 0, max => 2, default => 2}, + {name => 'accountUidMin', min => 100, max => 999_999_999, default => 2000}, + {name => 'accountUidMax', min => 100, max => 999_999_999, default => 99999}, + {name => 'groupGidMin', min => 10000, max => 999_999_999, default => 500_000}, + {name => 'ttyrecGroupIdOffset', min => 1, max => 999_999_999, default => 100_000}, + {name => 'minimumIngressRsaKeySize', min => 1024, max => 16384, default => 2048}, + {name => 'minimumEgressRsaKeySize', min => 1024, max => 16384, default => 2048}, + {name => 'maximumIngressRsaKeySize', min => 1024, max => 32768, default => 8192}, + {name => 'maximumEgressRsaKeySize', min => 1024, max => 32768, default => 8192}, + {name => 'moshTimeoutNetwork', min => 0, max => 86400 * 365, default => 86400}, + {name => 'moshTimeoutSignal', min => 0, max => 86400 * 365, default => 30}, + {name => 'idleLockTimeout', min => 0, max => 86400 * 365, default => 0}, + {name => 'idleKillTimeout', min => 0, max => 86400 * 365, default => 0}, + {name => 'warnBeforeLockSeconds', min => 0, max => 86400 * 365, default => 0}, + {name => 'warnBeforeKillSeconds', min => 0, max => 86400 * 365, default => 0}, + {name => 'MFAPasswordInactiveDays', min => -1, max => 365 * 5, default => -1}, + {name => 'MFAPasswordMinDays', min => 0, max => 365 * 5, default => 0}, + {name => 'MFAPasswordMaxDays', min => 0, max => 365 * 5, default => 90}, + {name => 'MFAPasswordWarnDays', min => 0, max => 365 * 5, default => 15}, + {name => 'sshClientDebugLevel', min => 0, max => 3, default => 0}, + {name => 'accountMaxInactiveDays', min => 0, max => 365 * 5, default => 0}, + {name => 'interactiveModeTimeout', min => 0, max => 86400 * 365, default => 15}, + {name => 'interactiveModeProactiveMFAexpiration', min => 0, max => 86400, default => 900}, + {name => 'dnsSupportLevel', min => 0, max => 2, default => 2}, ) { if (not defined $C->{$o->{'name'}}) { @@ -456,6 +457,19 @@ sub load_configuration { $C->{'ttyrecGroupIdOffset'} = $fixed; } + # ... groupGidMin must be high enough to avoid overlap with account UIDs and ttyrec groups + my $minRequiredGroupGid = $C->{'accountUidMax'} + $C->{'ttyrecGroupIdOffset'} + 1; + if ($C->{'groupGidMin'} < $minRequiredGroupGid) { + my $fixed = $minRequiredGroupGid; + push @errors, + "Configuration error: the configured 'groupGidMin' (" + . $C->{'groupGidMin'} + . ") would overlap with account UIDs and/or ttyrec GIDs (max ttyrec GID would be " + . ($C->{'accountUidMax'} + $C->{'ttyrecGroupIdOffset'}) + . "), setting it to $fixed"; + $C->{'groupGidMin'} = $fixed; + } + # ... ensure min <= max foreach my $key (qw{ Ingress Egress }) { my $minkey = "minimum${key}RsaKeySize";