Skip to content

Conversation

@Script-Nomad
Copy link
Collaborator

I had a need for this in order to implement my own parser for nessus's xml output and ran into some trouble that prompted me to implement a few changes to the library. The most significant change was the creation of a new exceptions class file in the base of the library (because it should be accessible from a base import).

The purpose of the exception class file is to introduce custom exceptions for the library which should be created for edge-case scenarios such as the one I encountered while parsing a nessus file. Specifically the error was caused by a configuration change in the scan for nessus to not scan the local host. This apparently creates a unique pluginID that is missing most of the ordinarily common and essential attributes that libnessus requires.

Aside from this tweak, it's mostly just adding some new functionality (like getting the CVE as a string) and some minor spelling mistakes I encountered while casually making changes.

Thanks in advance for reviewing this and your work on this library. Extremely useful stuff. 👍

It's always preferable to create custom exceptions for weird edge-cases
like missing attributes from the Report object. This is esspecially
helpful in cases where certain nessus plugins don't return all of the
attributes that are necessary, and gives users a straight-forward way to
handle and pass the exception accordingly.
implemented exception handling for missing attributes
plus fixed some minor spelling errors.
@bmx0r
Copy link
Owner

bmx0r commented Nov 15, 2018

Thanks a lot for you contribution to this project.
do you mind doing two things:

  • check my comment as i believe you made a typo :)
  • change travis.yml
-- pip install pep8 --use-mirrors
-- pip install pyflakes --use-mirrors
+- pip install pep8
+- pip install pyflakes

we should then have a partial green build
And then i will probably merge :)

@Script-Nomad
Copy link
Collaborator Author

Script-Nomad commented Nov 20, 2018

Alright. Changes are pushed. Thanks for catching that typo. Travis doesn't seem to like the pep8 . check though. 🤷‍♂️

There is another issue I encountered with a separate Nessus file that resulted in similar issues that cause the parser to fail. This is because it can't find all the necessary attributes, even though it would normally be handled by for report_item in root.findall("ReportItem") here.
https://github.com/bmx0r/python-libnessus/blob/master/libnessus/parser.py#L75

The problem is that this area of the code is meant to be transparent to the user, or at least I assume so based on the number of private method calls. Raising the error here won't really have any useful effect for the user if they create a NessusReport object, which calls the parser...which parses the hosts...which parses the items, and so on.

The issue is that there is at least one "Info" plugin that is missing these required attributes by design, and there may be more in the future. Ultimately, I think it would be more ideal to accept all ReportItems regardless of what attributes are missing and let the user decide whether or not to accept them or throw them away accordingly by raising the MissingAttribute exception to the top when the user tries to access an item property that is empty.

This is my temporary workaround just so that I can parse the file and throw away the invalid ReportItems:

        for report_item in root.findall("ReportItem"):
            try:
                _new_item = cls.parse_reportitem(report_item)
            except MissingAttribute:
                continue
            _vuln_list.append(_new_item)

Obviously not a permanent solution. I'm willing to help change the implementation, but I'll wait to know how you would want to handle the issue.

  1. Would you rather accept bad/empty attributes, or throw away bad ReportItems and risk missing details?
  2. Would you want to warn the user in either case and how would you want to handle that?

I've noticed that in every case, the problematic plugin is the "You didn't set Credentials" Info finding which is a bit annoying. It's actually completely empty, except for the name and description. So far, it looks like this is the only problematic plugin.

annoying_nessus_plugin

Curse you Tenable -.-

@bmx0r
Copy link
Owner

bmx0r commented Dec 30, 2018

Hey,
Sorry for this late answer...
You can solve this has it better suit you, but it would be nice to put at least a warning message.
Would you like to do this in the same PR? or will you open another one?
Regards

@Script-Nomad
Copy link
Collaborator Author

Hey, no worries. Sorry that I'm a bit late too. I can work on this and should be able to solve the issues of encountering empty plugins. I think the best solution is to catch any exceptions due to incomplete plugins and warn the user. I have a feeling that if I push this PR now that it will likely break things down the line, so I will fix things here when I have the time.

Implemented pythonic log parser

Added debug logging + PEP 257 Docstring changes

Implementing libnessus logging

Edited to PEP 257 compliant docstrings

Implemented "Strict" parsing option and log warnings to alert users when invalid report items are encountered
@Script-Nomad
Copy link
Collaborator Author

Script-Nomad commented Jun 4, 2019

Hi there @bmx0r Sorry this sat for so long, but I finally managed to gather some time to implement some changes that I think are a reasonable approach to dealing with the Bad/MissingAttributes situation. In order to prevent existing codebases that depend on libnessus from breaking, I implemented an optional "strict" boolean argument to all of the parser functions which get passed down to the private functions. By default, it is set to false. As long as strict parsing is false, it will simply warn users when it encounters an invalid ReportItem object due to missing attributes. Users may explicitly set strict parsing to True, which will throw an exception and force the user to deal with it as they please.

So far my testing of this has not introduced any other problems and works as intended. Unfortunately, I am not at all familiar with Travis-CI and cannot get it to behave. As far as I can tell, the code meets all the reasonable PEP8 standards, and works on Python 3.3+. I have not tested any versions prior to that. If it is explicitly required that all of the Travis-CI build checks pass, let me know, but otherwise, I believe this is good to go. If any specific changes are needed, mark them and I'll be happy to address.

Cheers 🍻

@Script-Nomad
Copy link
Collaborator Author

Fixed the Travis-CI build config.
As it turns out, pyflakes no longer works with python<=2.6 or python<=3.3, so I had to remove them from the config.
Pep8 checks still cause the build to fail because of issues in some of the imported modules, rather than the actual libnessus code, so they have been omitted.
The libnessus code itself is still pep8 compliant. 👍

@Script-Nomad
Copy link
Collaborator Author

@bmx0r Just wanted to drop a reminder in here in case this hasn't been seen yet. If changes are needed, let me know. Would be nice if this could get merged into pypi so that a local fork isn't needed anymore. 😆

@bmx0r bmx0r merged commit 88bdf57 into bmx0r:master Aug 15, 2019
@bmx0r
Copy link
Owner

bmx0r commented Aug 16, 2019

and finnaly: https://pypi.org/project/python-libnessus/
Sorry for the delay ;)
A thanks you a lot for the contribution :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants