Add schedule management code in manager.py so that one can configure cloudmarker to run at a fixed time everyday.

Commit 6ae974a added the documentation for EmailAlert to the documentation for cloudmarker.stores package. This should have been added to the documentation for cloudmarker.alerts package instead.

Running make checks should fix this because make checks invokes the make docs target which generates the correct Sphinx project files. The following steps need to be carried out to fix this:

# Create a new branch (the usual stuff).
git checkout master
git pull
git checkout -b docfix

# Create an __init__.py file for alerts package. This is necessary for
# this folder to be recognized as a package. Otherwise no documentation
# would be generated for this package. Make the content of this file
# similar to that of cloudmarker/events/__init__.py
vim cloudmarker/alerts/__init__.py

# View the changes.
git diff docs/api

# Add the changes.
git add docs/api

# Then commit, push, and create a pull request (again, the usual stuff).
git commit
git push origin docfix

The current signature of MongoDBStore looks like this:

def __init__(self, host, port, username=None, password=None, db='gcp', buffer_size=1000)

After reading @prateeknischal's review comments in an earlier pull request, I think this sigature should be like this:

def __init__(self, host='localhost', port=27017, db='cloudmarker', username=None, password=None, buffer_size=1000)

The exact order of the parameters does not matter technically because this signature is using sensible defaults for every parameter. However, just for the sake of following some convention, I have arbitrarily chosen the big-endian order here: a host contains port, a port has one or more MongoDB databases behind it, and a database has one or more users.

Right now, we generate logs in Azure plugin that look like:

Found document virtual_machine #0; sub_id: a8daa361-b951-46b5-8e9d-b375f2ffe9fd; name: vm1

I think we can omit the word document from the logs. It makes the logs more brief without losing any information.

Is it possible to add usage documentation for this project? Like what this project for and what it does? It will be helpful to get an essence of what it does.

Running sanity check on a fresh clone of Cloudmarker leads to this error:

$ python3 -m cloudmarker -f
...
Traceback (most recent call last):
  File "/usr/lib/python3.5/multiprocessing/process.py", line 249, in _bootstrap
    self.run()
  File "/usr/lib/python3.5/multiprocessing/process.py", line 93, in run
    self._target(*self._args, **self._kwargs)
  File "/home/susam/git/cloudmarker/cloudmarker/workers.py", line 68, in store_worker
    store_plugin.done()
  File "/home/susam/git/cloudmarker/cloudmarker/alerts/emailalert.py", line 71, in done
    smtp_connection = smtplib.SMTP(host=self.host, port=self.port)
  File "/usr/lib/python3.5/smtplib.py", line 251, in __init__
    (code, msg) = self.connect(host, port)
  File "/usr/lib/python3.5/smtplib.py", line 335, in connect
    self.sock = self._get_socket(host, port, self.timeout)
  File "/usr/lib/python3.5/smtplib.py", line 306, in _get_socket
    self.source_address)
  File "/usr/lib/python3.5/socket.py", line 694, in create_connection
    for res in getaddrinfo(host, port, 0, SOCK_STREAM):
  File "/usr/lib/python3.5/socket.py", line 733, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known

This error occurs because emailalert is enabled for mockaudit by default in config.base.yaml. This should be removed from mockaudit because there is no default configuration for the EmailAlert plugin that would work well for all users. More importantly, a sanity check after a fresh clone should run without any issues. Also, see https://github.com/cloudmarker/cloudmarker/pull/55/files#r270620747 for more discussion on this.

The handler created with TimedRotatingFileHandler is named as file_handler in baseconfig.py. Rename this to just file to be consistent with the handler name console that is also present in the same file.

Create a very minimal yet complete, rule comparison based FirewallRule Checker.

This issue is actually a thread to capture feedback on the Cloudmarker project from the developer and user community.

We seem to be having a proliferation of Azure cloud plugins. The Azure cloud plugins are heavyweight plugins because they launch multiple workers and threads. They also have overlapping concerns. For example, all of them iterate over all the subscriptions. Sometimes two of them iterate on the same type of resource. There is also a high configuration overhead because the same Azure credentials would need to be entered for each cloud plugin in the config file.

I am thinking we should change the AzCloud design a little bit so that the functionality of all other plugins become part of AzCloud plugin. Here's my proposal:

• Move azsql.py, azvm.py, azwebapp, etc. to a library package, say, azreaders or azlib.
• These individual modules no longer launch multiple threads and processes. They just yield each record in a single threaded manner.
• AzCloud's worker method, i.e., _get_resources() invokes these modules in multiple processes and threads to retrieve the data.
• The functionality of an individual module can be turned on and off via AzCloud's __init__() parameters. For example, in the params config of AzCloud, we can specify that things like we want azsql data to be pulled but we do not want azvm data to be pulled.

Note that we are similarly having a proliferation of event plugins but I think that's fine because the concerns of event plugins are much more isolated and they are very lightweight too.

This is a significant change, so I am looking for a consensus on this idea. If we have consensus, I am willing to perform the entire change myself, or if you prefer, distribute the change among ourselves.

Right now, the EmailAlert plugin just dumps the JSON data it receive into the email content. In order to make the emails meaningful to the recipients, the payload format should be made human-friendly.

See this comment for background on this.

The AzureCloud plugin currently accepts the following three parameters: tenant_id, client, and key.

I recommend naming them to:

• tenant (remove _id from the name to be consistent with client)
• client (no change required)
• secret (to be consistent with keyword argument name for ServicePrincipalCredentials)

Also, for future reference, I am noting down the several different terminologies in use for each of these fields. We would have to clarify these differences in documentation in future to avoid any confusion.

With different terms in use for the same fields in various official artifacts of Microsoft, I like the recommendation I have provided in this ticket. It also clearly expresses that the secret is a secret and therefore must be protected carefully.

Pull request #65 creates firewall rule records with cloud-independent property names and values in the com bucket of each record.

Now create a FirewallRuleEvent plugin that uses the cloud-independent common properties to detect insecurely exposed specific ports to the entire Internet.

The logger handlers in baseconfig.py are configured to be logging with DEBUG level. The default should be INFO in the favour of keeping sane defaults. If a users want DEBUG level logging, then they must configure it explicitly.

Check if simply removing the level: DEBUG lines makes the handlers fallback to the root logger's configuration which is already configured to be level: INFO.

See these comments for background on this:

• #98 (review)
• #98 (comment)
• #98 (comment)

Some decisions that need to be taken:

• What should we do if the payload size exceeds, say, 25 MB? Should we truncate the email? Should we split the payload between multiple emails?
• Should this be handled in the EmailAlert plugin or should it be handled in the utility function util.send_email()?

Send an email notification after all audits have run.

Point 5 in the instructions provided in the README contains this command:

python3 -m cloudmarker

This does not run audits right now but instead waits until a scheduled time to start audits. This could be confusing to someone new to the project. Replace this command with the following:

python3 -m cloudmarker -n

This command runs the audits right now.

Currently, this project does not have a dashboard to see the insights from the data collected, be it either anomaly data generated by FirewallCheck plugin or for the data fetched from the cloud. This issue is more of enhancement in which I plan to research a simple dashboard that can be easily integrated with the current system and is also easily maintainable + features rich.

Since cloudmarker.events.firewallruleevent.FirewallRuleEvent can detect weak firewall rules in both Azure and GCP clouds, we should now remove the GCP-specific cloudmarker.events.firewallevent.FirewallEvent plugin.

This would most likely involve:

git rm cloudmarker/events/firewallevent.py
git rm cloudmarker/test/test_firewallevent.py
make checks

Right now, the output of FileStore at say, looks like this:

[
{
  "raw": {
    "data": 0
  },
  ...
  "com": {
    "type": "mock",
    "origin_worker": "mockaudit-mockcloud",
    "origin_type": "cloud",
    "cloud_worker": "mockaudit-mockcloud",
    "store_worker": "mockaudit-filestore"
  }
}]

The opening square bracket is in its own line but the closing square bracket is not. Place the square bracket in its own line, so that the output looks symmetrical.

This is a development environment only issue. If logs directory already exists, make deps fails with the following error:

mkdir logs
mkdir: logs: File exists
make: *** [deps] Error 1

This issue has no serious impact on developers because the dependencies are installed correctly prior to this error. In the interest of neatness, we should fix this. A simple solution is to use the mkdir -p option, e.g., mkdir -p logs which does not report error for existing directories.

We should support tilde expansion in any paths read from configuration as much as possible.

For example, currently the filestore plugin module accepts a directory path as path parameter to write output files.

It should expand tilde (~) or (~user) in the path to the home directory of the user.

Python has a os.path.expanduser() function that does this. This should be used to do this task. Here is an example usage:

>>> import os
>>> os.path.expanduser('~susam')
'/Users/susam'
>>> os.path.expanduser('~susam/foo')
'/Users/susam/foo'
>>> os.path.expanduser('~')
'/Users/susam'
>>> os.path.expanduser('~/foo')
'/Users/susam/foo'

Currently we provide a number of subscription we would like to pull and check for misconfigurations. There might be scenarios where user would like to pull only certain subscriptions.

This would be a good functionality specially for developers to pull only specific subscription so that it checks for configurations only for his subscription and doesn't get flooded with other subscriptions.

The SMTP debuglevel should be False by default. A user should be able to enable it via configuration.

See this thread for details.

Develop support for Elasticsearch store.

Pull request #81 introduces EsStore plugin to index data into Elasticsearch. In earlier discussions, we concluded that the abbreviated names (e.g., "Es" for "Elasticsearch") look good for plugins when suitable and popular abbreviations exist.

To keep the remaining plugins consistent with this preference for abbreviations, we can rename azurecloud.AzureCloud to azcloud.AzCloud.

See the list of issue labels at https://github.com/cloudmarker/cloudmarker/issues/labels.

We have the following issue labels right now which were provided in GitHub by default:

I am planning to add the following additional labels to better label the issues:

Please review these labels and suggest if you would like any additional labels to be added. Please feel free to edit this comment and fill in or alter the color suggestions (if any) in the third column of the new labels.

The gcloud CLI command can get the list of VMs without requiring the zone parameter.

gcloud auth revoke
gcloud auth activate-service-account --key-file=keyfile.json
project_id=$(grep project_id keyfile.json | sed 's/.*: "\(.*\)".*/\1/')
gcloud compute instances list --project "$project_id"

We should debug the gcloud CLI code and figure out what it is doing to be able to list the VMs without requiring the zone parameter and reuse that in our project.

The module-level logger variable is named as _log in most places but _logger in one place.

$ grep -r "logging.getLogger" cloudmarker
cloudmarker/stores/mongodbstore.py:_log = logging.getLogger(__name__)
cloudmarker/clouds/azurecloud.py:_log = logging.getLogger(__name__)
cloudmarker/workers.py:_logger = logging.getLogger(__name__)
cloudmarker/manager.py:_log = logging.getLogger(__name__)

Since _log is the more frequently used name, we should settle on using _log everywhere.

Rename _logger to _log in workers.py.

The current CLI options look like this:

$ python3 -m cloudmarker -h
usage: cloudmarker [-h] [-c CONFIG [CONFIG ...]] [-f]

optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG [CONFIG ...], --config CONFIG [CONFIG ...]
                        Configuration file paths
  -f, --force           set this flag to force a run

I suggest that -f, --force be changed to the following:

  -n, --now             ignore schedule and run audits now

I suggest this change so that the option name makes more sense to new users. The function cloudmarker.util.parse_cli() needs to be modified for this.

