Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-09-16 Thread Thomas Goirand
On 09/15/2011 02:13 PM, Joerg Jaspert wrote:
 A total removal from Debian will not help fixing issues or find
 solutions, and keeping it in Experimental sounds like a much much better
 solution to me. I will upload a new version to Experimental soon.
 
 Thanks. To make it easy for all of us - can you do that within a week?

Done.

Thomas



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-09-15 Thread Joerg Jaspert

I really don't want to enter into this deeply or drag a long
discussion. I stated my opinion using my ftpmaster hat, and thats about
it.

I have no interest in digging deeply into this (or other software), nor
do I have the time for it, sorry. For more general topics I suggest
debian-devel@lists.d.o, or maybe one of my fellow colleagues gets around
to write their opinion too.

 I strongly believe that saying such a software writing configuration
 files for Apache, Postfix and bind shouldn't exist at all is wrong.
 There's a demand from users, and many companies have built software
 doing the same things, that they sell for a lot of money (see Plesk,
 cPanel, DirectAdmin, and many others). My previous reading of the policy
 manual was that *maintainer scripts* shouldn’t do that, and I believe
 this was the reading of other DDs too. If currently there's no way to do
 it cleanly, and being policy compliant, then we really need to solve
 this. What are your opinions and ideas?

As that touches FTPMaster stuff again:

Yes, §10.7.4 in policy talks about maintainer scripts
explicitly. Otherwise any software able to do anything with other files
(think of vim, emacs, $EDITOR, even sed) would violate policy.

And yes, having scripts do that to other packages files when called by
the admin, and only then, is fine.

The question then is if the software itself is functional without having
that modification done or not. MUST it have that extra thing done before
its possible to do ANYTHING?

How much people love a set of 3 echos wildly spit out during install
telling the local admin to go and do that because otherwise nothing
happens is a different thing. (Including the fact that notices and stuff
for the admin should NOT be an echo bla but rather use debconf nowadays).

 my thought is that this package should not be in sid. So yes, I support
 those thoughts by the ftp and release team members who spoke up on it
 yet.

 OTOH that makes it harder to keep track how its going on and we only end
 up with it back in NEW whenever, so it won't help anyone.

 My (strong) suggestion is to upload the latest version to experimental
 and then remove the one from sid. Keep it in experimental, get it
 developed further, fixed, whatever. Recheck it in some time (half a
 year, a year, whatever) and see if it would be able to end up in a
 stable release. If so, welcome back in unstable.

 A total removal from Debian will not help fixing issues or find
 solutions, and keeping it in Experimental sounds like a much much better
 solution to me. I will upload a new version to Experimental soon.

Thanks. To make it easy for all of us - can you do that within a week?

-- 
bye, Joerg
I’m a white male, age 18 to 49. Everyone listens to me, no matter how
dumb my suggestions are.



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-09-14 Thread Joerg Jaspert
 I think that going this way, with RC bugs preventing migration, is a
 much more reasonable way to manage the issues.

That depends on the severity of the issues. And on a possible wrong
perception of what unstable actually is.

Wrong: A dumping ground for software, and if it doesn't work out for a
   stable release we just file RC bugs to prevent migration.
   This, quite unneccessary, costs resources on the release team
   (and possibly QA team) side, which are very sparse anyways.

Right: Putting software into unstable means This is software that I, as
   its maintainer, happen to think ready for Debians next stable
   release.

   Sure, this well may have bugs, but then they should be fixed
   timely and the software kept up to par with policy and all other
   requirements for a stable release. And not out of it.

 If you feel like DTC should go away from old-stable, that's your
 decision too, I can accept it (more easily than a removal from SID),
 even though it would go against my opinion, since I think we should
 continue to support what we have in a given distribution until it dies.

Oldstable is a thing for the (O)SRMs, whatever they decide. If they tell
us next time we do a point release to remove it, gone it will be.

Now, reading through this bugreport and skimming over 

http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=dtc

my thought is that this package should not be in sid. So yes, I support
those thoughts by the ftp and release team members who spoke up on it
yet.

