[WIP] Add missing support for reports by gschneider-r7 · Pull Request #37 · rapid7/nexpose-client-python

gschneider-r7 · 2017-10-09T20:45:53Z

Description

Add support for loading and saving report configs.

Motivation and Context

See #12 and #27

How Has This Been Tested?

Added steps to run_demo.py to create, load, and then delete a report configuration.

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

I have updated the documentation accordingly (if changes are required).
I have added tests to cover my changes.
All new and existing tests passed.

jkee-r7 · 2017-10-11T14:50:07Z

+        - vuln-status
+        - cyberscope-component
+        - cyberscope-bureau
+        - cyberscope-enclave


Does this work for SQL reports?
I added Adhoc Reports to the Nexpose Komand plugin, and I needed to add version and query to this list.

Edit, I see that this list is not enforced, so it should be okay. Maybe just update the comment? 😄

Yep just need to update the comment here.

jkee-r7 · 2017-10-11T15:10:37Z

The generate report request times out after 60 seconds, so fails for larger reports.
I assume that adhoc report will be affected by the same bug?

I know this bug is not the purpose of this PR and is for later work, I just want to highlight that it may be a problem for Generate Adhoc Report.

ghost · 2017-10-11T16:35:26Z

+    report.add_filter('scan', 'last')  # this should use site/group/tag filter(s) instead of scan in real world use
+    report.add_common_vuln_filters()  # adds vuln filters to match UI defaults
+
+    # these imports should be at the top, or perhaps be auto imports in the init file?


jkee-r7 · 2017-10-11T16:53:28Z

+        assert data[2] == 'Content-Transfer-Encoding: base64'
+        assert data[3] == ''
+        assert data[0] == data[-1][:-2]
+        body = ''.join(data[4:-1])


I had experience when requesting an XML report & within the response there were multiple report items.
i.e. the response would look like this:

<ReportAdhocGenerateResponse success="1"/> --AxB9sl3299asdjvbA Content-Type: image/gif; name=Chart00000001.gif Content-Transfer-Encoding: base64 <Base64 encoded gif> --AxB9sl3299asdjvbA Content-Type: image/gif; name=Chart00000005.gif Content-Transfer-Encoding: base64 <Base64 encoded gif> --AxB9sl3299asdjvbA Content-Type: text/xml; name=report.xml Content-Transfer-Encoding: base64 <Base64 encoded xml> --AxB9sl3299asdjvbA Content-Type: image/gif; name=Chart00000002.gif Content-Transfer-Encoding: base64 <Base64 encoded gif>

So in this case, there should be some looping trying to find the actual report.

Also, it's really confusing to see splits & then calling each element without an example response.

Yeah this is just copied from the existing adhoc report method (which only takes a scan id) and it seems pretty limited. I would like this to be able to work the way the ruby gem does where you can write to file instead of holding it all in memory. Being able to capture the extra attachments (like charts) would be nice, but the ruby gem doesn't support that either so I'm not super concerned about it.

I agree with this. After experimenting a bit, it is possible to use Python's MIME parser to handle these cases but you need to modify the input slightly, i.e. prefix some headers. Here is an excerpt of how we implemented it:

full_msg = 'Content-Type: {}\r\n\r\n{}'.format( resp.headers['Content-Type'], resp.text, ) msg = email.message_from_string(full_msg) if not msg.is_multipart(): raise RuntimeError("Got unexpected (non multipart MIME) response from GenerateAdhocReport: {}".format(msg)) vuln_data = None # There will be several parts in the response (images etc.), # so linear search for the part that is the actual report. for part in msg.walk(): if part.get_filename() == "report.csv": assert part.get_content_type() == 'text/csv' # Decode the bytes. csv_ = part.get_payload(decode=True).decode('utf8') vuln_data = csv.DictReader(io.StringIO(csv_)) break else: raise RuntimeError("Could not find report in GenerateAdhocReport response")

A similar approach could work here as well I assume. That way all the verification can be deferred to a library but the content needs to be prepared properly.

jkee-r7 · 2017-10-11T17:11:28Z

+        If successful, the id will also have been updated in the provided ReportConfiguration object.
+        To create a new report, specify -1 as id.
+        """
+        # TODO: how to pass generate_now param downstream?


As in how to pass to Nexpose to generate the report now? If so:
<ReportSaveRequest session-id="{sid}" generate-now="{1/0}">

The problem is the magic under the hood with the ExecuteSave() function, it takes in a function as an argument and no way to pass arguments to that function. I suppose I could do something with **args or **kwargs to keep it compatible elsewhere. After this PR is done it's probably time to rewrite all of that stuff to just use the requests lib instead and reduce some of the "magic" involved.

jkee-r7 · 2017-10-11T17:15:53Z

+        self.timezone = timezone
+        self.language = None
+        self.filters = []
+        self.baseline = None


Why is baseline not passed as a param?

On line 107, I see that there is check to see if it exists, but I can't seem to locate where it gets set.

I've been mimicking the ruby gem for these classes and their constructors. I could add more optional arguments I guess, but how many is too many? 🤔

jkee-r7 · 2017-10-11T17:17:12Z

+        if delivery is not None:
+            cfg.delivery = Delivery.CreateFromXML(delivery)
+
+        # TODO: draw the rest of the owl


gschneider-r7 · 2017-10-13T17:13:01Z

The generate report request times out after 60 seconds, so fails for larger reports.
I assume that adhoc report will be affected by the same bug?

Yeah I need to figure something out for timeouts here. I think that in the real world adhoc reports don't scale very well anyway and should be avoided IMO. Since the consumer has to wait for the report to finish generating before receiving a response indicating success or failure, it would be better to instead use a regular report and poll the status on it while generating.

gschneider-r7 · 2017-10-24T23:47:46Z

So the NexposeSession class has a timeout property, which can be set globally for all requests it seems. I don't like the idea of changing "global" properties like that within specific functions, so I guess we'd have to leave it up to the user to set an appropriate timeout when running adhoc reports.

Due to the use of python-future for the urllib backports, the vcrpy lib can't wrap/patch the requests in python2 environment.

fruechel · 2017-11-17T21:42:22Z

        else:
            return self.ExecuteBasicOnReport("ReportDeleteRequest", report_id)

+    def RequestAdhocReport(self, report_config):


I don't quite understand why this is a new function...? Couldn't we just re-use the one below? Or do you want to make breaking changes here that wouldn't work with the below function?

I can't really remember why I made new ones, but they don't need to be new. I am not terribly concerned about backwards compatibility either at this point since there is a lot of work needed to get this to be on par with the ruby gem for functionality.

fruechel · 2017-11-17T21:49:31Z

I like the changes made in this PR. I would only change one thing: Make the API for the function with real arguments (template_id etc.). This makes the function self-documenting and allows consumers to get started quicker. This can then be augmented by a function that is more flexible, i.e. what you already have - so that the API doesn't limit what can be done (like it does now).

What do you think?

gschneider-r7 · 2018-01-11T19:44:40Z

Note to self: this branch needs to be updated with changes from #48

gschneider-r7 force-pushed the report_configs branch from 91b00a6 to d0168bb Compare October 10, 2017 19:51

jkee-r7 reviewed Oct 11, 2017

View reviewed changes

ghost reviewed Oct 11, 2017

View reviewed changes

jkee-r7 reviewed Oct 11, 2017

View reviewed changes

gschneider-r7 added 9 commits October 25, 2017 13:30

WIP: Add support for report configs

82edc83

WIP: Implement saving report config, add demo code

beeb49d

Implement more of the report config, fix test and names

2e79a37

Add support for generating AdhocReportConfiguration

4f6f6a4

Update run_demo.py for general use

27642c4

Fix report config loading, add vcr-backed test

b5970d0

Fix flake8 errors, remove login request from vcr cassette

8abaa83

Add py2/3 compatibility imports for report test

516b327

Fix py2 flake8 issues, skip vcr test on py2

10d4f79

Due to the use of python-future for the urllib backports, the vcrpy lib can't wrap/patch the requests in python2 environment.

gschneider-r7 force-pushed the report_configs branch from 510b7a9 to 10d4f79 Compare October 25, 2017 23:57

gschneider-r7 mentioned this pull request Nov 17, 2017

Improvements for AssetDetails, Tags & Unique Identifiers + Adhoc Reports #43

Merged

3 tasks

fruechel reviewed Nov 17, 2017

View reviewed changes

Conversation

gschneider-r7 commented Oct 9, 2017

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

jkee-r7 Oct 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkee-r7 commented Oct 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gschneider-r7 commented Oct 13, 2017

Uh oh!

gschneider-r7 commented Oct 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fruechel commented Nov 17, 2017

Uh oh!

gschneider-r7 commented Jan 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jkee-r7 Oct 11, 2017 •

edited

Loading