Refactor to understand

[bcpeinhardt]

Hello! This pull request is not meant to merged. I just bought access to your black hat rust course, and I've been using rust for two years now but I know nothing about cybersecurity (but want to!). I figured the best way for me to understand the security part was to go through section by section refactoring code until it made sense to me.
Obviously my little refactor incorporates parts of Rust you don't want to bother the reader with yet, but I do have some small suggestions and questions.

Questions:

1. What's the deal with this?

let socket_addrs: Vec<SocketAddr> =
            format!("{}:1024", subdomain).to_socket_addrs()?.collect();

2. If you were trying to avoid detection as mentioned in the text by pinging these over a longer period of time, how long would that be? Say generate a random wait between 60 - 360 seconds, or 2 - 5 hours, etc.

Suggestions:

1. You should probably scan for open ports before constructing a Subdomain struct. Initializing the open_ports field with an empty vec creates a struct which doesn't reflect reality. My version places scanning for open ports in the struct's constructor by implementing the from trait, but you could just wait to instantiate the Subdomain structs until you've found the open ports.
2. Having both a port struct with a field indicating whether the port is open and the field "open_ports" on the subdomain struct is a little strange, and risks representing an invalid state (the subdomain.open_ports vec could contain a port with port.is_open set to false). Whether or not a port is open should be indicated in one place only.
3. Since you've introduced the anyhow and thiserror crates, there's really no reason to be unwrapping anywhere.

[dmgolembiowski]

Hello! This pull request is not meant to merged. I just bought access to your black hat rust course, and I've been using rust for two years now but I know nothing about cybersecurity (but want to!). I figured the best way for me to understand the security part was to go through section by section refactoring code until it made sense to me. Obviously my little refactor incorporates parts of Rust you don't want to bother the reader with yet, but I do have some small suggestions and questions.

Questions:

1. What's the deal with this?

let socket_addrs: Vec<SocketAddr> =
            format!("{}:1024", subdomain).to_socket_addrs()?.collect();

2. If you were trying to avoid detection as mentioned in the text by pinging these over a longer period of time, how long would that be? Say generate a random wait between 60 - 360 seconds, or 2 - 5 hours, etc.

Suggestions:

1. You should probably scan for open ports before constructing a Subdomain struct. Initializing the open_ports field with an empty vec creates a struct which doesn't reflect reality. My version places scanning for open ports in the struct's constructor by implementing the from trait, but you could just wait to instantiate the Subdomain structs until you've found the open ports.

2. Having both a port struct with a field indicating whether the port is open and the field "open_ports" on the subdomain struct is a little strange, and risks representing an invalid state (the subdomain.open_ports vec could contain a port with port.is_open set to false). Whether or not a port is open should be indicated in one place only.

3. Since you've introduced the anyhow and thiserror crates, there's really no reason to be unwrapping anywhere.

I think I understand where the disconnect is happening. Namely, I agree with your point from (1) "... with an empty vec creates a struct which doesn't reflect reality" but if you take another look at different parts of the book where @skerkour uses the same pattern, you might think this is a clippy refactor or an accidental flaw. Just below the .collect() call, the scope checks if the number of retrieved SocketAddr is == 0 to potentially dodge a bigger problem — that the program can't request socket allocation from userspace. Maybe the system's hitting a soft/hard limit on open file handles? Maybe there's a permission issue? Who knows. Really — there should be a panic! imo or something comparable to propagating some io::Result to the caller.

Side note:
I found a spot where the .collect() call is doing something helpful. I believe it's on page 54. It looks like

pub fn scan_ports(mut subdomain: Subdomain) -> Subdomain {
    let socket_addresses: Vec<SocketAddr> = format!("{}:1024", subdomain.domain)
            .to_socket_addrs()
           .expect("port scanner: Creating socket address")
           .collect();
    if socket_addresses.len() == 0 {
       return subdomain;
    }    
   subdomain.open_ports = MOST_COMMON_PORTS_100
       .into_par_iter()
       .map(|port| scan_port(socket_addresses[0], *port))
       .filter(|port| port.is_open) // filter closed ports
       .collect();
   subdomain
} 

@bcpeinhardt are my assumptions and thoughts right? I'm second guessing myself on this.