|
From: Christian B. <pos...@cb...> - 2007-10-06 21:58:03
|
Hello,
Am Freitag, 5. Oktober 2007 schrieb David Goodwin:
> > > > I consider it security-critical to include the password in the
> > > > HTML code (browser cache etc.). Luckily, this code seems to be
> > > > buggy - at least, it never included the password for me.
> > >
> > > Ah, it should say $tPassword. I'd never intentionally display the
> > > unencrypted password in the form.
> >
> > This isn't really better - if someone uses unencrypted passwords,
> > there will be a cleartext password again.
>
> Ah, bugg-rit. I'd forgotten about that 'feature'
>
> As these are the passwords for the admin user, how about we change is
> so admin passwords are _always_ encrypted with something decent? As
> admin passwords aren't used for mailboxes, it wouldn't have any
> impact outside of postfixadmin.
Would be an idea. However, we'll need a way to handle existing=20
unencrypted passwords.
> > The correct and secure solution is not to insert the password at
> > all.
>
> My grievance with this is that when ever an admin is edited, I think
> your previous patch, implied that a users password had to be
> changed/set. Which is horrible.
No, the password does not need to be touched. See my change in r123:
=2D-- admin/edit-admin.php (Revision 107)
+++ admin/edit-admin.php (Revision 123)
@@ -89,2 +89,5 @@
=2D $result =3D db_query ("UPDATE $table_admin SET=20
modified=3DNOW(),active=3D'$sqlActive', password=3D'$fPassword' WHERE=20
username=3D'$username'");
=2D
+ $password_query =3D '';
+ if ($fPassword !=3D '') { # do not change password to empty one
+ $password_query =3D ", password=3D'$fPassword'";
+ }
+ $result =3D db_query ("UPDATE $table_admin SET=20
modified=3DNOW(),active=3D'$sqlActive' $password_query WHERE=20
username=3D'$username'");
In short: If no password was entered, do not touch it in the UPDATE=20
query (the $password_query part is empty in this case).
> > > <Smarty + Prepared statements would make me very happy />
> >
> > I don't know Smarty good enough to add a statement, but I'm afraid
> > it would add some overhead. There are other ways to make the
> > templates easier...
>
> To which I'd say :
>
> 1) Postfixadmin isn't going to be in a high demand, zillions of users
> style situation, so performance doesn't matter (that much!)
;-)
> 2) Using Smarty would stop the automatic spread of variables from the
> controller to the view - which would make it far more obvious what is
> given to the view layer, and what isn't.
Hmm, there are other ways to reach this goal. I don't say that Smarty is=20
bad, but you can reach _this_ goal by putting the templates into=20
functions or classes.
Anyway: The templates need a big cleanup first. Maybe I can even make a=20
nice render_table() function for all the list-* pages...
> 3) We can tell Smarty to sanitise all data it displays by default,
> rather than us having to run htmlentities everywhere, so we can stop
> XSS/HTML injection like problems very easily.
I'd say this is also solvable with functions or classes - you can=20
sanitize all parameters when calling the function ;-)
> > Prepared statements would be a good idea - we would get rid of SQL
> > injections easily.
>
> Indeed. Question is, what version(s) of PHP/MySQL to we need to
> support.=20
I checked php.net about this. If I didn't miss something, PHP does not=20
provide a prepare / execute for MySQL. We would have to use the MySQL=20
PREPARE and EXECUTE statements as query manually.
=46or pgsql prepared statement support was added in PHP 5.1.
> If we're thinking of older ones, then we'd have to use=20
> something like PEAR::DB or PEAR::MDB2 - which emulate
> prepared-statement-ness.
Ah, the old question of using an abstraction layer. I'm still not sure=20
if we really need it - but having some helper functions (for example
insert("table", array ("field1" =3D> "value1", "field2" =3D> "value2")
that create database statements might be a good idea.
> I added a min_password_length config setting in.... which should help
> too ($CONF['min_password_length'])
Yes, I already hit this in the test installation on my laptop ;-) (which=20
doesn't need strong passwords - it's not connected to Postfix at all).
Regards,
Christian Boltz
=2D-=20
[PDF] Fipptehler korrigiert -- wie konnten wir das bisher uebersehen ;)
=2D <xsl:if test=3D"position()=3D1">Sch=FCsselworte: </xsl:if>
+ <xsl:if test=3D"position()=3D1">Schl=FCsselworte: </xsl:if>
[David Haller in suse-linux-faq]
|
