*** Joins: nextgens (~florent@freenet/developer/nextgens) | 06:38 | |
nextgens | hi | 06:40 |
---|---|---|
nextgens | how to report a security vulnerability on mantis? | 06:41 |
nextgens | I've tried emailing security@mantisbt.org, but it doesn't exist | 06:41 |
nextgens | should I use the bugtracker and mark the issue private? or use the contact form on the website? | 06:42 |
nextgens | or 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 | |
paulr | lo | 07:46 |
dhx1 | paulr: hey | 07:51 |
dhx1 | PULSEAUDIO GRRRRRR | 07:51 |
dhx1 | it may well be worse than PHP :) | 07:53 |
paulr | heh | 07:56 |
paulr | horrible db queries are horrible | 07:56 |
paulr | :) | 07:56 |
nextgens | can't be worst than mantis' session handling. | 07:56 |
paulr | http://pastebin.com/JnL0Q3cr | 07:57 |
dhx1 | nextgens: haha | 07:57 |
paulr | it can | 07:57 |
dhx1 | nextgens: have you considered the master-1.3.x branch of MantisBT? It uses X-Content-Security-Policy | 07:57 |
paulr | or the 2.x branch | 07:57 |
paulr | heh | 07:57 |
dhx1 | session handling and authentication in MantisBT are 10 years old and desperately need replacement | 07:58 |
nextgens | no; we're running 1.2.11 | 07:58 |
nextgens | http://code.bulix.org/w1h0fu-81634?raw | 07:59 |
dhx1 | nextgens: mantisbt.org/bugs + set the issue as private | 08:00 |
paulr | dare I ask what you found | 08:00 |
dhx1 | nextgens: or let me know directly (d@hx.id.au) and I can have it fixed within a few minutes :) | 08:00 |
nextgens | my issue is related to session handling; it's not far from trivial to do authentication bypass | 08:00 |
nextgens | it's definitely practical | 08:01 |
nextgens | but reading http://www.mantisbt.org/bugs/view.php?id=11296 I suspect it's a known issue | 08:01 |
nextgens | http://code.bulix.org/fqzb4t-81633 | 08:02 |
nextgens | that's how the magic cookie is generated on 1.2.11 | 08:02 |
nextgens | how many bits of entropy do you think there is in the output of auth_generate_cookie_string() ? | 08:02 |
dhx1 | nextgens: for some bizarre reason we still store session information in the database (bad, bad, bad!) | 08:03 |
nextgens | well, I could leave with that | 08:03 |
nextgens | but not with a session with a very low amount of entropy... which is live until you change your password! | 08:03 |
dhx1 | nextgens: the correct approach would be to offload that to PHP's session handling with appropriate expiry | 08:03 |
nextgens | yes | 08:04 |
nextgens | but that's only part of the problem | 08:04 |
nextgens | you also need to change the sesion key when you change priviledges | 08:04 |
nextgens | (ie: are granted/revoked any priviledge) | 08:04 |
paulr | at same time there's 32 bytes of stuff that's arguably known and 32 bytes that's fairly random | 08:04 |
paulr | you'd do well to work that out | 08:04 |
nextgens | and obviously use a different identifier for each 'browser' | 08:05 |
nextgens | paulr> it's a lot less than that in practice | 08:05 |
nextgens | you use mersenne twister | 08:05 |
dhx1 | nextgens: 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, etc | 08:05 |
nextgens | two subsequent calls to mersenne twister | 08:05 |
paulr | dhx1: that'l lbe why I can't find that function anymore :) | 08:05 |
nextgens | which is a very bad idea | 08:05 |
nextgens | and well, a unix timestamp | 08:06 |
nextgens | that's less than 20bits of entropy overall | 08:06 |
paulr | do { | 08:06 |
paulr | $t_cookie_string = crypto_generate_uri_safe_nonce( 64 ); | 08:06 |
paulr | and yea | 08:06 |
nextgens | dhx1> people (like me) run 1.2.11 :p | 08:06 |
paulr | the dhx says, in next main release it completely changes | 08:06 |
paulr | dhx1: go write a CVE :P | 08:06 |
dhx1 | nextgens: 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.x | 08:07 |
nextgens | now, to quantify how much entropy there actually is, it's difficult | 08:07 |
* paulr would like to see 1.3 scrapped | 08:07 | |
nextgens | thanks to php's awkwardness | 08:07 |
nextgens | http://www.suspekt.org/2008/08/17/mt_srand-and-not-so-random-numbers/ | 08:07 |
nextgens | it depends on the version of the interpreter, the platform it runs on, ... whether it runs as mod_php or mod_cgi | 08:08 |
paulr | yep, there's been some bugfixes to that | 08:08 |
paulr | I remember when those first came out as issues | 08:08 |
paulr | was it really 4 years ago? :( | 08:08 |
nextgens | and obviously whether there's another 'oracle' available on the same vhost | 08:08 |
nextgens | but in practice it's a vulnerability in my books | 08:08 |
* paulr nods | 08:09 | |
paulr | I was having the discussion with dhx yesterday about security vulnerability reporting now-a-days and new features | 08:10 |
dhx1 | ah, the terrible days of mod_php (one PHP process with multiple threads) | 08:10 |
paulr | in that, mantis is quite an old platform | 08:11 |
nextgens | imho applications like mantis shouldn't deal with authentication | 08:11 |
paulr | we've been working at changing the db layer for example to something that might be more secure, as well the crypto stuff dhx jut mentioned | 08:11 |
nextgens | they should use a SSO platform and rely on that | 08:11 |
dhx1 | PHP had numerous vulnerabilities arising from an inability to correctly seed the internal PRNG for each thread | 08:11 |
paulr | depends what you have available as a SSO platform | 08:12 |
nextgens | authentication is something silly, but tricky to get right | 08:12 |
* paulr looks at linkedin | 08:12 | |
paulr | yea: ) | 08:12 |
nextgens | whatever; openid, oauth, whatever | 08:12 |
nextgens | CAS, browserid, ... it's not like there's a shortage of solutions | 08:13 |
dhx1 | I 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 style | 08:13 |
nextgens | dhx1> that works for geeks but not for 'lambda' users | 08:13 |
paulr | I think the problem with something like mantis is it's a legacy app | 08:13 |
dhx1 | nextgens: many of these systems are needlessly complex | 08:13 |
paulr | atm, 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 branch | 08:14 |
dhx1 | nextgens: and I am hesitant of authentication/security systems that are more complex than required | 08:14 |
paulr | all 3 of which contain slightly different ideas for the future :) | 08:14 |
* paulr just uses IIS authentication at work | 08:14 | |
nextgens | is any of these branches stable enough to use in production? | 08:15 |
paulr | i'd say no | 08:15 |
nextgens | ^-^ | 08:15 |
dhx1 | nextgens: 1.3.x is fine | 08:15 |
paulr | dhx1: I want to revert the db schema change from 1.3 | 08:15 |
paulr | and scrap that branch personally | 08:15 |
dhx1 | nextgens: there are a number of changes since 1.2.x that will require consideration for heavily customised bug trackers | 08:15 |
* nextgens has already hacked up a PBKDF2 storage of the passwords on top of 1.2.11 | 08:16 | |
dhx1 | nextgens: did you happen to create a git patchset? | 08:16 |
nextgens | no, but I guess I could diff it easily | 08:16 |
nextgens | the patch is small | 08:16 |
paulr | dhx1: we dont really want that if we gonna do auth plugins - just makes it harder later on | 08:16 |
nextgens | there's always a tradeoff in between what you can have now and what a nice design would be | 08:17 |
dhx1 | paulr: I'm still interested... and we can't wait forever for non-existent code | 08:17 |
nextgens | dhx1> let's talk about #11296 a bit | 08:18 |
dhx1 | nextgens: sure | 08:18 |
paulr | dhx1: nod | 08:18 |
nextgens | where did you guys heard that sessions should have unlimited lifetime? | 08:18 |
nextgens | I mean, I understand the usecase, writing a comment on Friday, posting it on Monday and expecting it to work | 08:19 |
dhx1 | nextgens: it is legacy code from 10+ years ago. I agree that sessions need to be time limited. | 08:19 |
nextgens | but that's not related to the session | 08:19 |
dhx1 | nextgens: 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 |
dhx1 | nextgens: we already have issues with CSRF token expiry (which is amusingly much shorter than the session expiry) | 08:20 |
nextgens | if 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 case | 08:20 |
nextgens | yeah, it's the same thing with CSRF | 08:21 |
dhx1 | we even generate a different CSRF nonce for _every_ single form or "action hyperlink"... a massively redundant overhead | 08:21 |
nextgens | dhx1> well, arguably I'd argue that CSRF tokens shouldn't expire | 08:21 |
dhx1 | nextgens: yep, it needs rewriting too | 08:22 |
nextgens | they should be linked to the session but shouldn't expire | 08:22 |
nextgens | ie: if the session is gone then yeah, the become invalid | 08:22 |
nextgens | *they | 08:22 |
dhx1 | nextgens: that is actually what already happens | 08:23 |
dhx1 | nextgens: MantisBT uses PHP session cookies _and_ the broken internal session IDs stored in MantisBT's database | 08:24 |
dhx1 | nextgens: CSRF tokens are linked to PHP sessions and will expire when the PHP session expires | 08:24 |
dhx1 | nextgens: but the MantisBT authentication session will still remain open :( | 08:24 |
nextgens | I'd suggest to solve one problem at the time | 08:25 |
nextgens | 1) ensure the cookie is actually random; stop the 'might not be unique' mess | 08:25 |
nextgens | use uniq_id() or something | 08:25 |
nextgens | stop hashing crap, assuming it will give you more entropy; it doesn't | 08:26 |
paulr | [which is alredy done for next main release] | 08:26 |
nextgens | 2) re-consider using the native php session management stuff - with one session per browser) | 08:26 |
nextgens | paulr> what's the ETA for that main release? | 08:28 |
nextgens | in any case, if such vulnerabilities are known, they should be documented | 08:28 |
dhx1 | nextgens: 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 |
nextgens | as it will 'help' to convince people to upgrade | 08:28 |
dhx1 | nextgens: agreed | 08:28 |
nextgens | let 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 helps | 08:29 |
* nextgens will grab the code of 1.3 and have a look | 08:30 | |
dhx1 | (1) is mostly fixed in master-1.3.x by use of OpenSSL's PRNG (no useless hashing crap either) | 08:30 |
nextgens | I've got two hours to kill on a plane later today | 08:30 |
dhx1 | I 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.x | 08:31 |
nextgens | hmm | 08:32 |
dhx1 | I'd be happy to see an advisory, if warranted. I assume you saw the recent one I sent to oss-security? | 08:32 |
nextgens | 2) re-consider using the native php session management stuff - with one session per browser)http://sourceforge.net/projects/mantisbt/files/mantis-development/ | 08:32 |
nextgens | there's no 1.3 there | 08:32 |
nextgens | (and that's what's linked on http://mantisbt.org/download.php) | 08:33 |
* nextgens looks at the nightly stuff now | 08:33 | |
nextgens | ok, found it | 08:33 |
dhx1 | nextgens: I suggest checking out the "master" branch from git://github.com/mantisbt/mantisbt.git | 08:34 |
paulr | also have a look at https://github.com/grangeway/mantisbt | 08:34 |
paulr | dhx1: did you see that I added phpdoc comments to whole of mantisbt2? | 08:34 |
dhx1 | dhx1: that way you can create your own fork to store and manage local changes | 08:34 |
paulr | do you always talk to yourself? :) | 08:34 |
dhx1 | paulr: not yet sorry, will check and talk to you about it soon :) | 08:35 |
dhx1 | nextgens: 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 attacks | 08:36 |
dhx1 | nextgens: see http://www.mantisbt.org/bugs/view.php?id=14088 for outstanding issues blocking the 1.3.x release | 08:37 |
nextgens | dhx1> if I merge my PBKDF2 patch to tip and provide a diff, would you merge it? | 08:37 |
dhx1 | nextgens: I agree with the principle of using PBKDF2 and assume the code is going to be good quality, so yes... for 1.3.x | 08:38 |
dhx1 | nextgens: distributions don't like us making large changes to stable branches | 08:38 |
dhx1 | nextgens: our real problem is that 1.3.x should have been released 6+ months ago | 08:38 |
paulr | i'd like to scrap 1.3 and release 2.0 in august | 08:38 |
paulr | ;) | 08:38 |
nextgens | my current version uses PBKDF2/sha256 but truncates the output to the size of the md5sum (didn't want to change the schema) | 08:39 |
nextgens | and the number of rounds depends on the year | 08:39 |
nextgens | ie: the number of rounds will increase automatically over time | 08:40 |
dhx1 | nextgens: how are existing passwords upgraded? first time login after the patch will rewrite the password in the database? | 08:40 |
nextgens | users login in with an old type of hash will see their hash updated to the new format | 08:40 |
nextgens | yeah | 08:40 |
nextgens | the catch is that users won't login until a while | 08:41 |
nextgens | thanks to the never-expirating sessions | 08:41 |
dhx1 | hmmm :( | 08:41 |
nextgens | but heh, here I just drop the sessions when I apply the patch | 08:41 |
nextgens | paulr> the auth_generate_random_password() from 1.3 is as bad as in 1.2.11 | 08:43 |
nextgens | especially on 32bits | 08:43 |
nextgens | where it might overflow | 08:43 |
nextgens | well, even if it doesn't | 08:43 |
nextgens | you really have only 2^31 possibilities best case scenario | 08:44 |
nextgens | in practice much less | 08:44 |
dhx1 | with 1.3.x? | 08:44 |
* nextgens checks which branch he's on | 08:44 | |
nextgens | I just checked out what's in git | 08:45 |
dhx1 | https://github.com/mantisbt/mantisbt/blob/master/core/authentication_api.php#L480 | 08:45 |
dhx1 | git checkout master | 08:45 |
nextgens | hmm, master-1.2.x | 08:45 |
dhx1 | ah | 08:45 |
nextgens | remotes/origin/master ? | 08:45 |
nextgens | or remotes/origin/next ? | 08:45 |
dhx1 | master | 08:46 |
dhx1 | https://github.com/mantisbt/mantisbt/blob/master/core/crypto_api.php#L169 | 08:46 |
nextgens | hmm, got it | 08:47 |
nextgens | hmm, that's convoluted. | 08:47 |
dhx1 | nextgens: note the crap about Windows not supporting OpenSSL's PRNG no longer applies to PHP 5.4 | 08:48 |
nextgens | I'd just hash the output of the PRNG if I were you | 08:48 |
nextgens | instead of doing some magic with base64 | 08:48 |
dhx1 | nextgens: It's just a generic function to produce a given amount of entropy (encoded using a URI-safe version of base64) | 08:48 |
nextgens | yeah, but it's used in functions like auth_generate_random_password() | 08:49 |
dhx1 | nextgens: you are probably after the crypto_generate_random_string function: https://github.com/mantisbt/mantisbt/blob/master/core/crypto_api.php#L69 | 08:49 |
dhx1 | nextgens: 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 |
nextgens | I understand the idea, I'm just saying that it's error prone | 08:50 |
nextgens | and won't help you to verify stuff in constant time | 08:51 |
nextgens | it's just easier when what you're comparing is the same size | 08:51 |
nextgens | it also makes it easier to validate input | 08:51 |
dhx1 | nextgens: the base64 encoding around the outside won't affect security because the length of the desired nonce is not secret | 08:52 |
nextgens | dhx1> the padding is not | 08:52 |
nextgens | well, whatever, it's not important anyway :) | 08:53 |
nextgens | will have a better look at it later; need to go pack now | 08:53 |
nextgens | bbl | 08:53 |
nextgens | was nice to talk to you guys | 08:53 |
dhx1 | nextgens: within base64_encode, perhaps... but we counter that by ensuring the function will never need to pad anything :) | 08:54 |
dhx1 | nextgens: likewise, and thanks for motivating me to do some work on MantisBT :) | 08:54 |
dhx1 | nextgens: hope you have a good trip | 08: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/Dwk9Zw | 08:54 |
GitHub24 | [mantisbt/master-2.0.x] Remove unused bugnote text id - Paul Richards | 08:54 |
GitHub24 | [mantisbt/master-2.0.x] remove todo item - this should stay in cf api - Paul Richards | 08:54 |
*** Parts: GitHub24 (~GitHub24@sh2.rs.github.com) () | 08:54 | |
dhx1 | paulr: now to see what you've done :P | 08:55 |
paulr | wha? | 08:55 |
paulr | i'm thinking aim for mid august for 2.0 | 08:55 |
paulr | thoughts? | 08:55 |
dhx1 | oooo :o | 08:56 |
dhx1 | far too soon | 08:56 |
paulr | well alpha | 08:56 |
dhx1 | based on current progress | 08:56 |
dhx1 | your branch (and 'next') both use translation file formats that don't work with the existing TranslateWiki MantisBT interface | 08:56 |
dhx1 | there is a fair amount of work involved in fixing the i18n problem | 08:57 |
paulr | siebrand says he can generate files in the format in the 2.0 branch | 08:57 |
dhx1 | paulr: fair enough | 09:00 |
dhx1 | paulr: 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/KWpGLg | 09: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 Richards | 09:01 |
*** Parts: GitHub114 (~GitHub114@sh2.rs.github.com) () | 09:01 | |
paulr | dhx1: need to go fix something for someone - will be back in 20 minutes | 09:02 |
paulr | '2.0' to make it clear internal API is changing | 09:02 |
paulr | e.g. database format is completely changed | 09:02 |
dhx1 | paulr: 1.x releases indicate that too? | 09:02 |
dhx1 | hmmm | 09:02 |
dhx1 | I see | 09:02 |
paulr | i.e. we've got the db_query( select * from {bugs} stuff | 09:02 |
paulr | so that's gonan mean plugins will need updating | 09:03 |
paulr | therefore 2.x | 09:03 |
dhx1 | they already needed updating in 1.3.x :) | 09:03 |
paulr | you still gona be up for an hour right? | 09:03 |
dhx1 | I thought 1.x.x = minor patches that don't add functionality | 09:03 |
paulr | brb | 09:03 |
dhx1 | 1.x = major incremental changes that may change APIs (at least until we stabilise the code base) and make other drastic changes | 09:04 |
dhx1 | 1, 2, 3... = completely new architecture, programming language, approach, etc | 09:04 |
dhx1 | not that I think version numbers are overly useful... I prefer to think of software as incremental changes which don't break existing code straight away | 09:05 |
dhx1 | paulr: I don't see the point in having all of that PHPdoc stuff... it seems like unhelpful clutter | 09:07 |
dhx1 | paulr: the variable names in most cases are a good indication of what a variable does | 09:07 |
dhx1 | paulr: function names provide a good indication otherwise | 09:07 |
dhx1 | paulr: I'd prefer to see inline documentation reserved for meaninful use to explain WHY something is the way it is | 09: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/yJdKXg | 10:49 |
GitHub174 | [mantisbt/master-2.0.x] format confirmation message - Paul Richards | 10: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/ZAbFkw | 19:44 |
GitHub140 | [mantisbt/master-1.2.x] Localisation updates from http://translatewiki.net. - Siebrand Mazeland | 19: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!