Sunday, 2012-06-10

*** Joins: nextgens (~florent@freenet/developer/nextgens)06:38
nextgenshi06:40
nextgenshow to report a security vulnerability on mantis?06:41
nextgensI've tried emailing security@mantisbt.org, but it doesn't exist06:41
nextgensshould I use the bugtracker and mark the issue private? or use the contact form on the website?06:42
nextgensor do something else?06:42
*** Joins: paulr (~IceChat09@cpc1-enfi15-2-0-cust580.hari.cable.virginmedia.com)06:52
*** Joins: dhx1 (~anonymous@60-242-247-232.static.tpgi.com.au)07:33
paulrlo07:46
dhx1paulr: hey07:51
dhx1PULSEAUDIO GRRRRRR07:51
dhx1it may well be worse than PHP :)07:53
paulrheh07:56
paulrhorrible db queries are horrible07:56
paulr:)07:56
nextgenscan't be worst than mantis' session handling.07:56
paulrhttp://pastebin.com/JnL0Q3cr07:57
dhx1nextgens: haha07:57
paulrit can07:57
dhx1nextgens: have you considered the master-1.3.x branch of MantisBT? It uses X-Content-Security-Policy07:57
paulror the 2.x branch07:57
paulrheh07:57
dhx1session handling and authentication in MantisBT are 10 years old and desperately need replacement07:58
nextgensno; we're running 1.2.1107:58
nextgenshttp://code.bulix.org/w1h0fu-81634?raw07:59
dhx1nextgens: mantisbt.org/bugs + set the issue as private08:00
paulrdare I ask what you found08:00
dhx1nextgens: or let me know directly (d@hx.id.au) and I can have it fixed within a few minutes :)08:00
nextgensmy issue is related to session handling; it's not far from trivial to do authentication bypass08:00
nextgensit's definitely practical08:01
nextgensbut reading http://www.mantisbt.org/bugs/view.php?id=11296 I suspect it's a known issue08:01
nextgenshttp://code.bulix.org/fqzb4t-8163308:02
nextgensthat's how the magic cookie is generated on 1.2.1108:02
nextgenshow many bits of entropy do you think there is in the output of auth_generate_cookie_string() ?08:02
dhx1nextgens: for some bizarre reason we still store session information in the database (bad, bad, bad!)08:03
nextgenswell, I could leave with that08:03
nextgensbut not with a session with a very low amount of entropy... which is live until you change your password!08:03
dhx1nextgens: the correct approach would be to offload that to PHP's session handling with appropriate expiry08:03
nextgensyes08:04
nextgensbut that's only part of the problem08:04
nextgensyou also need to change the sesion key when you change priviledges08:04
nextgens(ie: are granted/revoked any priviledge)08:04
paulrat same time there's 32 bytes of stuff that's arguably known and 32 bytes that's fairly random08:04
paulryou'd do well to work that out08:04
nextgensand obviously use a different identifier for each 'browser'08:05
nextgenspaulr> it's a lot less than that in practice08:05
nextgensyou use mersenne twister08:05
dhx1nextgens: the master-1.3.x branch generates cryptographically secure nonces (OpenSSL's PRNG or falling back to /dev/urandom) for the session cookie, CSRF nonces, CAPTCHA verification, signup URLs, etc08:05
nextgenstwo subsequent calls to mersenne twister08:05
paulrdhx1: that'l lbe why I can't find that function anymore :)08:05
nextgenswhich is a very bad idea08:05
nextgensand well, a unix timestamp08:06
nextgensthat's less than 20bits of entropy overall08:06
paulrdo {08:06
paulr$t_cookie_string = crypto_generate_uri_safe_nonce( 64 );08:06
paulrand yea08:06
nextgensdhx1> people (like me) run 1.2.11 :p08:06
paulrthe dhx says, in next main release it completely changes08:06
paulrdhx1: go write a CVE :P08:06
dhx1nextgens: absolutely agreed, I would like to see 1.3.x out the door quickly as well because it is _significantly_ more secure than 1.2.x08:07
nextgensnow, to quantify how much entropy there actually is, it's difficult08:07
* paulr would like to see 1.3 scrapped 08:07
nextgensthanks to php's awkwardness08:07
nextgenshttp://www.suspekt.org/2008/08/17/mt_srand-and-not-so-random-numbers/08:07
nextgensit depends on the version of the interpreter, the platform it runs on, ... whether it runs as mod_php or mod_cgi08:08
paulryep, there's been some bugfixes to that08:08
paulrI remember when those first came out as issues08:08
paulrwas it really 4 years ago? :(08:08
nextgensand obviously whether there's another 'oracle' available on the same vhost08:08
nextgensbut in practice it's a vulnerability in my books08:08
* paulr nods08:09
paulrI was having the discussion with dhx yesterday about security vulnerability reporting now-a-days and new features08:10
dhx1ah, the terrible days of mod_php (one PHP process with multiple threads)08:10
paulrin that, mantis is quite an old platform08:11
nextgensimho applications like mantis shouldn't deal with authentication08:11
paulrwe've been working at changing the db layer for example to something that might be more secure, as well the crypto stuff dhx jut mentioned08:11
nextgensthey should use a SSO platform and rely on that08:11
dhx1PHP had numerous vulnerabilities arising from an inability to correctly seed the internal PRNG for each thread08:11
paulrdepends what you have available as a SSO platform08:12
nextgensauthentication is something silly, but tricky to get right08:12
* paulr looks at linkedin08:12
paulryea: )08:12
nextgenswhatever; openid, oauth, whatever08:12
nextgensCAS, browserid, ... it's not like there's a shortage of solutions08:13
dhx1I have been wishing for the past 10 years for a replacement to X.509/PKI (centralised architectural trust model) for use on the web... think web-of-trust PGP style08:13
nextgensdhx1> that works for geeks but not for 'lambda' users08:13
paulrI think the problem with something like mantis is it's a legacy app08:13
dhx1nextgens: many of these systems are needlessly complex08:13
paulratm, i've been working on a test mantis-2.x branch, dhx on a next branch, and then we've kinda got a 1.3.x branch08:14
dhx1nextgens: and I am hesitant of authentication/security systems that are more complex than required08:14
paulrall 3 of which contain slightly different ideas for the future :)08:14
* paulr just uses IIS authentication at work08:14
nextgensis any of these branches stable enough to use in production?08:15
paulri'd say no08:15
nextgens^-^08:15
dhx1nextgens: 1.3.x is fine08:15
paulrdhx1: I want to revert the db schema change from 1.308:15
paulrand  scrap that branch personally08:15
dhx1nextgens: there are a number of changes since 1.2.x that will require consideration for heavily customised bug trackers08:15
* nextgens has already hacked up a PBKDF2 storage of the passwords on top of 1.2.1108:16
dhx1nextgens: did you happen to create a git patchset?08:16
nextgensno, but I guess I could diff it easily08:16
nextgensthe patch is small08:16
paulrdhx1: we dont really want that if we gonna do auth plugins - just makes it harder later on08:16
nextgensthere's always a tradeoff in between what you can have now and what a nice design would be08:17
dhx1paulr: I'm still interested... and we can't wait forever for non-existent code08:17
nextgensdhx1> let's talk about #11296 a bit08:18
dhx1nextgens: sure08:18
paulrdhx1: nod08:18
nextgenswhere did you guys heard that sessions should have unlimited lifetime?08:18
nextgensI mean, I understand the usecase, writing a comment on Friday, posting it on Monday and expecting it to work08:19
dhx1nextgens: it is legacy code from 10+ years ago. I agree that sessions need to be time limited.08:19
nextgensbut that's not related to the session08:19
dhx1nextgens: one of the things I wanted to achieve with a future version of MantisBT is proper re-authentication on demand (without losing submitted forms)08:19
dhx1nextgens: we already have issues with CSRF token expiry (which is amusingly much shorter than the session expiry)08:20
nextgensif your session cookie had a lifetime of 30mins, I wouldn't argue about the practicality of a session-prediction attack; but that's not the case08:20
nextgensyeah, it's the same thing with CSRF08:21
dhx1we even generate a different CSRF nonce for _every_ single form or "action hyperlink"... a massively redundant overhead08:21
nextgensdhx1> well, arguably I'd argue that CSRF tokens shouldn't expire08:21
dhx1nextgens: yep, it needs rewriting too08:22
nextgensthey should be linked to the session but shouldn't expire08:22
nextgensie: if the session is gone then yeah, the become invalid08:22
nextgens*they08:22
dhx1nextgens: that is actually what already happens08:23
dhx1nextgens: MantisBT uses PHP session cookies _and_ the broken internal session IDs stored in MantisBT's database08:24
dhx1nextgens: CSRF tokens are linked to PHP sessions and will expire when the PHP session expires08:24
dhx1nextgens: but the MantisBT authentication session will still remain open :(08:24
nextgensI'd suggest to solve one problem at the time08:25
nextgens1) ensure the cookie is actually random; stop the 'might not be unique' mess08:25
nextgensuse uniq_id() or something08:25
nextgensstop hashing crap, assuming it will give you more entropy; it doesn't08:26
paulr[which is alredy done for next main release]08:26
nextgens2) re-consider using the native php session management stuff - with one session per browser)08:26
nextgenspaulr> what's the ETA for that main release?08:28
nextgensin any case, if such vulnerabilities are known, they should be documented08:28
dhx1nextgens: I recommend using it now. The only reason it hasn't been released is that the translation string format changed and no one bothered updating our translation system (TranslateWiki)08:28
nextgensas it will 'help' to convince people to upgrade08:28
dhx1nextgens: agreed08:28
nextgenslet me know if I can help with that; I work for a security consultancy company, we can release an advisory and get a CVE if that helps08:29
* nextgens will grab the code of 1.3 and have a look08:30
dhx1(1) is mostly fixed in master-1.3.x by use of OpenSSL's PRNG (no useless hashing crap either)08:30
nextgensI've got two hours to kill on a plane later today08:30
dhx1I say "mostly" because we still do naive comparisons using PHP's == or === equality mechanism... which is wide open to timing analysis attacks (how practical they would be is another matter)08:31
dhx1(2) definitely needs correcting and hasn't been done yet in 1.3.x08:31
nextgenshmm08:32
dhx1I'd be happy to see an advisory, if warranted. I assume you saw the recent one I sent to oss-security?08:32
nextgens2) re-consider using the native php session management stuff - with one session per browser)http://sourceforge.net/projects/mantisbt/files/mantis-development/08:32
nextgensthere's no 1.3 there08:32
nextgens(and that's what's linked on http://mantisbt.org/download.php)08:33
* nextgens looks at the nightly stuff now08:33
nextgensok, found it08:33
dhx1nextgens: I suggest checking out the "master" branch from git://github.com/mantisbt/mantisbt.git08:34
paulralso have a look at https://github.com/grangeway/mantisbt08:34
paulrdhx1: did you see that I added phpdoc comments to whole of mantisbt2?08:34
dhx1dhx1: that way you can create your own fork to store and manage local changes08:34
paulrdo you always talk to yourself? :)08:34
dhx1paulr: not yet sorry, will check and talk to you about it soon :)08:35
dhx1nextgens: I banned inline JavaScript and cross-site content loading in the master branch... so you get the advantage of having the fallback of X-Content-Security-Policy to protect some users from XSS attacks08:36
dhx1nextgens: see http://www.mantisbt.org/bugs/view.php?id=14088 for outstanding issues blocking the 1.3.x release08:37
nextgensdhx1> if I merge my PBKDF2 patch to tip and provide a diff, would you merge it?08:37
dhx1nextgens: I agree with the principle of using PBKDF2 and assume the code is going to be good quality, so yes... for 1.3.x08:38
dhx1nextgens: distributions don't like us making large changes to stable branches08:38
dhx1nextgens: our real problem is that 1.3.x should have been released 6+ months ago08:38
paulri'd like to scrap 1.3 and release 2.0 in august08:38
paulr;)08:38
nextgensmy current version uses PBKDF2/sha256 but truncates the output to the size of the md5sum (didn't want to change the schema)08:39
nextgensand the number of rounds depends on the year08:39
nextgensie: the number of rounds will increase automatically over time08:40
dhx1nextgens: how are existing passwords upgraded? first time login after the patch will rewrite the password in the database?08:40
nextgensusers login in with an old type of hash will see their hash updated to the new format08:40
nextgensyeah08:40
nextgensthe catch is that users won't login until a while08:41
nextgensthanks to the never-expirating sessions08:41
dhx1hmmm :(08:41
nextgensbut heh, here I just drop the sessions when I apply the patch08:41
nextgenspaulr> the auth_generate_random_password() from 1.3 is as bad as in 1.2.1108:43
nextgensespecially on 32bits08:43
nextgenswhere it might overflow08:43
nextgenswell, even if it doesn't08:43
nextgensyou really have only 2^31 possibilities best case scenario08:44
nextgensin practice much less08:44
dhx1with 1.3.x?08:44
* nextgens checks which branch he's on08:44
nextgensI just checked out what's in git08:45
dhx1https://github.com/mantisbt/mantisbt/blob/master/core/authentication_api.php#L48008:45
dhx1git checkout master08:45
nextgenshmm, master-1.2.x08:45
dhx1ah08:45
nextgensremotes/origin/master ?08:45
nextgensor remotes/origin/next ?08:45
dhx1master08:46
dhx1https://github.com/mantisbt/mantisbt/blob/master/core/crypto_api.php#L16908:46
nextgenshmm, got it08:47
nextgenshmm, that's convoluted.08:47
dhx1nextgens: note the crap about Windows not supporting OpenSSL's PRNG no longer applies to PHP 5.408:48
nextgensI'd just hash the output of the PRNG if I were you08:48
nextgensinstead of doing some magic with base6408:48
dhx1nextgens: It's just a generic function to produce a given amount of entropy (encoded using a URI-safe version of base64)08:48
nextgensyeah, but it's used in functions like auth_generate_random_password()08:49
dhx1nextgens: you are probably after the crypto_generate_random_string function: https://github.com/mantisbt/mantisbt/blob/master/core/crypto_api.php#L6908:49
dhx1nextgens: the idea was to pack in as much entropy as possible into nonces while minimising the length of the nonce (expand the character set)08:50
nextgensI understand the idea, I'm just saying that it's error prone08:50
nextgensand won't help you to verify stuff in constant time08:51
nextgensit's just easier when what you're comparing is the same size08:51
nextgensit also makes it easier to validate input08:51
dhx1nextgens: the base64 encoding around the outside won't affect security because the length of the desired nonce is not secret08:52
nextgensdhx1> the padding is not08:52
nextgenswell, whatever, it's not important anyway :)08:53
nextgenswill have a better look at it later; need to go pack now08:53
nextgensbbl08:53
nextgenswas nice to talk to you guys08:53
dhx1nextgens: within base64_encode, perhaps... but we counter that by ensuring the function will never need to pad anything :)08:54
dhx1nextgens: likewise, and thanks for motivating me to do some work on MantisBT :)08:54
dhx1nextgens: hope you have a good trip08:54
*** Joins: GitHub24 (~GitHub24@sh2.rs.github.com)08:54
GitHub24[mantisbt] grangeway pushed 2 new commits to master-2.0.x: http://git.io/Dwk9Zw08:54
GitHub24[mantisbt/master-2.0.x] Remove unused bugnote text id - Paul Richards08:54
GitHub24[mantisbt/master-2.0.x] remove todo item - this should stay in cf api - Paul Richards08:54
*** Parts: GitHub24 (~GitHub24@sh2.rs.github.com) ()08:54
dhx1paulr: now to see what you've done :P08:55
paulrwha?08:55
paulri'm thinking aim for mid august for 2.008:55
paulrthoughts?08:55
dhx1oooo :o08:56
dhx1far too soon08:56
paulrwell alpha08:56
dhx1based on current progress08:56
dhx1your branch (and 'next') both use translation file formats that don't work with the existing TranslateWiki MantisBT interface08:56
dhx1there is a fair amount of work involved in fixing the i18n problem08:57
paulrsiebrand says he can generate files in the format in the 2.0 branch08:57
dhx1paulr: fair enough09:00
dhx1paulr: why call it 2.0 instead of just 1.3?09:00
*** Joins: GitHub114 (~GitHub114@sh2.rs.github.com)09:01
GitHub114[mantisbt] grangeway pushed 1 new commit to master-2.0.x: http://git.io/KWpGLg09:01
GitHub114[mantisbt/master-2.0.x] db_now in database does not require a timezone boolean to be added - we always want to store unix timestamp in the DB - Paul Richards09:01
*** Parts: GitHub114 (~GitHub114@sh2.rs.github.com) ()09:01
paulrdhx1: need to go fix something for someone - will be back in 20 minutes09:02
paulr'2.0' to make it clear internal API is changing09:02
paulre.g. database format is completely changed09:02
dhx1paulr: 1.x releases indicate that too?09:02
dhx1hmmm09:02
dhx1I see09:02
paulri.e. we've got the db_query( select * from {bugs} stuff09:02
paulrso that's gonan mean plugins will need updating09:03
paulrtherefore 2.x09:03
dhx1they already needed updating in 1.3.x :)09:03
paulryou still gona be up for an hour right?09:03
dhx1I thought 1.x.x = minor patches that don't add functionality09:03
paulrbrb09:03
dhx11.x = major incremental changes that may change APIs (at least until we stabilise the code base) and make other drastic changes09:04
dhx11, 2, 3... = completely new architecture, programming language, approach, etc09:04
dhx1not that I think version numbers are overly useful... I prefer to think of software as incremental changes which don't break existing code straight away09:05
dhx1paulr: I don't see the point in having all of that PHPdoc stuff... it seems like unhelpful clutter09:07
dhx1paulr: the variable names in most cases are a good indication of what a variable does09:07
dhx1paulr: function names provide a good indication otherwise09:07
dhx1paulr: I'd prefer to see inline documentation reserved for meaninful use to explain WHY something is the way it is09:08
*** Quits: dhx1 (~anonymous@60-242-247-232.static.tpgi.com.au) (Quit: Leaving)09:57
*** Joins: GitHub174 (~GitHub174@sh3.rs.github.com)10:49
GitHub174[mantisbt] grangeway pushed 1 new commit to master-2.0.x: http://git.io/yJdKXg10:49
GitHub174[mantisbt/master-2.0.x] format confirmation message - Paul Richards10:49
*** Parts: GitHub174 (~GitHub174@sh3.rs.github.com) ()10:49
*** Joins: giallu (~giallu@fedora/giallu)14:24
*** Quits: paulr (~IceChat09@cpc1-enfi15-2-0-cust580.hari.cable.virginmedia.com) (Quit: There's nothing dirtier then a giant ball of oil)18:59
GitHub140[mantisbt] siebrand pushed 1 new commit to master-1.2.x: http://git.io/ZAbFkw19:44
GitHub140[mantisbt/master-1.2.x] Localisation updates from http://translatewiki.net. - Siebrand Mazeland19:44
*** Quits: giallu (~giallu@fedora/giallu) (Ping timeout: 244 seconds)19:54
*** Quits: sdfjkljkdfsljkl (~sdfjkljkd@static.96.23.63.178.clients.your-server.de) (Remote host closed the connection)20:00
*** Joins: sdfjkljkdfsljkl (~sdfjkljkd@static.96.23.63.178.clients.your-server.de)20:00
*** Joins: dhx1 (~anonymous@60-242-247-232.static.tpgi.com.au)21:28

Generated by irclog2html.py 2.10.0 by Marius Gedminas - find it at mg.pov.lt!