* [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction @ 2025-05-07 12:42 Adolf Belka 2025-05-07 12:44 ` Michael Tremer 0 siblings, 1 reply; 5+ messages in thread From: Adolf Belka @ 2025-05-07 12:42 UTC (permalink / raw) To: development; +Cc: Adolf Belka - Realised that I had not tested the old password beinhg correct or not. Previous check gave the same answer irrespective of the output coming from the htpasswd verification. - This changes the variable used for the system_output result to an array and then checks if the first element contains the failure message that htpasswd gives if password verification fails. - Tested out with correct and incorrect old passwords and gave the correct answer in both cases. Confirmed also that the check for the user being present works correctly for both an existing and new user name, which it did. Fixes: bug12755 Tested-by: Adolf Belka <adolf.belka@ipfire.org> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> --- html/cgi-bin/chpasswd.cgi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/html/cgi-bin/chpasswd.cgi b/html/cgi-bin/chpasswd.cgi index c00caca20..46c3e02f6 100644 --- a/html/cgi-bin/chpasswd.cgi +++ b/html/cgi-bin/chpasswd.cgi @@ -77,11 +77,11 @@ if ($cgiparams{'SUBMIT'} eq $tr{'advproxy chgwebpwd change password'}) # Check if a user with this name and password exists in the userdb file # and if it does then change the password to the new one my $user = &General::system_output("grep", "$cgiparams{'USERNAME'}", "$userdb"); - my $old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); + my @old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); if (!$user) { $errormessage = $tr{'advproxy errmsg invalid user'}; goto ERROR; - } elsif (!$old_password) { + } elsif (@old_password[0] =~ /password verification failed/) { $errormessage = $tr{'advproxy errmsg password incorrect'}; goto ERROR; } else { -- 2.49.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction 2025-05-07 12:42 [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction Adolf Belka @ 2025-05-07 12:44 ` Michael Tremer 2025-05-07 13:52 ` Adolf Belka 0 siblings, 1 reply; 5+ messages in thread From: Michael Tremer @ 2025-05-07 12:44 UTC (permalink / raw) To: Adolf Belka; +Cc: development Hello Adolf, Thanks for the patch. Is there no return code that we get from htpasswd instead of parsing the output? -Michael > On 7 May 2025, at 13:42, Adolf Belka <adolf.belka@ipfire.org> wrote: > > - Realised that I had not tested the old password beinhg correct or not. Previous check > gave the same answer irrespective of the output coming from the htpasswd verification. > - This changes the variable used for the system_output result to an array and then > checks if the first element contains the failure message that htpasswd gives if > password verification fails. > - Tested out with correct and incorrect old passwords and gave the correct answer in > both cases. Confirmed also that the check for the user being present works correctly > for both an existing and new user name, which it did. > > Fixes: bug12755 > Tested-by: Adolf Belka <adolf.belka@ipfire.org> > Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> > --- > html/cgi-bin/chpasswd.cgi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/html/cgi-bin/chpasswd.cgi b/html/cgi-bin/chpasswd.cgi > index c00caca20..46c3e02f6 100644 > --- a/html/cgi-bin/chpasswd.cgi > +++ b/html/cgi-bin/chpasswd.cgi > @@ -77,11 +77,11 @@ if ($cgiparams{'SUBMIT'} eq $tr{'advproxy chgwebpwd change password'}) > # Check if a user with this name and password exists in the userdb file > # and if it does then change the password to the new one > my $user = &General::system_output("grep", "$cgiparams{'USERNAME'}", "$userdb"); > - my $old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); > + my @old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); > if (!$user) { > $errormessage = $tr{'advproxy errmsg invalid user'}; > goto ERROR; > - } elsif (!$old_password) { > + } elsif (@old_password[0] =~ /password verification failed/) { > $errormessage = $tr{'advproxy errmsg password incorrect'}; > goto ERROR; > } else { > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction 2025-05-07 12:44 ` Michael Tremer @ 2025-05-07 13:52 ` Adolf Belka 2025-05-07 14:02 ` Adolf Belka 0 siblings, 1 reply; 5+ messages in thread From: Adolf Belka @ 2025-05-07 13:52 UTC (permalink / raw) To: Michael Tremer; +Cc: IPFire: Development-List Hi Michael, On 07/05/2025 14:44, Michael Tremer wrote: > Hello Adolf, > > Thanks for the patch. Is there no return code that we get from htpasswd instead of parsing the output? It gives a return code for everything, with numbers of 0 to 7, except for the use of the -v option to verify the password. This gives password verification failed if the existing password for the specified user is not correct and Password for user fred correct. if the user specified was fred and the specified password was correct. It does the above for both the interactive -v and the batch mode using the command line of -bv I had to use the check for if the string was found in the return variable because if I checked if the string matched the contents of the variable it always failed so I think there is a hidden Carriage Return or something in the output from htpasswd for the verification test. Regards, Adolf. > > -Michael > >> On 7 May 2025, at 13:42, Adolf Belka <adolf.belka@ipfire.org> wrote: >> >> - Realised that I had not tested the old password beinhg correct or not. Previous check >> gave the same answer irrespective of the output coming from the htpasswd verification. >> - This changes the variable used for the system_output result to an array and then >> checks if the first element contains the failure message that htpasswd gives if >> password verification fails. >> - Tested out with correct and incorrect old passwords and gave the correct answer in >> both cases. Confirmed also that the check for the user being present works correctly >> for both an existing and new user name, which it did. >> >> Fixes: bug12755 >> Tested-by: Adolf Belka <adolf.belka@ipfire.org> >> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> >> --- >> html/cgi-bin/chpasswd.cgi | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/html/cgi-bin/chpasswd.cgi b/html/cgi-bin/chpasswd.cgi >> index c00caca20..46c3e02f6 100644 >> --- a/html/cgi-bin/chpasswd.cgi >> +++ b/html/cgi-bin/chpasswd.cgi >> @@ -77,11 +77,11 @@ if ($cgiparams{'SUBMIT'} eq $tr{'advproxy chgwebpwd change password'}) >> # Check if a user with this name and password exists in the userdb file >> # and if it does then change the password to the new one >> my $user = &General::system_output("grep", "$cgiparams{'USERNAME'}", "$userdb"); >> - my $old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); >> + my @old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); >> if (!$user) { >> $errormessage = $tr{'advproxy errmsg invalid user'}; >> goto ERROR; >> - } elsif (!$old_password) { >> + } elsif (@old_password[0] =~ /password verification failed/) { >> $errormessage = $tr{'advproxy errmsg password incorrect'}; >> goto ERROR; >> } else { >> -- >> 2.49.0 >> >> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction 2025-05-07 13:52 ` Adolf Belka @ 2025-05-07 14:02 ` Adolf Belka 2025-05-08 13:11 ` Michael Tremer 0 siblings, 1 reply; 5+ messages in thread From: Adolf Belka @ 2025-05-07 14:02 UTC (permalink / raw) To: Michael Tremer; +Cc: IPFire: Development-List Hi Michael, On 07/05/2025 15:52, Adolf Belka wrote: > Hi Michael, > > On 07/05/2025 14:44, Michael Tremer wrote: >> Hello Adolf, >> >> Thanks for the patch. Is there no return code that we get from htpasswd instead of parsing the output? > > It gives a return code for everything, with numbers of 0 to 7, except for the use of the -v option to verify the password. There might be a status code returned. The man page says 3 if the password was entered interactively and the verification entry didn't match but elsewhere it does suggest that interactively is not via the -bv option but where you just use -v and manually type the password when requested on the command line. If the status 3 is a valid status code, how can I access that from the output of the &General::system_output subroutine? I could give it a try out and if it does work then I could do a v3 patch. Regards, Adolf. > This gives > > password verification failed > > if the existing password for the specified user is not correct and > > Password for user fred correct. > > if the user specified was fred and the specified password was correct. > > It does the above for both the interactive -v and the batch mode using the command line of -bv > > I had to use the check for if the string was found in the return variable because if I checked if the string matched the contents of the variable it always failed so I think there is a hidden Carriage Return or something in the output from htpasswd for the verification test. > > Regards, > > Adolf. > >> >> -Michael >> >>> On 7 May 2025, at 13:42, Adolf Belka <adolf.belka@ipfire.org> wrote: >>> >>> - Realised that I had not tested the old password beinhg correct or not. Previous check >>> gave the same answer irrespective of the output coming from the htpasswd verification. >>> - This changes the variable used for the system_output result to an array and then >>> checks if the first element contains the failure message that htpasswd gives if >>> password verification fails. >>> - Tested out with correct and incorrect old passwords and gave the correct answer in >>> both cases. Confirmed also that the check for the user being present works correctly >>> for both an existing and new user name, which it did. >>> >>> Fixes: bug12755 >>> Tested-by: Adolf Belka <adolf.belka@ipfire.org> >>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> >>> --- >>> html/cgi-bin/chpasswd.cgi | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/html/cgi-bin/chpasswd.cgi b/html/cgi-bin/chpasswd.cgi >>> index c00caca20..46c3e02f6 100644 >>> --- a/html/cgi-bin/chpasswd.cgi >>> +++ b/html/cgi-bin/chpasswd.cgi >>> @@ -77,11 +77,11 @@ if ($cgiparams{'SUBMIT'} eq $tr{'advproxy chgwebpwd change password'}) >>> # Check if a user with this name and password exists in the userdb file >>> # and if it does then change the password to the new one >>> my $user = &General::system_output("grep", "$cgiparams{'USERNAME'}", "$userdb"); >>> - my $old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); >>> + my @old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); >>> if (!$user) { >>> $errormessage = $tr{'advproxy errmsg invalid user'}; >>> goto ERROR; >>> - } elsif (!$old_password) { >>> + } elsif (@old_password[0] =~ /password verification failed/) { >>> $errormessage = $tr{'advproxy errmsg password incorrect'}; >>> goto ERROR; >>> } else { >>> -- >>> 2.49.0 >>> >>> >> >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction 2025-05-07 14:02 ` Adolf Belka @ 2025-05-08 13:11 ` Michael Tremer 0 siblings, 0 replies; 5+ messages in thread From: Michael Tremer @ 2025-05-08 13:11 UTC (permalink / raw) To: Adolf Belka; +Cc: IPFire: Development-List Hello Adolf, I just gave this a try: ipfire build chroot (x86_64) root:~$ htpasswd -vb /var/ipfire/auth/users admin ipfire; echo $? User admin not found 6 ipfire build chroot (x86_64) root:~$ htpasswd -b /var/ipfire/auth/users admin ipfire Adding password for user admin ipfire build chroot (x86_64) root:~$ htpasswd -vb /var/ipfire/auth/users admin ipfire; echo $? Password for user admin correct. 0 ipfire build chroot (x86_64) root:~$ htpasswd -vb /var/ipfire/auth/users admin ipfire2; echo $? password verification failed 3 ipfire build chroot (x86_64) root:~$ htpasswd -vb /var/ipfire/auth/users admin2 ipfire2; echo $? User admin2 not found 6 This is in the dev system, so the password file was empty to start with. Basically if the username and password match the return code is zero. If something else happened it isn’t. And this is exactly what I would check for: Okay on zero, not okay on anything else. I would not even case why htpasswd was upset because it does not matter in our use-case. -Michael > On 7 May 2025, at 15:02, Adolf Belka <adolf.belka@ipfire.org> wrote: > > Hi Michael, > > On 07/05/2025 15:52, Adolf Belka wrote: >> Hi Michael, >> On 07/05/2025 14:44, Michael Tremer wrote: >>> Hello Adolf, >>> >>> Thanks for the patch. Is there no return code that we get from htpasswd instead of parsing the output? >> It gives a return code for everything, with numbers of 0 to 7, except for the use of the -v option to verify the password. > There might be a status code returned. The man page says > > 3 if the password was entered interactively and the verification entry didn't match > > but elsewhere it does suggest that interactively is not via the -bv option but where you just use -v and manually type the password when requested on the command line. > > If the status 3 is a valid status code, how can I access that from the output of the &General::system_output subroutine? > > I could give it a try out and if it does work then I could do a v3 patch. > > Regards, > > Adolf. > >> This gives >> password verification failed >> if the existing password for the specified user is not correct and >> Password for user fred correct. >> if the user specified was fred and the specified password was correct. >> It does the above for both the interactive -v and the batch mode using the command line of -bv >> I had to use the check for if the string was found in the return variable because if I checked if the string matched the contents of the variable it always failed so I think there is a hidden Carriage Return or something in the output from htpasswd for the verification test. >> Regards, >> Adolf. >>> >>> -Michael >>> >>>> On 7 May 2025, at 13:42, Adolf Belka <adolf.belka@ipfire.org> wrote: >>>> >>>> - Realised that I had not tested the old password beinhg correct or not. Previous check >>>> gave the same answer irrespective of the output coming from the htpasswd verification. >>>> - This changes the variable used for the system_output result to an array and then >>>> checks if the first element contains the failure message that htpasswd gives if >>>> password verification fails. >>>> - Tested out with correct and incorrect old passwords and gave the correct answer in >>>> both cases. Confirmed also that the check for the user being present works correctly >>>> for both an existing and new user name, which it did. >>>> >>>> Fixes: bug12755 >>>> Tested-by: Adolf Belka <adolf.belka@ipfire.org> >>>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> >>>> --- >>>> html/cgi-bin/chpasswd.cgi | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/html/cgi-bin/chpasswd.cgi b/html/cgi-bin/chpasswd.cgi >>>> index c00caca20..46c3e02f6 100644 >>>> --- a/html/cgi-bin/chpasswd.cgi >>>> +++ b/html/cgi-bin/chpasswd.cgi >>>> @@ -77,11 +77,11 @@ if ($cgiparams{'SUBMIT'} eq $tr{'advproxy chgwebpwd change password'}) >>>> # Check if a user with this name and password exists in the userdb file >>>> # and if it does then change the password to the new one >>>> my $user = &General::system_output("grep", "$cgiparams{'USERNAME'}", "$userdb"); >>>> - my $old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); >>>> + my @old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}"); >>>> if (!$user) { >>>> $errormessage = $tr{'advproxy errmsg invalid user'}; >>>> goto ERROR; >>>> - } elsif (!$old_password) { >>>> + } elsif (@old_password[0] =~ /password verification failed/) { >>>> $errormessage = $tr{'advproxy errmsg password incorrect'}; >>>> goto ERROR; >>>> } else { >>>> -- >>>> 2.49.0 >>>> >>>> >>> >>> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-08 13:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-05-07 12:42 [PATCH v2] chpasswd.cgi: Fixes bug12755 - v2 with password verification correction Adolf Belka 2025-05-07 12:44 ` Michael Tremer 2025-05-07 13:52 ` Adolf Belka 2025-05-07 14:02 ` Adolf Belka 2025-05-08 13:11 ` Michael Tremer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox