Issues with random graph

[sherdencooper]

Hi, I am working on generating the random graph and I found some logical issues with the random graph generation design (maybe).

1. First, I think the add_leak_neighbors_vulnerability function in generate_network.py should not return library directly. Since it is a global dictionary, using it to assign the vulnerability will make all nodes share the same vulnerability outcomes which is not reasonable. It can be fixed by return return copy.deepcopy(library)
2. Here the nodes except the entry node are iteratively assigned with vulnerabilities and services. However, some nodes may be assigned empty services before their corresponding access passwords are generated in the assigned_passwords. I fix this issue simply by repeating the loop.

I am not familiar with network attacks, so would it be convenient for you to check whether the editions are right? Thx a lot.

[blumu]

@sherendcooper Thanks for reporting. Could you share a diff or a PR so we can review the change you are proposing?

[blumu]

@sherdencooper Libraries of vulnerabilities are not necessarily global. The function add_leak_neighbors_vulnerability takes an optional library parameter, if not specified a new library gets created. This way each node can also have its own specific 'library' of vulnerabilities. For instance, in create_node_data, the function add_leak_neighbors_vulnerability gets called via create_vulnerabilities_from_traffic_data to create the set of vulnerabilities assigned to a particular node.
I don't think deep copy is needed here, since a new library gets created each time add_leak_neighbors_vulnerability is called (with library parameter set to None)

[blumu]

For 2. That's indeed a bug. The fix is to generate the nodes and services first with vulnerabilities set to empty. Then loop again over the nodes to generate and assign vulnerabilities for each node.

[sherdencooper]

I am not sure whether the problem1 is raised by the difference in the python version, but in my running without deepcopy, the generated graph does not have the vulnerabilities outcomes as expected. By debugging, I found that the code does not generate new library as it was expected to. All nodes are assigned with the same library address. I made a toy example to explain the issue I found. Could you try to run this toy example in python 3.8.0?

from typing import NamedTuple,Dict
import copy

VulnerabilityID = str
class VulnerabilityInfo(NamedTuple):
    """Definition of a known vulnerability"""
    cost: float = 1.0

VulnerabilityLibrary = Dict[VulnerabilityID, VulnerabilityInfo]


def toy_assign_1():
    def add_leak_neighbors_vulnerability(
            node_id: int,
            library: VulnerabilityLibrary = {}) -> VulnerabilityLibrary:
        library['ScanWindowsCredentialManagerForRDP'] = VulnerabilityInfo(cost=node_id)
        return library
    nodes = [0,1,2,3,4]
    nodes_cost = [0,0,0,0,0]
    for count, node in enumerate(nodes):
        nodes_cost[count] = add_leak_neighbors_vulnerability(count)
    print(nodes_cost)

def toy_assign_2():
    def add_leak_neighbors_vulnerability(
            node_id: int,
            library: VulnerabilityLibrary = {}) -> VulnerabilityLibrary:
        library['ScanWindowsCredentialManagerForRDP'] = VulnerabilityInfo(cost=node_id)
        return copy.deepcopy(library)
    nodes = [0,1,2,3,4]
    nodes_cost = [0,0,0,0,0]
    for count, node in enumerate(nodes):
        nodes_cost[count] = add_leak_neighbors_vulnerability(count)
    print(nodes_cost)


if __name__ == "__main__":
    toy_assign_1()
    toy_assign_2()

[blumu]

@sherdencooper I see! There is indeed a bug then, it seems that the empty library {} defined as the default value for the optional parameter is shared across all calls to the function! So a possible fix is set the default to None instead (library=None) and add the code:

if library is None:
  library = {}

[blumu]

@sherdencooper Reopening this bug. I think you might have closed it by accident. I've create a PR to fix the first part of the bug.

[blumu]

@sheredencooper Fix proposed in #58 Give it a try and let me know if it works for you.

[sherdencooper]

Thanks for your fix, it works for me! BTW, I am doing some research work about RL and using your simulator. It's the greatest cyber attack simulator I have found! When I was using your simulator, I found something not so reasonable about the reward design.

1. In here, the reward at each step is calculated as reward = max(0., reward). I see the code about penalty in actions.py and I understand why you cancel that. When I tried to remove this max() operation, the reward became highly negative and the agent learns nothing. However, it makes the agent overfit because the reward is so sparse. I think adding some time cost is necessary such as -1 or -0.5.
2. In here, when giving the reward for NEW_SUCCESSFULL_ATTACK_REWARD, it does not take into consideration whether the attacked the node is already owned. It's meaningless to attack a node already owned by the attacker. It will make the agent repeatedly launch attacks between owned nodes and ignore discovering new nodes.

In my experiment, I trained an agent with the original reward design in the chain env. The agent can perfectly take ownership of the network in training. When I saved the model and evaluate it with epsilon-greedy, the success rate is only about 90%. When I patched the two points I proposed above and trained an agent with the same parameters, the successful rate for evaluation is about 100%. I think the original reward design makes the agent overfit.

Could you please take a look at the two points and give some feedback? Anyway, thanks again for your codes, it helps with my research, and I even would like to use them in my next research project about online learning :)