Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib: feeds-config-generator does not add feed name as parameter #1314

Closed
SYNchroACK opened this issue Sep 4, 2018 · 10 comments
Closed
Labels
bug Indicates an unexpected problem or unintended behavior component: contrib
Milestone

Comments

@SYNchroACK
Copy link
Contributor

# python3 /opt/dev_intelmq/contrib/feeds-config-generator/intelmq_gen_feeds_conf --feeds-file=/opt/dev_intelmq/intelmq/etc/feeds.yaml

Select a feed: 57

RUNTIME CONFIGURATION:

{
    "openphish-phishing-collector": {
        "description": "placeholder",
        "group": "Collector",
        "module": "intelmq.bots.collectors.http.collector_http",
        "name": "OpenPhish Phishing",
        "parameters": {
            "http_url": "https://www.openphish.com/feed.txt",
            "rate_limit": 86400
        }
    },
    "openphish-phishing-parser": {
        "description": "placeholder",
        "group": "Parser",
        "module": "intelmq.bots.parsers.openphish.parser",
        "name": "OpenPhish Phishing",
        "parameters": {}
    }
}




PIPELINE CONFIGURATION:

{
    "openphish-phishing-collector": {
        "destination-queues": [
            "openphish-phishing-parser-queue"
        ]
    },
    "openphish-phishing-parser": {
        "source-queue": "openphish-phishing-parser-queue",
        "destination-queues": [
            "FIXME"
        ]
    }
}
# cat /opt/intelmq/etc/pipeline.conf 
{
    "openphish-phishing-collector": {
        "destination-queues": [
            "openphish-phishing-parser-queue"
        ]
    },
    "openphish-phishing-parser": {
        "source-queue": "openphish-phishing-parser-queue",
        "destination-queues": [
            "FIXME"
        ]
    }
}
# cat /opt/intelmq/etc/runtime.conf 
{
    "openphish-phishing-collector": {
        "description": "placeholder",
        "group": "Collector",
        "module": "intelmq.bots.collectors.http.collector_http",
        "name": "OpenPhish Phishing",
        "parameters": {
            "http_url": "https://www.openphish.com/feed.txt",
            "rate_limit": 86400
        }
    },
    "openphish-phishing-parser": {
        "description": "placeholder",
        "group": "Parser",
        "module": "intelmq.bots.parsers.openphish.parser",
        "name": "OpenPhish Phishing",
        "parameters": {}
    }
}
# su - intelmq

$ intelmqctl status 
intelmqctl: Bot openphish-phishing-collector is stopped.
intelmqctl: Bot openphish-phishing-parser is stopped.

$ intelmqctl start openphish-phishing-collector 
intelmqctl: Starting openphish-phishing-collector...
intelmqctl: Bot openphish-phishing-collector is running.

$ cat /opt/intelmq/var/log/openphish-phishing-collector.log 
2018-09-04 18:17:32,832 - openphish-phishing-collector - INFO - HTTPCollectorBot initialized with id openphish-phishing-collector and intelmq 1.1.0 and python 3.6.5 (default, Apr  1 2018, 05:46:30) as process 1655.
2018-09-04 18:17:32,833 - openphish-phishing-collector - INFO - Bot is starting.
2018-09-04 18:17:32,834 - openphish-phishing-collector - INFO - Pipeline ready.
2018-09-04 18:17:32,834 - openphish-phishing-collector - INFO - Downloading report from 'https://www.openphish.com/feed.txt'.
2018-09-04 18:17:33,574 - openphish-phishing-collector - INFO - Report downloaded.
2018-09-04 18:17:33,589 - openphish-phishing-collector - ERROR - Bot has found a problem.
Traceback (most recent call last):
  File "/opt/dev_intelmq/intelmq/lib/bot.py", line 167, in start
    self.process()
  File "/opt/dev_intelmq/intelmq/bots/collectors/http/collector_http.py", line 97, in process
    self.send_message(report)
  File "/opt/dev_intelmq/intelmq/lib/bot.py", line 831, in send_message
    super(CollectorBot, self).send_message(*messages, path=path)
  File "/opt/dev_intelmq/intelmq/lib/bot.py", line 812, in __add_report_fields
    report.add("feed.name", self.parameters.name)
AttributeError: 'Parameters' object has no attribute 'name'
2018-09-04 18:17:33,592 - openphish-phishing-collector - INFO - Bot will continue in 15 seconds.
$ intelmqctl --version
1.1.0
@SYNchroACK SYNchroACK added bug Indicates an unexpected problem or unintended behavior component: configuration labels Sep 4, 2018
@SYNchroACK SYNchroACK added this to the 1.1.0 milestone Sep 4, 2018
@ghost ghost changed the title Feeds.yaml outdated contrib: feeds-config-generator does not add feed name as parameter Sep 5, 2018
@ghost
Copy link

ghost commented Sep 5, 2018

That's a bug in the script. It needs to add the name of the feed ("Phishing" in this case) as parameter for the bot.

@ghost ghost modified the milestones: 1.1.0, 1.1.1 Sep 5, 2018
@SYNchroACK
Copy link
Contributor Author

SYNchroACK commented Sep 6, 2018

That's a bug in the script. It needs to add the name of the feed ("Phishing" in this case) as parameter for the bot.

Approach 1

The name of the feed is already being generated by the script like the configuration required. However, the script does not put the value inside parameters:

{
    "openphish-phishing-collector": {
        "description": "placeholder",
        "group": "Collector",
        "module": "intelmq.bots.collectors.http.collector_http",
        "name": "OpenPhish Phishing",
        "parameters": {
            "http_url": "https://www.openphish.com/feed.txt",
            "rate_limit": 86400
        }
    },

...

You can see here on this line: https://github.com/certtools/intelmq/blob/develop/intelmq/lib/bot.py#L495

So, unless there is a specific reason for that (I don't know), I would suggest to change the line (https://github.com/certtools/intelmq/blob/develop/intelmq/lib/bot.py#L812) to the following one:

report.add("feed.name", self.name)

@sebix makes sense the way I'm thinking?

Approach 2

Or may be the other way around (I think I start understanding your point) is:

Let me know which approach you want me to follow (or other one) and I will create a pull request.

@ghost
Copy link

ghost commented Sep 7, 2018

Approach 1:

The name of the feed is already being generated by the script like the configuration required. However, the script does not put the value inside parameters:

What I called a bug in the script (a bug because it does not behave what's expected by intelmq).

So, unless there is a specific reason for that (I don't know), I would suggest to change the line (https://github.com/certtools/intelmq/blob/develop/intelmq/lib/bot.py#L812) to the following one:

That would be a change in the behavior, that at least needs backwards compatibility and consensus.

Approach 2:

Add "name" parameter to the "feeds.yaml" file in all Collectors.

The feed name is already the name of the section, e.g. "Phishing". Or alternatively concatenate provider and feed name.

The removal of self.name in bots/the runtime configuration is also needs backwards compatibility but this is inconsistent because all bots have names, except Collectors.

@SYNchroACK
Copy link
Contributor Author

So, why do we need a self.parameters.name when we have self.name. Every time we configure a collector we use the name field to define a name and not the parameters: { name: "..."}.

So, I think the fix is just change this line (https://github.com/certtools/intelmq/blob/develop/intelmq/lib/bot.py#L812) to:

report.add("feed.name", self.name)

Makes sense?

@ghost
Copy link

ghost commented Sep 10, 2018

So, why do we need a self.parameters.name when we have self.name.

Better ask that yourself, this was already the case before I joined this project ;)

Joking apart, the difference is that name/description/group/module describe the bot as in the BOTS file. So self.name is the name of the bot class itself. In the manager the fields group, module and name are read-only (name can be discussed here, but module and group are definitely read-only values).

The parameter name is used as feed.name in collectors. That's also the reason why it is called "name", but the name of the parameter "name" is not very good. Consequently it has been proposed to change it to "feed", see #1144 and #1241 for the implementation for the develop branch, meaning version >= 1.2.0

@SYNchroACK
Copy link
Contributor Author

Better ask that yourself, this was already the case before I joined this project ;)

Or may be better write on documentation?!?!
The name field is indeed clean and clear, what a choice!!! ...without documentation, just perfect! ;)

Joking apart, the difference is that name/description/group/module describe the bot as in the BOTS file. So self.name is the name of the bot class itself. In the manager the fields group, module and name are read-only (name can be discussed here, but module and group are definitely read-only values).

The parameter name is used as feed.name in collectors. That's also the reason why it is called "name", but the name of the parameter "name" is not very good. Consequently it has been proposed to change it to "feed", see #1144 and #1241 for the implementation for the develop branch, meaning version >= 1.2.0

The #1144 and #1241 are exactly the opposite of "Consequently it has been proposed to change it to feed"

So... What needs to be done regarding this topic? code and docs are not sync regarding this field...

@SYNchroACK
Copy link
Contributor Author

@wagner-certat let me know if this updates make sense #1321

@ghost
Copy link

ghost commented Sep 12, 2018

code and docs are not sync regarding this field...

Where?

Please note that the change from name to feed is done for 1.2.0 (develop branch) not the maintenance branch (1.1.1) you are currently working on (which is fine IMO btw).

I'm currently reviewing the PR.

@SYNchroACK
Copy link
Contributor Author

SYNchroACK commented Sep 12, 2018

Where?

In both branches (maintenance and develop) and I don't see the requirement for name field parameter in the following docs page:
https://github.com/certtools/intelmq/blob/develop/docs/Feeds.md#abusech
https://github.com/certtools/intelmq/blob/maintenance/docs/Feeds.md#abusech
https://github.com/certtools/intelmq/blob/develop/docs/Bots.md#collectors
https://github.com/certtools/intelmq/blob/maintenance/docs/Bots.md#collectors

Please note that the change from name to feed is done for 1.2.0 (develop branch) not the maintenance branch (1.1.1) you are currently working on (which is fine IMO btw).

I'm not understanding .... are you saying that in version 1.2.0 (develop branch) changed FROM name TO feed? I'm confused with this line:
https://github.com/certtools/intelmq/blob/develop/intelmq/lib/bot.py#L808

I'm currently reviewing the PR.

Thank you! My pull request tries to sync docs and code.

@ghost
Copy link

ghost commented Sep 13, 2018

Hm, wait. I'm finally confused.

Ok, the (new) logic is:
The fields feed.* can be added to reports by specifying the rear part as parameter. E.g. the parameter provider of a collector's configuration becomes feed.provider, same for feed.documentation, feed.code and feed.name. So your assumptions were correct :)

Ad docs:
Confirmed, it's missing in the feeds' documentation. For the bots it is mention in https://github.com/certtools/intelmq/blob/develop/docs/Bots.md#common-parameters with the old name.

@ghost ghost closed this as completed in 26dcb26 Oct 19, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior component: contrib
Projects
None yet
Development

No branches or pull requests

1 participant