From 449bb6cd46c1bb9625d9319f298b594205c5f0f1 Mon Sep 17 00:00:00 2001 From: topunix <833824+topunix@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:21:16 -0500 Subject: [PATCH 1/2] Bug 1851434: guard secure_mail call for installs without SecureMail extension --- Bugzilla/Bug.pm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index a44108fca..5e72c5d3f 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -1108,8 +1108,9 @@ sub update { # we only audit when bugs protected with a secure-mail enabled group # are made public if ( - !scalar @{$self->groups_in} && any { $old_groups{$_}->secure_mail } - @$removed_gr + !scalar @{$self->groups_in} + && Bugzilla->has_extension('SecureMail') + && any { $old_groups{$_}->secure_mail } @$removed_gr ) { Bugzilla->audit(sprintf( From 318df3ff1e0bbd96d7a8faa830db3498fd060dbd Mon Sep 17 00:00:00 2001 From: topunix <833824+topunix@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:23:13 -0500 Subject: [PATCH 2/2] Bug 1851434: restore native password strength check assert_password_is_secure has been an empty stub since 8edd2715c removed Data::Password::passwdqc, so no password strength check has run since. Restore the pre-passwdqc logic from e9adcde46: a minimum length check plus the optional character class rule keyed off the existing password_complexity param. Add a password_check hook so a stricter policy can be supplied by an extension rather than by a core dependency. Remove the six passwdqc_* params, their checkers and their help text, along with t/903-passwdqc-conf.t, since nothing reads them. Update two test fixture passwords that did not satisfy the bmo complexity rule, which is enabled in the CI answers file and had been unenforced while the stub was empty. --- Bugzilla.pm | 20 +++++ Bugzilla/Config/Auth.pm | 90 ------------------- Bugzilla/Hook.pm | 22 +++++ qa/config/checksetup_answers.txt | 2 +- qa/config/generate_test_data.pl | 2 +- t/903-passwdqc-conf.t | 40 --------- .../en/default/admin/params/auth.html.tmpl | 74 --------------- 7 files changed, 44 insertions(+), 206 deletions(-) delete mode 100644 t/903-passwdqc-conf.t diff --git a/Bugzilla.pm b/Bugzilla.pm index 8ce44ec93..652fa2ec0 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -240,6 +240,26 @@ sub github_secret { sub assert_password_is_secure { my ($class, $password1) = @_; + my $reason; + + if (length($password1) < USER_PASSWORD_MIN_LENGTH) { + $reason = 'too short'; + } + elsif (Bugzilla->params->{password_complexity} eq 'bmo') { + my $features = 0; + $features++ if $password1 =~ /[a-z]/; + $features++ if $password1 =~ /[A-Z]/; + $features++ if $password1 =~ /[0-9]/; + $features++ if $password1 =~ /[^A-Za-z0-9]/; + $features++ if length($password1) > 12; + $reason = 'not complex enough' if $features < 3; + } + + # Allow an extension to apply a stricter policy. + Bugzilla::Hook::process('password_check', + {password => $password1, reason => \$reason}); + + ThrowUserError('password_insecure', {reason => $reason}) if $reason; } sub assert_passwords_match { diff --git a/Bugzilla/Config/Auth.pm b/Bugzilla/Config/Auth.pm index 6bf49ade4..0f91db1c1 100644 --- a/Bugzilla/Config/Auth.pm +++ b/Bugzilla/Config/Auth.pm @@ -12,8 +12,6 @@ use strict; use warnings; use Bugzilla::Config::Common; -use Types::Standard qw(Tuple Maybe); -use Types::Common::Numeric qw(PositiveInt); our $sortkey = 300; @@ -95,47 +93,6 @@ sub get_param_list { {name => 'password_check_on_login', type => 'b', default => '1'}, - { - name => 'passwdqc_min', - type => 't', - default => 'undef, 24, 11, 8, 7', - checker => \&_check_passwdqc_min, - }, - - { - name => 'passwdqc_max', - type => 't', - default => '40', - checker => \&_check_passwdqc_max, - }, - - { - name => 'passwdqc_passphrase_words', - type => 't', - default => '3', - checker => \&check_numeric, - }, - - { - name => 'passwdqc_match_length', - type => 't', - default => '4', - checker => \&check_numeric, - }, - - { - name => 'passwdqc_random_bits', - type => 't', - default => '47', - checker => \&_check_passwdqc_random_bits, - }, - - { - name => 'passwdqc_desc', - type => 'l', - default => 'The password must be complex.', - }, - {name => 'auth_delegation', type => 'b', default => 0,}, {name => 'duo_host', type => 't', default => '',}, @@ -161,52 +118,5 @@ sub get_param_list { return @param_list; } -my $passwdqc_min = Tuple [ - Maybe [PositiveInt], - Maybe [PositiveInt], - Maybe [PositiveInt], - Maybe [PositiveInt], - Maybe [PositiveInt], -]; - -sub _check_passwdqc_min { - my ($value) = @_; - my @values = map { $_ eq 'undef' ? undef : $_ } split(/\s*,\s*/, $value); - - unless ($passwdqc_min->check(\@values)) { - return "must be list of five values, that are either integers > 0 or undef"; - } - - my ($max, $max_pos); - my $pos = 0; - foreach my $value (@values) { - if (defined $max && defined $value) { - if ($value > $max) { - return "Int$pos is larger than Int$max_pos ($max)"; - } - } - elsif (defined $value) { - $max = $value; - $max_pos = $pos; - } - $pos++; - } - return ""; -} - -sub _check_passwdqc_max { - my ($value) = @_; - return "must be a positive integer" unless PositiveInt->check($value); - return "must be greater than 8" unless $value > 8; - return ""; -} - -sub _check_passwdqc_random_bits { - my ($value) = @_; - return "must be a positive integer" unless PositiveInt->check($value); - return "must be between 24 and 85 inclusive" - unless $value >= 24 && $value <= 85; - return ""; -} 1; diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index 07ef07fde..ee09b031b 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -1415,6 +1415,28 @@ your template. =back +=head2 password_check + +Allows an extension to apply a stricter password policy than the built-in +check. This is called after the core check has run. + +Params: + +=over + +=item C + +The plain-text password being validated. + +=item C + +A reference to a scalar. If the core check already rejected the password, +it contains the reason. Set it to a short reason string to reject a +password that the core check accepted. Leaving it unchanged accepts the +core result. + +=back + =head2 path_info_whitelist By default, Bugzilla removes the Path-Info information from URLs before diff --git a/qa/config/checksetup_answers.txt b/qa/config/checksetup_answers.txt index 79d9cb90c..6aba213bc 100644 --- a/qa/config/checksetup_answers.txt +++ b/qa/config/checksetup_answers.txt @@ -1,6 +1,6 @@ $answer{'ADMIN_EMAIL'} = 'admin@mozilla.bugs'; $answer{'ADMIN_OK'} = 'Y'; -$answer{'ADMIN_PASSWORD'} = 'password'; +$answer{'ADMIN_PASSWORD'} = 'password01!'; $answer{'ADMIN_REALNAME'} = 'QA Admin'; $answer{'NO_PAUSE'} = 1; $answer{'apache_size_limit'} = 700000; diff --git a/qa/config/generate_test_data.pl b/qa/config/generate_test_data.pl index c5ce33034..a948ba1d0 100644 --- a/qa/config/generate_test_data.pl +++ b/qa/config/generate_test_data.pl @@ -547,7 +547,7 @@ BEGIN Bugzilla::User->create({ login_name => $watch_user, - cryptpassword => "password1", + cryptpassword => "Password1", disable_mail => 1, }); diff --git a/t/903-passwdqc-conf.t b/t/903-passwdqc-conf.t deleted file mode 100644 index 0bc4954fe..000000000 --- a/t/903-passwdqc-conf.t +++ /dev/null @@ -1,40 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. -use strict; -use warnings; -use 5.14.0; -use lib qw( . lib local/lib/perl5 ); -use autodie; - -use Test::More 1.302; -use ok 'Bugzilla::Config::Auth'; - -ok( - length(Bugzilla::Config::Auth::_check_passwdqc_min("undef, 24, 11, 8, 7")) == 0, - "default value is valid" -); -ok( - length(Bugzilla::Config::Auth::_check_passwdqc_min("underf, 24, 11, 8, 7")) - != 0, - "underf is not valid" -); -is( - Bugzilla::Config::Auth::_check_passwdqc_min("undef, 24, 25, 8, 7"), - "Int2 is larger than Int1 (24)", - "25 can't come after 24" -); -ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("")) != 0, - "empty string is invalid"); -ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("24")) != 0, - "24 is invalid"); -ok(length(Bugzilla::Config::Auth::_check_passwdqc_min("-24")) != 0, - "-24 is invalid"); -ok( - length(Bugzilla::Config::Auth::_check_passwdqc_min("10, 10, 10, 10, 0")) != 0, - "10, 10, 10, 10, 0 is invalid"); - -done_testing; diff --git a/template/en/default/admin/params/auth.html.tmpl b/template/en/default/admin/params/auth.html.tmpl index ecadf0aaf..8101ef434 100644 --- a/template/en/default/admin/params/auth.html.tmpl +++ b/template/en/default/admin/params/auth.html.tmpl @@ -24,73 +24,6 @@ desc = "Set up your authentication policies" %] -[% desc_passwdqc_min = BLOCK %] - [Int0, Int1, Int2, Int3, Int4] -

- The minimum allowed password lengths for different kinds of passwords - and passphrases. "undef" can be used to disallow passwords of a given - kind regardless of their length. Each subsequent number is required to - be no larger than the preceding one. - -

- Int0 is used for passwords consisting of characters from one character - class only. The character classes are: digits, lower-case letters, - upper-case letters, and other characters. There is also a special - class for non-ASCII characters, which could not be classified, but are - assumed to be non-digits. -

- Int1 is used for passwords consisting of characters from two character - classes that do not meet the requirements for a passphrase. -

- Int2 is used for passphrases. Note that besides meeting this length - requirement, a passphrase must also consist of a sufficient number of - words (see the "passphrase_words" option below). -

- Int3 and Int4 are used for passwords consisting of characters from - three and four character classes, respectively. - -

- When calculating the number of character classes, upper-case letters - used as the first character and digits used as the last character of a - password are not counted. - -

- In addition to being sufficiently long, passwords are required to - contain enough different characters for the character classes and the - minimum length they have been checked against. -[% END %] - -[% desc_passwdqc_max = BLOCK %] - The maximum allowed password length. This can be used to prevent users - from setting passwords that may be too long for some system services. - It must be larger than 8. -[% END %] - -[% desc_passwdqc_passphrase_words = BLOCK %] - The number of words required for a passphrase, or 0 to disable the - support for user-chosen passphrases. -[% END %] - -[% desc_passwdqc_match_length = BLOCK %] - The length of common substring required to conclude that a password is - at least partially based on information found in a character string, - or 0 to disable the substring search. Note that the password will not - be rejected once a weak substring is found; it will instead be - subjected to the usual strength requirements with the weak substring - partially discounted. -

- The substring search is case-insensitive and is able to detect and - remove a common substring spelled backwards. -[% END %] - -[% desc_random_bits = BLOCK %] - The size of randomly-generated passphrases in bits (24 to 85). -[% END %] - -[% desc_passwdqc_desc = BLOCK %] - This should be a short paragraph describing the password requirements in plain English. - Limited HTML allowed. -[% END %] [% param_descs = { auth_env_id => "Environment variable used by external authentication system " _ @@ -201,13 +134,6 @@ "will be permitted to create their own accounts and all accounts " _ "will have to be created by an administrator.", - passwdqc_min => desc_passwdqc_min, - passwdqc_max => desc_passwdqc_max - passwdqc_passphrase_words => desc_passwdqc_passphrase_words, - passwdqc_match_length => desc_passwdqc_match_length, - passwdqc_random_bits => desc_random_bits, - passwdqc_desc => desc_passwdqc_desc, - password_complexity => "Set the complexity required for passwords. In all cases must the passwords " _ "be at least ${constants.USER_PASSWORD_MIN_LENGTH} characters long." _