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

[P2P] Basic transport security #544

Closed
12 tasks
bryanchriswhite opened this issue Feb 24, 2023 · 7 comments · Fixed by #598
Closed
12 tasks

[P2P] Basic transport security #544

bryanchriswhite opened this issue Feb 24, 2023 · 7 comments · Fixed by #598
Assignees
Labels
p2p P2P specific changes

Comments

@bryanchriswhite
Copy link
Contributor

bryanchriswhite commented Feb 24, 2023

Objective

Ensure transport-level encryption is enabled and required by default for all network participants.

The initial implementation of transport security should be compatible with the current formulation of V1 identity, this may be as simple as using the crypto helpers introduced in #534 together with go-libp2p's p2p/security/tls package.

Origin Document

image

Goals

  • P2P network peers support, and require, TLS with their counterparts.
  • TLS configuration prefers a GCM variant of AES for symmetric encryption to optimize for speed and security.

Deliverable

  • ...
  • A PR that enables configuring TLS security using hard-coded test fixtures
  • Document the testing methodology in the source code so its repeatable (Bonus: automate)
  • Unit tests for bad/unavailable certificates when turned on (Bonus: E2E tests)

Non-goals / Non-deliverables

  • Configuring remote infrastructure (i.e. DevNet/TestNet) to have certificates automatically available
  • Changing any identity or crypto types and/or interfaces
  • Manipulating X509 certificate(s) / chain(s)

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  1. Setup testing environment, following development docs
  2. docker exec -it node1.consensus ash (see: SO post)
    1. (in container) ip a, note IP address
    2. (in container) cat /sys/class/net/eth0/iflink, note the "interface number"
    3. <Ctrl+d> or exit to exit the container shell
    4. (back on the host) ip link|grep <INTERFACE_NUMBER>, note the interface name, e.g. (if interface number was 55, name would be vetha165c5f@if54):
      55: vetha165c5f@if54
      
  3. Repeat 1.1-1.3 for node2.consensus container
  4. Open wireshark (may need escalated privilege to see desired network interfaces)
  5. Select the interface matching node1.consensus
  6. Filter the captured packets by the IP matching node2.consensus
  7. Look for a packet which has the PSH flag set
  8. Inspect the Data contained in the package to see if any cleartext is readable (e.g. protobuf types: ...type.googleapis.com/consensus.HotstuffMessage...)
  • Task specific tests or benchmarks: make ...
  • New tests or benchmarks: make ...
  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @bryanchriswhite

@bryanchriswhite bryanchriswhite added the p2p P2P specific changes label Feb 24, 2023
@bryanchriswhite bryanchriswhite self-assigned this Feb 24, 2023
@bryanchriswhite bryanchriswhite moved this to Backlog in V1 Dashboard Feb 24, 2023
@Olshansk
Copy link
Member

Love the details in the testing methodology. I added some deliverables/non-goals. Check them out when you have a chance

@jessicadaugherty jessicadaugherty moved this from Backlog to Up Next in V1 Dashboard Mar 13, 2023
@jessicadaugherty
Copy link
Contributor

May be done by default. Please confirm by going through testing methodology @bryanchriswhite

@bryanchriswhite bryanchriswhite changed the title [Libp2p] Basic transport security [P2P] Basic transport security Mar 20, 2023
@bryanchriswhite
Copy link
Contributor Author

bryanchriswhite commented Mar 21, 2023

It looks like this indeed may be taken care of by default when using libp2p. I've updated the testing methodology to reflect the steps I took to produce the packet captures and screenshots below.

  1. The docs do say:

    If no security transport is provided, the host uses the go-libp2p's noise and/or tls encrypted transport to encrypt all traffic;

  2. Packet capturing seems to corroborate this:

Before (packet capture dump)

localnet_clear

After (packet capture dump)

localnet_opaque

@bryanchriswhite bryanchriswhite moved this from Up Next to In Review in V1 Dashboard Mar 21, 2023
@bryanchriswhite bryanchriswhite moved this from In Review to In Progress in V1 Dashboard Mar 21, 2023
@bryanchriswhite
Copy link
Contributor Author

bryanchriswhite commented Mar 21, 2023

Based on packet inspection, it looks like libp2p is preferring noise over TLS (which should be fine) but I haven't dug into that part of libp2p's code yet to see the details for myself.

Outstanding concerns:

  • Ensure that nodes require encrypted transport; i.e. they refuse connections from peers which aren't using encrypted transports

@bryanchriswhite bryanchriswhite linked a pull request Mar 21, 2023 that will close this issue
17 tasks
@bryanchriswhite bryanchriswhite moved this from In Progress to In Review in V1 Dashboard Mar 21, 2023
@Olshansk
Copy link
Member

Olshansk commented Mar 21, 2023

Thanks @bryanchriswhite! Wanted to share a few TILs along with som questions


Packet capturing seems to corroborate this:

TIL the word corroborate 😅


I haven't done packet inspection and according to GPT, it's still the best tool out there. Found it pretty surprising no better UI has come along.

Screenshot 2023-03-21 at 10 50 57 AM


I wasn't familiar with Noise so I read about it for a bit.

Fun fact: just one individual designed the whole thing. http://noiseprotocol.org/noise.html#introduction

  1. Does this use the private key of the node to initiate the handshake before we have a symmetric key?
  2. My understanding is that Noise is a framework rather than a protocol, so is the only difference that we're not leveraging 509 certificates (which we don't have anyhow) in the case of permissionless nodes?

@bryanchriswhite
Copy link
Contributor Author

... Found it pretty surprising no better UI has come along.

I noticed that the version I installed at least uses QT, so at least with respect to aesthetics, it should match the appearance of the desktop environment theme. It looks a bit blander than it should in my screenshots because I had to run it with escalated privileges, which has the effect of applying a different (basic and/or default) theme. With respect to UX, I don't feel qualified to make any assessment as I barely even understand what all the tool is capable of.

Fun fact: just one individual designed the whole thing. http://noiseprotocol.org/noise.html#introduction

🤨 interesting indeed

  1. Does this use the private key of the node to initiate the handshake before we have a symmetric key?

I did some quick digging and it looks like the way libp2p uses noise is as follows:

  1. DefaultSecurity is defined

  2. When the config is applied, the first Security() option function in DefaultSecurity is called, which assigns cfg.SecurityTransports, containing a reference to noise.New

  3. The noise.New constructor is registered with some dependency injection system which I don't yet understand... it's eventually called.

  4. noise.New receives and stores the the pokt private key in the returned Transport's privateKey field.

  5. Via another lengthy execution path, upgrader#setupSecurity() is called, which will call either Transport#SecureInbound or Transport#SecureOutbound.

  6. Both #SecureInbound and #SecureOutbound call newSecureSession(), which copies the reference to the pokt private key from Transport#privateKey to a new secureSession's localkey.

  7. newSecureSession calls secureSession#runHandshake

  8. secureSession#runHandshake generates a new "static keypair" (noise terminology; see: noise spec) but leaves the "ephemeral keypair" empty.

  9. It looks like the pokt key is only subsequently used during to sign the "handshake payload" which the spec describes like so:

    A handshake message consists of some DH public keys followed by a payload. The payload may contain certificates or other data chosen by the application.

  1. My understanding is that Noise is a framework rather than a protocol, so is the only difference that we're not leveraging 509 certificates (which we don't have anyhow) in the case of permissionless nodes?

I believe that's a fairly accurate characterization. I haven't dug into the TLS libp2p security option yet but I would assume that it would be configured (by default) in a non-conventional way such that it's usable outside the context of well-known certificate authorities (permissionless, as you mentioned). I would expect that it generates a basic X509 chain consisting of a single certificate which is self-signed using the pokt keypair, and that signature is what's verified in the TLS handshake to authenticate a remote peer.

My understanding of noise is limited as I haven't had to look into it previously much more than a cursory read of the spec. It does indeed identify itself a "protocol framework" whereas TLS is a protocol in its own right. However, due to the necessarily non-conventional usage of TLS, I would argue that it's more accurate to say that we're using a custom protocol in either case (noise and TLS) but it just happens to be the that that's how noise was specified, whereas TLS is being modified / repurposed to apply to this scenario.

@Olshansk
Copy link
Member

Olshansk commented Apr 6, 2023

@bryanchriswhite Appreciate the thorough research, investigation and response. Makes sense to me and I think it's definitely more than enough for what we need from a practical standpoint. This is good context to have if we ever end up needing to debug and dive deeper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2p P2P specific changes
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants