Move ICMP packet creation and parsing out of Pinger implementation

[acb-mv]

This moves createICMPPacket and parseICMPResponse out of the Pinger implementation to their own namespace (an instance-free struct named ICMP). This will be useful for reusing them in a new Pinger implementation that uses the IAN tunnel rather than CFSocket.

---

This change is 

[linear]

IOS-750 Separate ICMP parsing/formatting logic from Pinger implementation

[pinkisemils]

Reviewed 1 of 3 files at r1, 1 of 2 files at r2, all commit messages.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @acb-mv)

---

ios/PacketTunnelCore/Pinger/ICMP.swift line 26 at r2 (raw file):

        case ipv4PacketTooSmall
        case icmpHeaderTooSmall
        case invalidIPVersion

We should log, thus also capture, the unexpected value. This more or less applies to most other error types about unexpected values.

---

ios/PacketTunnelCore/Pinger/ICMP.swift line 86 at r2 (raw file):

            // Verify ICMP type.
            guard icmpHeaderPointer.pointee.type == ICMP_ECHOREPLY else {
                throw Error.malformedResponse(.invalidEchoReplyType)

It might be reasonable to receive an ICMP message from our host about something else, other than an ICMP response, so I don' t think this should error out.

[rablador]

Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @acb-mv)

[buggmagnet]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @acb-mv)

---

ios/PacketTunnelCore/Pinger/ICMP.swift line 126 at r2 (raw file):

    }

    // swiftlint:disable:next file_length

⚠️ Superfluous Disable Command Violation: SwiftLint rule 'file_length' did not trigger a violation in the disabled region; remove the disable command (superfluous_disable_command)

[acb-mv]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @pinkisemils and @rablador)

---

ios/PacketTunnelCore/Pinger/ICMP.swift line 26 at r2 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

We should log, thus also capture, the unexpected value. This more or less applies to most other error types about unexpected values.

I'll add it to the echo reply type. The "too small" values are probably not worth bothering with, as meaningful diagnostic data would be the entire malformed packet (which would add overhead, and be better obtained with a debugger). As for IP version, if that mismatches then we either have an IPv6 header that somehow snuck through or a malformed structure altogether, so I'm not sure if there's much point logging the IP version value in the error structure.

---

ios/PacketTunnelCore/Pinger/ICMP.swift line 86 at r2 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

It might be reasonable to receive an ICMP message from our host about something else, other than an ICMP response, so I don' t think this should error out.

I have moved this check to the caller.

[acb-mv]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @buggmagnet, @pinkisemils, and @rablador)

---

ios/PacketTunnelCore/Pinger/ICMP.swift line 126 at r2 (raw file):

Previously, buggmagnet wrote…

⚠️ Superfluous Disable Command Violation: SwiftLint rule 'file_length' did not trigger a violation in the disabled region; remove the disable command (superfluous_disable_command)

Done.

[pinkisemils]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @buggmagnet)