OTOH that makes it harder to keep track how its going on and we only end
up with it back in NEW whenever, so it won't help anyone.


My (strong) suggestion is to upload the latest version to experimental
and then remove the one from sid. Keep it in experimental, get it
developed further, fixed, whatever. Recheck it in some time (half a
year, a year, whatever) and see if it would be able to end up in a
stable release. If so, welcome back in unstable.


-- 
bye, Joerg
Das kannst du vielleicht mir erzaehlen, aber nicht jemanden, der Ahnung hat.



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-09-14 Thread Thomas Goirand
Hi,

Thanks for voicing your opinion.

On 09/15/2011 02:15 AM, Joerg Jaspert wrote:
 I think that going this way, with RC bugs preventing migration, is a
 much more reasonable way to manage the issues.
 
 That depends on the severity of the issues. And on a possible wrong
 perception of what unstable actually is.
 
 Wrong: A dumping ground for software, and if it doesn't work out for a
stable release we just file RC bugs to prevent migration.
This, quite unneccessary, costs resources on the release team
(and possibly QA team) side, which are very sparse anyways.
 
 Right: Putting software into unstable means This is software that I, as
its maintainer, happen to think ready for Debians next stable
release.
 
Sure, this well may have bugs, but then they should be fixed
timely and the software kept up to par with policy and all other
requirements for a stable release. And not out of it.

I believe I have done timely fixes of the security issues that have been
(recently) reported. I also uploaded a security update for the
old-stable (which took longer to release), but I still don't have an
answer for that one (see below).

Now, I believe everybody understands that having fixes for being policy
compliant is another much longer process: since I had a different
reading of the policy manual for #637501 and #637622, a major redesign
has to happen. I believe I will find a solution for #637501 in the long
run, thanks to discussions I had with other DDs at Debconf and new
design ideas that I have. But this implies other package maintainers to
cooperate, which might be the harder part here.

I would like to use this opportunity to discuss #637622 here as well.

I have seen many pointing fingers, but very few showing interest in
finding solutions for this problem. Remember that this is an issue that
Debian EDU is also facing. Software generating configuration
automatically is a Debian problem at large. I would be pleased to have
advices and opinion on what would be the right way to do it.

I strongly believe that saying such a software writing configuration
files for Apache, Postfix and bind shouldn't exist at all is wrong.
There's a demand from users, and many companies have built software
doing the same things, that they sell for a lot of money (see Plesk,
cPanel, DirectAdmin, and many others). My previous reading of the policy
manual was that *maintainer scripts* shouldn’t do that, and I believe
this was the reading of other DDs too. If currently there's no way to do
it cleanly, and being policy compliant, then we really need to solve
this. What are your opinions and ideas?

 Oldstable is a thing for the (O)SRMs, whatever they decide. If they tell
 us next time we do a point release to remove it, gone it will be.
 
 Now, reading through this bugreport and skimming over 
 
 http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=dtc

FYI, somebody pointed out that I failed to tag fixed-in for #637630
and #637632, so only (with quotes to tell I understand it's serious
issues) #637501 and #637622 are now relevant to the SID version, and
also I've uploaded a lenny-security version (which I have no news from,
I don't know if it has been accepted or rejected). If the package is to
be removed from Lenny, I'd really like that the security update that I
uploaded be shipped first. There would be nothing more wrong than just
removing the package without giving support to our users.

 my thought is that this package should not be in sid. So yes, I support
 those thoughts by the ftp and release team members who spoke up on it
 yet.
 
 OTOH that makes it harder to keep track how its going on and we only end
 up with it back in NEW whenever, so it won't help anyone.
 
 My (strong) suggestion is to upload the latest version to experimental
 and then remove the one from sid. Keep it in experimental, get it
 developed further, fixed, whatever. Recheck it in some time (half a
 year, a year, whatever) and see if it would be able to end up in a
 stable release. If so, welcome back in unstable.

A total removal from Debian will not help fixing issues or find
solutions, and keeping it in Experimental sounds like a much much better
solution to me. I will upload a new version to Experimental soon.

Cheers,

Thomas Goirand (zigo)



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-09-13 Thread Thomas Goirand
Hi,

After long period of thinking, I still don't agree with the decision to
remove DTC from Debian.

- If you feel DTC is not policy compliant, then I welcome you to fill
bugs, and by the way, you did it already.

- If you feel there's issues with the coding style, then go ahead as
well with the BTS. I don't mind if you send RC bugs against the package
in SID, and if it doesn't migrate until the issues are solved. Please go
ahead and fill one more bug about more though and consistent checks of
user inputs. I don't mind that, I agree with its principle, and I will
work on it. Once this is reworked, I can later ping you for a code
review, and we can see how it goes (P. Kern suggest that he would agree
to review later on...).

I think that going this way, with RC bugs preventing migration, is a
much more reasonable way to manage the issues.

If you feel like DTC should go away from old-stable, that's your
decision too, I can accept it (more easily than a removal from SID),
even though it would go against my opinion, since I think we should
continue to support what we have in a given distribution until it dies.

Thomas

P.S: FYI, I have sent updates in both SID and lenny-security (that last
one may be waiting on the security team approval...)



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-13 Thread Mike O'Connor
On Fri, 12 Aug 2011 17:52:59 +0800, Thomas Goirand tho...@goirand.fr wrote:
 
  * This package depends on being able to modify configuration files of
  other packages. (see #637501 and the bugs referenced in that bug)
 
 Yes, which is the goal of the software, yes.
 

If the gaol this software is to violate debian policy by modifying the
configuratio files of other packages, I don't know why we are wasting so
much time on this, we should just be rid of it.

 Also, I had some discussions with many DDs, some during debconf11, like
 with Ian Jackson, Raphael Hertzog, and many others, on how to fix this
 on a clean way, and I have plans for it.
 

I'm troubled by this notion.  This package has been around for years,
we are supposed to be encouraged by the fact that you have talked to
other developers about how it might possibly be able to comply with
policy sometime in the future?

stew


pgpJ0KocsR6wm.pgp
Description: PGP signature


Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-13 Thread Thomas Goirand
On 08/13/2011 12:38 PM, Mike O'Connor wrote:
 On Sat, 13 Aug 2011 09:27:18 +0800, Thomas Goirand tho...@goirand.fr wrote:
 On 08/13/2011 12:27 AM, Ansgar Burchardt wrote:
  * No priviledge separation: everything -- including apache -- runs as
the user dtc which also owns config files for apache, bind and
others. This probably makes this user root-equivalent.

 But the latest Git version uses sbox to jail each customer in a chroot
 (running on a union filesystem using aufs), making it quite hard to be
 harmful.
 
 And since the dtc user owns the chroot_template directory.  A compromise
 of the dtc user means that any new chroots should be considered
 compromised.

How much of a problem is it, if the web script is in a chroot, and
protected with the setlimits calls of sbox?

Thomas



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-13 Thread Ansgar Burchardt
Thomas Goirand tho...@goirand.fr writes:
 On 08/13/2011 12:27 AM, Ansgar Burchardt wrote:
  * No parameter binding: all SQL queries are build using string
manipulation; most parameters come directly from $_REQUEST, escaping
(via mysql_real_escape_string) is only done in some places.

 No, you are totally discarding
 /usr/share/dtc/shared/drawlib/templates.php. Escaping with
 mysql_real_escape_string isn't needed when there's enough input
 checking.

If you assume you never do mistakes, then yes, that is enough.  You can
add that to the list:

 * Code assumes there are no bugs, breaks horribly if there is one.

  * No scheme used to make sure only sanitized variables are ever used.
Together with the first point this makes SQL injections very likely.

 There is. Each input is checked against a regular expression.

How is this enforced? As long as you access $_REQUEST directly, you
cannot know that this variable has been checked.

 See how
 things are built in each forms in /usr/share/dtc/shared/inc/forms, using
 the dtcListItemsEdit() function, which is object-like manipulation. The
 issue is that in some few places, it's not using that, and that's what
 has been caught for the list.php since this code hasn't been re-factored.

some few places seems to mean quite a lot of places.

 #611680 isn't relevant and has no impact, as written on the BTS.
 
 It has some impact as it makes the code more brittle: when a small
 breach is found, it is easy to escalate it higher.

 Fixing it wouldn't change the issue. If you have access to the central
 MySQL database, it's basically finished anyway, since you have
 credentials to access remote dom0. If you have suggestions on how to
 solve that, let me know, but I don't think there is a solution.

Having a single invalid value in the database does usually not imply
full access to the database.

 #566654 I believe, isn't more dangerous than the /etc/mysql/debian.cnf.
 So unless /etc/mysql/debian.cnf is removed from Debian, fixing #566654
 is useless, IMHO.
 
 dtc is accessible remotely, the system account for mysql should only be
 accessible locally. This means read access to files (or backups) gives
 remote access; to make use of the mysql system account, you would
 already need to have some kind of local access.

 No, there's phpmyadmin anyway.

If it is installed. dtc does not depend on it.

Regards,
Ansgar



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Mike O'Connor
Package: ftp.debian.org
Severity: normal

It's a shame having to do this for a package with an active maintainer, but I
strongly feel like dtc should be removed from debian.  My reasons for thinking
this:

* It seems like anyone that spends any time looking at this package finds
security bugs.

* If you don't want to look specifically for security bugs, there are plenty of
other RC bug s to be found.

* This package depends on being able to modify configuration files of other
packages. (see #637501 and the bugs referenced in that bug)

I'm troubled by the responses that the many security bugs in these packages get
from the maintainer who is also the upstream author.  I'm worried that the
maintainer/upstream author does not have an adequate respect for security
related issues.

stew



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Thomas Goirand
 * It seems like anyone that spends any time looking at this package
 finds security bugs.

No. Other software in Debian with more severe security record didn't
have such kind of bug open. See for example Samba, bind, and many
others. Or maybe you also want these to be removed from Debian?

This is purely your appreciation and your view on my software, I don't
think this is reality.

Also, the fact that I want the software to stay in Debian is precisely
so that it has more eyes to look into the code, and then improve the
quality. This isn't exactly a small software here.

 * If you don't want to look specifically for security bugs, there are
 plenty of other RC bug s to be found.

Which I'm fixing, but please do not force me to hurry on them and do bad
patches.

 * This package depends on being able to modify configuration files of
 other packages. (see #637501 and the bugs referenced in that bug)

Yes, which is the goal of the software, yes.

Also, I had some discussions with many DDs, some during debconf11, like
with Ian Jackson, Raphael Hertzog, and many others, on how to fix this
on a clean way, and I have plans for it.

 I'm troubled by the responses that the many security bugs in these
 packages get from the maintainer who is also the upstream author.
 I'm worried that the maintainer/upstream author does not have an
 adequate respect for security related issues.

And me, I'm really seriously thinking you don't know how to handle
security issues as well, given the fact that you've open public bugs,
when you should have get in touch with me privately. This shows as well
a big disrespect for what I do, if opening this bug wasn't enough.

I have already fixed what Ansgar reported, and I am currently working on
other fixes. Please mind to explain why I do not have adequate respect
for security related issues. I believe I do, since I have in the past
made some QA uploads in timely manners (not even talking about this one
package here).

Cheers,

Thomas



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Philipp Kern
On Fri, Aug 12, 2011 at 05:52:59PM +0800, Thomas Goirand wrote:
  * It seems like anyone that spends any time looking at this package
  finds security bugs.
 No. Other software in Debian with more severe security record didn't
 have such kind of bug open. See for example Samba, bind, and many
 others. Or maybe you also want these to be removed from Debian?
 
 This is purely your appreciation and your view on my software, I don't
 think this is reality.

It is shared by a bunch of people, including myself, though.  Your
responses to the security bugs were below subpar, to put it mildly.
There's not only lack of common sense in security, there's also
ignorance and offensive behaviour.

 Also, the fact that I want the software to stay in Debian is precisely
 so that it has more eyes to look into the code, and then improve the
 quality. This isn't exactly a small software here.

Debian is not an incubator for bad software, I'm afraid.  We're here for
technical excellence.  (C.f. your comment in README.Debian about how
hard it is for you to comply with our beloved policy.)

Your fixes to obvious bugs are also wrong and not properly thought
through.

 And me, I'm really seriously thinking you don't know how to handle
 security issues as well, given the fact that you've open public bugs,
 when you should have get in touch with me privately. This shows as well
 a big disrespect for what I do, if opening this bug wasn't enough.

Stop shooting the messenger, thanks.

Kind regards
Philipp Kern


signature.asc
Description: Digital signature


Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Philipp Kern
On Fri, Aug 12, 2011 at 01:05:38PM +0200, Philipp Kern wrote:
 It is shared by a bunch of people, including myself, though.  Your
 responses to the security bugs were below subpar, to put it mildly.
 There's not only lack of common sense in security, there's also
 ignorance and offensive behaviour.

In case that the bug numbers are not obvious: #614302, #614304, #611680,
#414480, #566654.

For RC bugs: #633616.  I won't hold any older against you, here.

The thing is: At every point in time where someone spends some on your
packages, they find a bunch of RC bugs.  That's a) because the code
quality is insanely bad and b) because the packaging is horrible.

We shouldn't hold back our criticism out of respect, though.  After all
we don't hide bugs.

As much as you might hate public disclosure (I made the same mistake
when I started in Debian), when a bug's public, so be it.  Don't blame
the submitter, he wasted his time on your package to make it better.

Kind regards
Philipp Kern


signature.asc
Description: Digital signature


Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Alexander Reichle-Schmehl
Hi!

Am 12.08.2011 11:52, schrieb Thomas Goirand:

 And me, I'm really seriously thinking you don't know how to handle
 security issues as well, given the fact that you've open public bugs,
 when you should have get in touch with me privately. This shows as well
 a big disrespect for what I do, if opening this bug wasn't enough.

Does the phrase We Won't Hide Problems ring a bell for you?


Best regards,
  Alexander

PS: Also note that dtc isn't part of a stable release, so what's the
problem with opening security related bugs?



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Philipp Kern
On Fri, Aug 12, 2011 at 01:15:34PM +0200, Philipp Kern wrote:
 In case that the bug numbers are not obvious: #614302, #614304, #611680,
 #414480, #566654.
 
 For RC bugs: #633616.  I won't hold any older against you, here.

Other notable bugs (I just looked at dtc-xen's bug list which had some of
dtc-common aswell): #637501, #566650, #616359 (no reply yet since March,
suggests general breakage in the install script)

The newest ones are #637485, #637477, #637469 and #637487.  No, of
course you couldn't have replied to those yet, but they show some
general issues with the code and the packaging.

Kind regards
Philipp Kern


signature.asc
Description: Digital signature


Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Mike O'Connor
Thomas Goirand tho...@goirand.fr wrote:

And me, I'm really seriously thinking you don't know how to handle
security issues as well, given the fact that you've open public bugs,
when you should have get in touch with me privately. This shows as well
a big disrespect for what I do, if opening this bug wasn't enough.

Note that when I first attempted to alert you to the issue that started
http://lists.debian.org/debian-release/2011/07/msg00325.html that first
you obviously didn't actually read my report fully.  My report:

On Mon, 11 Jul 2011 23:43:19 -0400, Mike O'Connor s...@vireo.org wrote:

Although dtc-xen creates a password protected RSA for SSL communication with
the SOAP daemon in /etc/dtc-xen/privkey.pem, it leaves a plaintext copy in
/etc/dtc-xen/dtc-xen.cert.key.

Your reply:

On Fri, 15 Jul 2011 12:33:18 +0200, Thomas Goirand tho...@goirand.fr wrote:
 I don't think there's an grave issue here, the
 key might be world readable, but there is a
 passphrase in it,

But you also ask for it to be disclosed publicly:

On Fri, 15 Jul 2011 12:33:18 +0200, Thomas Goirand tho...@goirand.fr wrote:
 if someone can
 submit this bug in the BTS for me (with this message
 in the bug entry) I'd be fracking grateful!



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Ansgar Burchardt
Thomas Goirand tho...@goirand.fr writes:
 Now, let me take a step back and try to comment on the big picture.

 To make things clear: I do understand that these last MySQL and shell
 insertion are really serious issues with grave consequences, and I
 agree that some parts of the code is sloppy to say the least. I am the
 one to blame for sure, for not looking enough at contributions and
 re-factoring code enough. I really don't want you guys to think that
 I'm not taking it seriously. I'd like however that you understand that
 a lot of work has been recently (over let's say the last 2 years), to
 re-factor the code. And that this work will continue.

I do not think any single of the reported bug lead to this removal
request, but when I did look at dtc's code I did find several systematic
problems all over the code (not only in some parts) that resulted in all
the security bugs I reported so far:

 * No parameter binding: all SQL queries are build using string
   manipulation; most parameters come directly from $_REQUEST, escaping
   (via mysql_real_escape_string) is only done in some places.
 * No scheme used to make sure only sanitized variables are ever used.
   Together with the first point this makes SQL injections very likely.
 * Checking variables and using them often happens at totally different
   places (often different files). This makes it even harder to make
   sure they are always safe to use.
 * Related to the last point, control flow is a mess. Most seems to be
   sphagetti code, there is a lot of duplication, many global variables,
   etc.
 * Variables used in HTML output are not escaped either. I did not yet
   look at this in detail.
 * exec(...) is used. It's not really needed for appending lines to a
   given file, and most other uses do at least not require a shell to be
   involved either.
 * No priviledge separation: everything -- including apache -- runs as
   the user dtc which also owns config files for apache, bind and
   others. This probably makes this user root-equivalent.

All of this together make it in my opinion impossible to write a secure
program, many parts seem to do the exact opposite of good practices.

The rest of this mail are some comments on specific bugs. Feel free to
skip them, they do not matter too much to this discussion, but I think
demonstrate some of the problems above:

 #611680 isn't relevant and has no impact, as written on the BTS.

It has some impact as it makes the code more brittle: when a small
breach is found, it is easy to escalate it higher.

 #566654 I believe, isn't more dangerous than the /etc/mysql/debian.cnf.
 So unless /etc/mysql/debian.cnf is removed from Debian, fixing #566654
 is useless, IMHO.

dtc is accessible remotely, the system account for mysql should only be
accessible locally. This means read access to files (or backups) gives
remote access; to make use of the mysql system account, you would
already need to have some kind of local access.

In any case I don't think other programs being not perfect should not
invalidate this bug (I would prefer mysql would not need the system
account, but that's a different topic).

[ About #637477 and #637487: ]
 I have already fixed what Ansgar reported, and I am currently working
 on other fixes. [...]

These patches[1] look more like a bandaid than a proper solution. I hope
you do not see them as fixed :/

[1] 
http://git.gplhost.com/gitweb/?p=dtc.git;a=commitdiff;h=01c4eea40736b89cb396b0596f2808c8f64b8729

 [#637482] (eg: the issue is the hostname of the server not being
 configured correctly, and hostname --domain not replying anything,
 which isn't a DTC issue per say, but more an administrator issue).

This is minor, but dtc did not complain when I gave an empty domain
name.  It should give a proper error message when it does expect one.

Regards,
Ansgar



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Thomas Goirand
On 08/13/2011 12:27 AM, Ansgar Burchardt wrote:
  * No parameter binding: all SQL queries are build using string
manipulation; most parameters come directly from $_REQUEST, escaping
(via mysql_real_escape_string) is only done in some places.

No, you are totally discarding
/usr/share/dtc/shared/drawlib/templates.php. Escaping with
mysql_real_escape_string isn't needed when there's enough input checking.

  * No scheme used to make sure only sanitized variables are ever used.
Together with the first point this makes SQL injections very likely.

There is. Each input is checked against a regular expression. See how
things are built in each forms in /usr/share/dtc/shared/inc/forms, using
the dtcListItemsEdit() function, which is object-like manipulation. The
issue is that in some few places, it's not using that, and that's what
has been caught for the list.php since this code hasn't been re-factored.

  * Checking variables and using them often happens at totally different
places (often different files). This makes it even harder to make
sure they are always safe to use.

This avoids code duplication which you are criticizing just below.

  * Related to the last point, control flow is a mess. Most seems to be
sphagetti code, there is a lot of duplication, many global variables,
etc.

I love the many global variable one. That's typical from someone that
has a quick look, but doesn't see further. Global variable are used in
very specific cases. For table names and configuration variables (coming
from the config table field names). These are there to force
declaration of what a function uses. The only place where what you are
saying is relevant are whats in shared/vars/global_vars.php, where you
see 8 global variables (which is where you found one issue).

  * No priviledge separation: everything -- including apache -- runs as
the user dtc which also owns config files for apache, bind and
others. This probably makes this user root-equivalent.

But the latest Git version uses sbox to jail each customer in a chroot
(running on a union filesystem using aufs), making it quite hard to be
harmful.

 #611680 isn't relevant and has no impact, as written on the BTS.
 
 It has some impact as it makes the code more brittle: when a small
 breach is found, it is easy to escalate it higher.

Fixing it wouldn't change the issue. If you have access to the central
MySQL database, it's basically finished anyway, since you have
credentials to access remote dom0. If you have suggestions on how to
solve that, let me know, but I don't think there is a solution.

 #566654 I believe, isn't more dangerous than the /etc/mysql/debian.cnf.
 So unless /etc/mysql/debian.cnf is removed from Debian, fixing #566654
 is useless, IMHO.
 
 dtc is accessible remotely, the system account for mysql should only be
 accessible locally. This means read access to files (or backups) gives
 remote access; to make use of the mysql system account, you would
 already need to have some kind of local access.

No, there's phpmyadmin anyway.

 [ About #637477 and #637487: ]
 I have already fixed what Ansgar reported, and I am currently working
 on other fixes. [...]
 
 These patches[1] look more like a bandaid than a proper solution. I hope
 you do not see them as fixed :/
 
 [1] 
 http://git.gplhost.com/gitweb/?p=dtc.git;a=commitdiff;h=01c4eea40736b89cb396b0596f2808c8f64b8729

Right, I intend to fix it better, but that's a quick fix before I have
time for a better solution.

Thomas



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Mike O'Connor
On Sat, 13 Aug 2011 09:27:18 +0800, Thomas Goirand tho...@goirand.fr wrote:
 On 08/13/2011 12:27 AM, Ansgar Burchardt wrote:
   * No priviledge separation: everything -- including apache -- runs as
 the user dtc which also owns config files for apache, bind and
 others. This probably makes this user root-equivalent.
 
 But the latest Git version uses sbox to jail each customer in a chroot
 (running on a union filesystem using aufs), making it quite hard to be
 harmful.
 

And since the dtc user owns the chroot_template directory.  A compromise
of the dtc user means that any new chroots should be considered
compromised.

The www-data user that apache normally runs under has very little
privileges for a reason.  On sanely setup systems, the www-data user
doesn't get to modify many files at all.  In your setup, a compromise of
the webserver gets to modify the named configuration, the mta
configuration, gets to modify, for instance, the ls binary that gets
installed into the chroots you mention above...


pgplhoyaej2ja.pgp
Description: PGP signature


Bug#637509: RM: dtc -- RoQA; consistently buggy and non-policy compliant

2011-08-12 Thread Mike O'Connor
On Fri, 12 Aug 2011 17:52:59 +0800, Thomas Goirand tho...@goirand.fr wrote:
  * It seems like anyone that spends any time looking at this package
  finds security bugs.
 
.snip.
 
 This is purely your appreciation and your view on my software, I don't
 think this is reality.
 

I was waiting for something in the oven tonight before I go to bed, and
I find 3 more security bugs: #637617, #637618, #637619

stew



pgpXW5Kxs84T7.pgp
Description: PGP signature