AzWebAppTLSEvent currently checks the minimum TLS version enabled in an Azure web app and generates an event if it this version does not match the string '1.2'. This is an exact string match. This would lead to incorrect results, for, say, an Azure web app that has its minimum TLS version set to '1.3'. Although, this is a more secure web app configuration, this event plugin would still generate an event for it because it fails to account for the fact that version '1.3' is more recent than version '1.2'. Therefore, this plugin should do a more sophisticated version comparison instead of an exact string match.

See this comment for more context about this issue: #154 (comment)

@mitprasoon: The key min_tls_version is a string. So this expression will be true only of the value of the key min_tls_version is 1.2
This will evaluate to false even if min_tls_version is 1.3
TLS 1.3 is already made available to its customers by many cloud vendors. I suggest to make this check configurable with 1.2 set to minimum tls version for now.

To reproduce this issue, create a fresh new virtual Python environment, so that it pulls the latest versions of the dependencies including PyYAML 5.1 which is necessary to reproduce this issue:

make rmvenv venv deps

Now on running Cloudmarker, we get YAMLLoadWarning about yaml.load(). For example,

$ make test
cloudmarker/cloudmarker/baseconfig.py:110:
YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated,
as the default Loader is unsafe. Please read https://msg.pyyaml.org/loa
d for full details.
  config_dict = yaml.load(config_yaml)
...

See https://msg.pyyaml.org/loa for more details on this warning.

It looks like, replacing the yaml.load() call with yaml.safe_load() call would be a good fix for this issue.

I believe this project is all about monitoring cloud platforms, so why not add support for AWS. AWS has a robust API package boto3. Below is a snip, I will be glad to take this up if I can get the project and contribution details.

class Aws:

    def __init__(self, aws_key, aws_id, aws_reg, end_pt_url):
        self.aws_key = aws_key
        self.aws_id = aws_id
        self.aws_reg = aws_reg
        self.end_pt_url = end_pt_url

    def session_to_aws(self):
        # this gets the instances from the given aws account.
        vpcc = boto3.session.Session()
        return vpcc.resource(service_name='ec2', use_ssl=True, verify=False, aws_access_key_id=self.aws_id,
                             aws_secret_access_key=self.aws_key,
                             region_name=self.aws_reg, endpoint_url=self.end_pt_url)`

This can grab the instance, SG and all other stuff. Then we can work on serializing the data and send it to the appropriate indexing engine.

The key sub_id seems too short and may not be obvious to what "sub" expands to. I think it is good to name it subscription_id instead as per the Azure terminology.

See the "Found" logs in AzureCloud plugin. GCPCloud plugin should also have similar logs.

Adding topics like cis-benchmark and security to this repo will help index it and make it easily searchable within similar topics.

E.g., https://github.com/topics/cis-benchmark

See these comments for background on this.

This is a discussion. Right now an example configuration looks like this:

audits:
  mockaudit:
    clouds:
      - mockcloud
    stores:
      - filestore
    checks:
      - mockcheck
    alerts:
      - filestore

We talk about these plugins in this manner:

• The cloud plugins generate cloud records.
• The store plugins store the cloud records.
• The check plugins generate event records.
• the alert plugins alert on the event records.

I think two separate terms "check" and "event" are superfluous and unnecessary. The word "event" is an industry standard, so I propose we settle only on a single word "event" here. So my proposal is a configuration like this:

audits:
  mockaudit:
    clouds:
      - mockcloud
    stores:
      - filestore
    events:
      - mockevent
    alerts:
      - filestore

We would now talk about the plugins in this manner:

• The cloud plugins generate cloud records.
• The store plugins store the cloud records.
• The event plugins generate event records.
• the alert plugins alert on the event records.

Let us know what you think.

The documentation at https://cloud.google.com/sdk/gcloud/reference/auth/activate-service-account refers to the service account key file as "key file", so I think to be consistent with official documentation we should rename the service_account_key_path parameter to just key_file_path.