| *** nkinder has joined #openstack-security | 00:09 | |
| *** fletcher has joined #openstack-security | 00:13 | |
| *** pdesai has quit IRC | 00:16 | |
| *** voodookid has quit IRC | 00:18 | |
| *** tmcpeak has joined #openstack-security | 00:18 | |
| *** tmcpeak has quit IRC | 00:20 | |
| *** markvoelker has quit IRC | 00:26 | |
| *** pdesai has joined #openstack-security | 00:28 | |
| *** pdesai has quit IRC | 00:28 | |
| *** bdpayne has quit IRC | 00:34 | |
| *** tmcpeak has joined #openstack-security | 00:40 | |
| *** tmcpeak has quit IRC | 00:42 | |
| *** ljfisher has quit IRC | 00:56 | |
| *** markvoelker has joined #openstack-security | 01:09 | |
| *** markvoelker has quit IRC | 01:14 | |
| *** salv-orlando has quit IRC | 01:20 | |
| *** jamielennox is now known as jamielennox|away | 01:59 | |
| *** browne has quit IRC | 02:08 | |
| *** bpokorny_ has quit IRC | 02:09 | |
| *** bpokorny has joined #openstack-security | 02:09 | |
| *** markvoelker has joined #openstack-security | 02:10 | |
| *** bpokorny has quit IRC | 02:14 | |
| *** markvoelker has quit IRC | 02:15 | |
| *** salv-orlando has joined #openstack-security | 02:20 | |
| *** markvoelker has joined #openstack-security | 02:27 | |
| *** markvoelker has quit IRC | 02:33 | |
| *** jamielennox|away is now known as jamielennox | 02:37 | |
| *** jamielennox is now known as jamielennox|away | 02:52 | |
| *** jamielennox|away is now known as jamielennox | 02:55 | |
| *** jamielennox is now known as jamielennox|away | 02:56 | |
| *** jamielennox|away is now known as jamielennox | 02:57 | |
| *** fletcher has quit IRC | 03:05 | |
| *** amrith is now known as _amrith_ | 03:11 | |
| *** browne has joined #openstack-security | 03:12 | |
| *** salv-orlando has quit IRC | 03:21 | |
| *** markvoelker has joined #openstack-security | 03:28 | |
| *** _amrith_ is now known as amrith | 03:28 | |
| *** markvoelker has quit IRC | 03:33 | |
| *** markvoelker has joined #openstack-security | 04:29 | |
| *** mgagne has quit IRC | 04:31 | |
| *** markvoelker has quit IRC | 04:34 | |
| *** mgagne has joined #openstack-security | 04:45 | |
| *** mgagne is now known as Guest75711 | 04:45 | |
| *** openstackgerrit has quit IRC | 04:46 | |
| *** openstackgerrit has joined #openstack-security | 04:52 | |
| *** markvoelker has joined #openstack-security | 05:31 | |
| *** jamielennox is now known as jamielennox|away | 05:32 | |
| *** openstack has joined #openstack-security | 05:36 | |
| -sendak.freenode.net- [freenode-info] channel trolls and no channel staff around to help? please check with freenode support: http://freenode.net/faq.shtml#gettinghelp | 05:36 | |
| *** markvoelker has quit IRC | 05:36 | |
| *** salv-orlando has joined #openstack-security | 05:38 | |
| openstackgerrit | Leon Zachery proposed openstack/security-doc: Add reference links to Openstack Security Guide https://review.openstack.org/160868 | 06:13 |
|---|---|---|
| *** salv-orlando has quit IRC | 06:29 | |
| *** markvoelker has joined #openstack-security | 06:32 | |
| *** markvoelker has quit IRC | 06:38 | |
| *** markvoelker has joined #openstack-security | 07:34 | |
| *** markvoelker has quit IRC | 07:39 | |
| *** openstackgerrit has quit IRC | 07:49 | |
| *** openstackgerrit has joined #openstack-security | 07:49 | |
| *** salv-orlando has joined #openstack-security | 08:15 | |
| *** markvoelker has joined #openstack-security | 08:35 | |
| *** markvoelker has quit IRC | 08:39 | |
| *** salv-orlando has quit IRC | 08:45 | |
| *** salv-orlando has joined #openstack-security | 08:45 | |
| *** openstack has joined #openstack-security | 15:28 | |
| *** amrith is now known as _amrith_ | 15:44 | |
| *** dwyde has joined #openstack-security | 15:54 | |
| openstackgerrit | Robert Clark proposed stackforge/anchor: Multiple changes so that Anchor runs correctly https://review.openstack.org/161149 | 16:20 |
| openstackgerrit | Tim Kelsey proposed stackforge/anchor: Fixing several issues in Anchor startup https://review.openstack.org/161301 | 16:32 |
| *** _amrith_ is now known as amrith | 16:44 | |
| *** bpokorny has joined #openstack-security | 16:45 | |
| *** tmcpeak has quit IRC | 17:06 | |
| *** amrith is now known as _amrith_ | 17:12 | |
| *** tmcpeak has joined #openstack-security | 17:16 | |
| *** fletcher has joined #openstack-security | 17:25 | |
| fletcher | tmcpeak: heyo, around? | 17:26 |
| tmcpeak | fletcher: was just looking for you | 17:29 |
| tmcpeak | you have your Bandit instance handy? | 17:29 |
| fletcher | boom, unfortunately I have to bounce in about 30 minutes but I think that should be enough time | 17:29 |
| fletcher | I do, yah | 17:29 |
| tmcpeak | yeah, that will be plenty | 17:29 |
| tmcpeak | can you clone this, run Bandit against it and read the results: https://github.com/openstack/trove.git | 17:30 |
| *** edmondsw has quit IRC | 17:30 | |
| *** ukbelch has joined #openstack-security | 17:30 | |
| ukbelch | hullo thar | 17:30 |
| fletcher | with master I assume? | 17:30 |
| tmcpeak | ukbelch: you'll be interested in this as well :) | 17:30 |
| tmcpeak | shouldn't matter | 17:31 |
| ukbelch | that's why im here! | 17:31 |
| tmcpeak | but for carity lett's use belch's checkin | 17:31 |
| tmcpeak | *clarity | 17:31 |
| tmcpeak | actually yeah, use Belch's check-in so we're all dealing with the same thing | 17:31 |
| ukbelch | what's been discussed already | 17:31 |
| fletcher | well master has full line ranges right now, was the only thing Iw as thinking | 17:31 |
| fletcher | not much, we just started ukbelch | 17:31 |
| tmcpeak | ukbelch: nothing, just asked him to clone Trove and run | 17:31 |
| tmcpeak | ok, anything with full line ranges should be fine | 17:32 |
| ukbelch | ok, so what's the use case for needing all line numbers for a given statement output? | 17:32 |
| *** rkgudboy has joined #openstack-security | 17:32 | |
| tmcpeak | well before we get to that, I wanted to show the issue we're having with it | 17:33 |
| ukbelch | sure | 17:33 |
| fletcher | ok, it's running now | 17:33 |
| ukbelch | menewhile, I can fix the json output issue, I figured there may be one there (hence the specific comment asking for the check :)) | 17:33 |
| fletcher | The use case is comparing against diffs in CI | 17:33 |
| tmcpeak | interesting- ukbelch: I'm not seeing that issue on master | 17:34 |
| fletcher | So for example, the example file I put in the comment | 17:34 |
| tmcpeak | is this something new your check-in introduced? | 17:34 |
| fletcher | the popen spans 5,6,10 | 17:34 |
| tmcpeak | ok let me pull that up | 17:34 |
| fletcher | if somebody checks in something on line 10, that changes shell=True, i want to be able to catch that | 17:34 |
| ukbelch | yes, I refactored how the reporting worked to remove tuples | 17:34 |
| fletcher | just having '5' doesn't help me | 17:34 |
| tmcpeak | right, yeah, I agree | 17:35 |
| ukbelch | ahh, I get you | 17:35 |
| tmcpeak | in that case 5 isn't accurate, the issue spans the three lines | 17:35 |
| fletcher | right, yah | 17:35 |
| tmcpeak | popen by itself isn't an issue | 17:35 |
| ukbelch | so, the issue there is because it's not the line itself getting checked, it's the function call | 17:35 |
| tmcpeak | ukbelch: I'm not seeing that big ugly thing in trove output with amster | 17:36 |
| tmcpeak | master | 17:36 |
| ukbelch | when the visitor functionality sees the popen, the tester is called, which checks the parameters being passed | 17:36 |
| tmcpeak | right | 17:36 |
| ukbelch | so it records the last line visited before the test is called, which is the popen line | 17:36 |
| tmcpeak | yeah, current master output is more accurate though in that it shows all 3 lines | 17:37 |
| fletcher | Is there a specific file in this trove repo we can focus on that has the issue you guys were worried about? I'm getting a ton of output :/ | 17:37 |
| tmcpeak | yeah, hang on, I'll dig it up | 17:37 |
| ukbelch | it would require a modification of the test itself to change that behaviour, sadly | 17:37 |
| fletcher | I really think the original behavior of line ranges was more accurate | 17:37 |
| ukbelch | the code output should show all three lines | 17:37 |
| fletcher | but i'll run it against this file and see what master outputs | 17:37 |
| tmcpeak | fletcher: https://github.com/openstack/trove/blob/master/trove/common/cfg.py#L33 | 17:38 |
| *** rkgudboy has quit IRC | 17:38 | |
| ukbelch | what I do notice is that the json output doesn't show line numbers in it's code output | 17:38 |
| fletcher | Yah, that was purposeful but I'm also not opposed to adding themn | 17:39 |
| fletcher | so i see the ouput from cfg.py | 17:39 |
| ukbelch | which means there's nothing to show all lines touched by that statement (especially as the newlines are compressed) | 17:39 |
| fletcher | I don't see any long line number lists | 17:39 |
| fletcher | huh? | 17:40 |
| tmcpeak | yeah, ukbelch: this new output artifact seems to be related to your check in, old line number solution doesn't suffer from that | 17:40 |
| tmcpeak | sadly this new statement processing logic is introducing this large statement issue | 17:41 |
| ukbelch | looking at your example output fletcher. it's only showing one newline between the first and second parameter to Popen | 17:41 |
| tmcpeak | fletcher: git fetch his patch and run against Trove and you'll see the issue | 17:41 |
| ukbelch | of course it is travis, that's the point :) | 17:41 |
| tmcpeak | yeah, old output was better though :/ | 17:42 |
| ukbelch | but yeah, need to pull my patch to see the extended output | 17:42 |
| fletcher | hmm, ukbelch, i think its just hiding in the center | 17:42 |
| fletcher | "subprocess.Popen([ 'ls',\n'-l'],\n shell=True)\n" | 17:42 |
| fletcher | i see 3 \n | 17:42 |
| ukbelch | yes, but if the lines are 5, 6 and 10, shouldn't there be 5 \n? | 17:42 |
| fletcher | No, it doesn't include blank lines | 17:43 |
| fletcher | other wise line range would be '5,6,7,8,9,10' | 17:43 |
| ukbelch | which I agree with, if it displayes line numbers for all code output | 17:43 |
| ukbelch | otherwise, actually mapping that to diffs or the actual code file could get confusing | 17:43 |
| ukbelch | but, that's a small issue and a matter of taste | 17:44 |
| fletcher | hmm, so I'm totally fine with prepending the lines of code with the line number | 17:44 |
| tmcpeak | yeah, so fletcher: with the JSON diff thing, if somebody adds a comment, all line numbers are going to get shifted | 17:44 |
| tmcpeak | this is a known problem, right? | 17:44 |
| ukbelch | it's easy enough to include the full line range in the json line_nums output | 17:44 |
| fletcher | I don't think it makes mapping to diffs harder. You just get "line touched" from the diff and do a "is this in line_range_list" of vuln | 17:44 |
| ukbelch | if that's the desired output | 17:44 |
| *** browne has quit IRC | 17:45 | |
| tmcpeak | I think I'm somewhat confused but that's par for the course :D | 17:45 |
| tmcpeak | so belch, this latest check-in with statement processing introduces a new problem with large statements we didn't used to have | 17:45 |
| fletcher | tmcpeak: Yah, it's just getting line numbers from the AST | 17:45 |
| fletcher | (obviously, not being pedantic) | 17:45 |
| tmcpeak | right | 17:46 |
| ukbelch | what new problem travis? | 17:46 |
| tmcpeak | with the disgusting long statement output in Trove | 17:46 |
| ukbelch | give me a sec to run it again with master, but ultimately it's rather fixing a problem in master, in that it's not printing the full context of some statements | 17:47 |
| tmcpeak | ukbelch: can you provide an example of that please? | 17:47 |
| ukbelch | the big messy output :) | 17:47 |
| tmcpeak | well I'd argue we don't want that | 17:48 |
| tmcpeak | can you show an example where the master output is less desirable than the new output? | 17:48 |
| ukbelch | give me a second | 17:48 |
| ukbelch | I should be able to find a few | 17:49 |
| ukbelch | the only case it currently works perfectly for, is function calls | 17:49 |
| ukbelch | >> Probable insecure usage of temp file/directory - ../trove/trove/tests/unittests/guestagent/test_dbaas.py::[125] | 17:49 |
| ukbelch | 125 output = "mysqld would've been started with the these args:\n"\ | 17:49 |
| ukbelch | incorrect, as the issue isn't on that specific line, there's no temp directory here | 17:49 |
| ukbelch | >> Probable insecure usage of temp file/directory - ../trove/trove/tests/unittests/guestagent/test_backups.py::[364] | 17:50 |
| ukbelch | 364 restore_location='/tmp/somewhere') | 17:50 |
| tmcpeak | sure, doesn't include the full context | 17:50 |
| ukbelch | incorrect, as we don't know what the rest of the statement preceeding line 364 is | 17:50 |
| ukbelch | >> Probable insecure usage of temp file/directory - ../trove/trove/tests/unittests/guestagent/test_backups.py::[295] | 17:50 |
| ukbelch | 295 restr = RunnerClass(None, restore_location="/tmp", | 17:50 |
| ukbelch | incorrect, it's missing the rest of the parameters going into RunnerClass | 17:50 |
| ukbelch | >> Probable insecure usage of temp file/directory - ../trove/trove/tests/unittests/guestagent/test_backups.py::[69] | 17:51 |
| ukbelch | 69PREPARE = ("sudo innobackupex --apply-log /var/lib/mysql " | 17:51 |
| ukbelch | incorrect - isn't showing the temp directory part of the statement at issue | 17:51 |
| ukbelch | shoudl I go on? | 17:51 |
| fletcher | ukbelch: looking at files now to gain context | 17:52 |
| ukbelch | the whole point is, we had to add a fixed "n" parameter to output more lines, so we could see all the context of statements spread over multiple lines. The fix changes that need. | 17:52 |
| ukbelch | spitting out the entire mess of crappy config code is, as I see it, intended behaviour. Specifying the actual line number affected, thus being able to see which line in that statement is at fault, is therefore necessary | 17:53 |
| ukbelch | the question is, if we can modify the tests so it outputs the correct line in the case of function calls | 17:54 |
| tmcpeak | is there any way to have the best of both worlds? Since function tests output worked perfectly, leave the output as is for that, but in other contexts (like string) output the whole statement? | 17:54 |
| tmcpeak | yeah | 17:54 |
| ukbelch | no, we can't really | 17:54 |
| ukbelch | that's not how the reporting works. We can't pick and choose based on the statment type | 17:55 |
| ukbelch | it would be a minefield | 17:55 |
| fletcher | In the end, more information is almost always better | 17:55 |
| tmcpeak | yeah.. I also agree that the arbitrary guess n lines around context solution was hacky | 17:55 |
| fletcher | But all that did was print code | 17:55 |
| tmcpeak | fletcher: yeah, the problem is that in Trove they define this like 60 line list | 17:55 |
| fletcher | it didn't add those lines into any of the logic | 17:55 |
| tmcpeak | so the output is ridiculous | 17:55 |
| ukbelch | so, we can look to resolve two things. We can fix up the specific tests, and we can decide exactly what we want in the json output | 17:55 |
| fletcher | right, that seems like a bit of a corner case | 17:55 |
| tmcpeak | and the current result shows the problem is in all of those lines | 17:55 |
| fletcher | Ah ok, so I get it | 17:56 |
| tmcpeak | fletcher: it is a corner case, but I'm concerned with the output being inaccurate | 17:56 |
| tmcpeak | it's saying there is an issue with all of those lines, when there really isn't | 17:56 |
| ukbelch | exactly, that config crap is very much a corner case | 17:56 |
| ukbelch | and it's entirely accurate | 17:56 |
| fletcher | A developer has to figure out which of 60 lines is causing teh erorr | 17:56 |
| ukbelch | your problem is, it's verbose | 17:56 |
| tmcpeak | exactly | 17:56 |
| ukbelch | that's why we output the specific line number Fletcher :) | 17:57 |
| tmcpeak | well, we could do statement line and issue line | 17:57 |
| fletcher | so can we have two fields | 17:57 |
| tmcpeak | output both | 17:57 |
| tmcpeak | yeah | 17:57 |
| ukbelch | he sees a block of code, see's line xx is the issue, and because we print line numbers they can hop straight to it | 17:57 |
| fletcher | "exact line number" "context range" | 17:57 |
| fletcher | or something | 17:57 |
| tmcpeak | yeah, that seems like it makes both cases happy | 17:57 |
| fletcher | right, which is fine for manul case | 17:57 |
| fletcher | but problematic for automating | 17:57 |
| ukbelch | we can have that for json, sure, if that's what we want. That's easy | 17:57 |
| fletcher | it's pointless to have json output if I have to reparse a string to pull out line numnbers | 17:58 |
| tmcpeak | no, just have them stored as a separate field as you said | 17:58 |
| tmcpeak | ignore the field you don't care about | 17:58 |
| fletcher | yah, i agere with that | 17:58 |
| *** ukbelch_ has joined #openstack-security | 17:58 | |
| ukbelch_ | for terminal output, outputting a huge list of numbers is entirely useless | 17:58 |
| ukbelch_ | and confusing/distracting | 17:59 |
| tmcpeak | I also agree with that | 17:59 |
| fletcher | that interests me, does that mean the Shell=True test will actually flag line 10 and return 5,6,10 - that would be awesome | 17:59 |
| fletcher | i agree with that terminal output | 17:59 |
| ukbelch_ | potentially. I need to check if changing the context object within a test will affect the test output, but if it does it just means fixing the line number within the test itself | 18:00 |
| ukbelch_ | right now we have two line related objects in the context. We have context['lineno'] and context['statement']['lineno'] | 18:01 |
| ukbelch_ | sorry, thats context['statement']['linerange'] | 18:01 |
| fletcher | I've unfortunately got to leave for a few hours, but I'll be back on in a bit to discuss more. If we can ensure JSON output has [5,6,10] list for the pOpen warning I provided, I'll be happy :) fwiw | 18:01 |
| tmcpeak | ok cool | 18:01 |
| tmcpeak | later fletcher | 18:01 |
| ukbelch_ | we can do that as an additional parameter, or replace the single number | 18:01 |
| ukbelch_ | what would be your preference? | 18:01 |
| tmcpeak | additional param is my preference | 18:02 |
| tmcpeak | additional param which doesn't get output on terminal | 18:02 |
| *** ukbelch has quit IRC | 18:02 | |
| ukbelch_ | completely separate things :) | 18:02 |
| ukbelch_ | we store both already, and both are used in the reporting already (the range is used to pull the code lines, the specific number output to highlight the issue) | 18:03 |
| ukbelch_ | it's just a case of sticking both in the json output :) | 18:03 |
| tmcpeak | ok cool | 18:03 |
| ukbelch_ | it's fletcher's use case, so I'll add it as an extra parameter for now, and he can comment when he gets back | 18:04 |
| tmcpeak | sounds good | 18:05 |
| *** bdpayne has joined #openstack-security | 18:08 | |
| *** ukbelch_ has quit IRC | 18:11 | |
| *** dwyde has quit IRC | 18:11 | |
| *** ukbelch has joined #openstack-security | 18:11 | |
| *** ukbelch has quit IRC | 18:13 | |
| *** ukbelch has joined #openstack-security | 18:16 | |
| *** browne has joined #openstack-security | 18:17 | |
| *** logan2 has quit IRC | 18:18 | |
| *** ukbelch has quit IRC | 18:21 | |
| *** ukbelch has joined #openstack-security | 18:21 | |
| *** pdesai has joined #openstack-security | 18:45 | |
| *** pdesai has quit IRC | 18:54 | |
| *** _amrith_ is now known as amrith | 19:07 | |
| *** pdesai has joined #openstack-security | 19:32 | |
| *** pdesai has quit IRC | 19:35 | |
| *** dwyde has joined #openstack-security | 19:41 | |
| *** dave-mccowan has joined #openstack-security | 20:01 | |
| openstackgerrit | bruce-benjamin proposed openstack/security-doc: Added input about volume encryption feature https://review.openstack.org/161012 | 20:11 |
| *** pdesai has joined #openstack-security | 20:21 | |
| openstackgerrit | bruce-benjamin proposed openstack/security-doc: Added input about volume encryption feature https://review.openstack.org/161012 | 20:24 |
| openstackgerrit | Dave Belcher proposed stackforge/bandit: Refactored AST processing https://review.openstack.org/160166 | 20:27 |
| *** pdesai has left #openstack-security | 20:46 | |
| *** amrith is now known as _amrith_ | 21:11 | |
| *** ukbelch has quit IRC | 21:22 | |
| *** ukbelch has joined #openstack-security | 21:42 | |
| *** ukbelch has quit IRC | 21:44 | |
| *** ukbelch has joined #openstack-security | 21:47 | |
| *** tmcpeak has quit IRC | 22:00 | |
| *** tmcpeak has joined #openstack-security | 22:13 | |
| *** jamielennox|away is now known as jamielennox | 22:14 | |
| *** tmcpeak has quit IRC | 22:15 | |
| *** _amrith_ is now known as amrith | 22:22 | |
| *** micael has joined #openstack-security | 22:23 | |
| *** micael has quit IRC | 22:26 | |
| *** ukbelch has quit IRC | 22:26 | |
| *** ukbelch has joined #openstack-security | 22:26 | |
| *** dwyde has quit IRC | 22:36 | |
| *** dwyde has joined #openstack-security | 22:50 | |
| *** bknudson has quit IRC | 22:50 | |
| *** browne has quit IRC | 22:55 | |
| *** browne has joined #openstack-security | 22:58 | |
| *** ljfisher has quit IRC | 23:21 | |
| ukbelch | exit | 23:36 |
| *** ukbelch has quit IRC | 23:36 | |
| *** bknudson has joined #openstack-security | 23:52 | |
| openstackgerrit | David Wyde proposed stackforge/bandit: Refactor functional tests to clarify scoring https://review.openstack.org/161005 | 23:53 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!