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

Amnezia PoC #1415

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Amnezia PoC #1415

wants to merge 7 commits into from

Conversation

jmwample
Copy link
Contributor

@jmwample jmwample commented Oct 28, 2024

PR allowing for drop in replacement of wireguard-go with amneziawg.

In total this is pretty straight-forward - the amnezia lib uses the same external interface but has extra (optional) arguments that are added as part of the String configuration passed in on initialization. So we swap out the library paths and then add in the options and things just work.

For now, this PR is a client change only. And as none of the servers support the more advanced features of amnezia, we cannot use handshake (init or response) message padding or header remapping yet.

All parameters should be the same between Client and Server, except Jc - it can vary.

- Jc — (1 ≤ Jc ≤ 128); recommended range is from 3 to 10 inclusive
- Jmin — (Jmin < Jmax); recommended value is 50
- Jmax — ( Jmin < Jmax ≤ 1280); recommended value is 1000
- S1 — (S1 < 1280; S1 + 56 ≠ S2); recommended range is from 15 to 150 inclusive
- S2 — (S2 < 1280); recommended range is from 15 to 150 inclusive
- H1/H2/H3/H4 — must be unique among each other;

For the Parameters that we can set there is no requirement that they match or even stay constant. So we can hard code them in for now, or (if we want) randomize them on the client on connection startup using amnezia.

What do these parameters do?

  • jc - enables junk packets preceding the handshake init message
  • jmin - sets the lower bound for the size of junk packets
  • jmax - sets the upper bound for the size of junk packets

How does Amnezia actually differ from wireguard?

  1. Junk packets can be send PRECEDING the Handshake init - the number and length of these packets is configurable.
  2. Junk bytes can be PREPENDED to the handshake init message.
  3. Junk bytes can be PREPENDED to the handshake response message.
  4. The four Header type values can be remapped to alternate values.

Changes 2, 3, and 4 REQUIRE both client and server to run AmneziaWG with matching configuration options S1, S2, H1, H2, H3, and H4.

General Question(s)

How do we want to make this available?

  • library option that can be enabled in settings in the app
  • separate beta build
  • auto-detect locale and try amnezia
  • other?

This change is Reviewable

IOS_BUILD=false
DOCKER_BUILD=true
AMNEZIA_BUILD=false
while true; do
Copy link
Contributor

@pronebird pronebird Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Been meaning to do that. We should extract args parsing into main :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can split this out into it's own little PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR created here - #1432

I think it now works for the enabled build platforms

@jmwample
Copy link
Contributor Author

I am not sure why the ios build is not finding libwg in /build/lib/aarch64-apple-ios. It could be that I missed a path replacement or implied lib name in wireguard/build-wureguard-go.sh somewhere.

@pronebird
Copy link
Contributor

pronebird commented Oct 29, 2024

I am not sure why the ios build is not finding libwg in /build/lib/aarch64-apple-ios. It could be that I missed a path replacement or implied lib name in wireguard/build-wureguard-go.sh somewhere.

Try ls -la /build/lib and see what's in there

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 26 files reviewed, 1 unresolved discussion (waiting on @jmwample)


nym-vpn-core/crates/nym-vpn-lib/build.rs line 16 at r1 (raw file):

        .expect("failed to extract build metadata");

    let manifest_path = env::var_os("CARGO_MANIFEST_DIR").expect("manifest dir is not set");

FYI Maybe you need to rebase on main because crates/nym-wg-go already provides the linker flags, so there is no need to modify build.rs for individual targets.

jmwample and others added 2 commits October 30, 2024 16:01
basic structs and feature for amnezia configuration - libs still build
Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 41 files reviewed, all discussions resolved (waiting on @aniampio)


wireguard/build-wireguard-go.sh line 17 at r1 (raw file):

Previously, jmwample (Jack Wampler) wrote…

PR created here - #1432

I think it now works for the enabled build platforms

Thank you for that!

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 41 files reviewed, 1 unresolved discussion (waiting on @aniampio and @jmwample)

a discussion (no related file):
Do you think there is a way we can maintain the same libwg codebase without copying all code and support amnesia at the same time?

I am not very familiar with Go toolchain. I know there things like tags for conditional compilation.

For imports, it seems that Amnezia is merely a mirror of golang.zx2c4.com/wireguard repo so maybe we could rewrite the repo URL to amnezia in go.mod while on the outside maintain the same imports from zx2c4.com.

I am not sure what the best way to tackle this, but I'd like to avoid copying the same code for as much as possible because there will be patches to the libwg and the last thing we want is to sync between libwg and amnezia fork, if possible of course.


@jmwample
Copy link
Contributor Author

jmwample commented Oct 31, 2024

Do you think there is a way we can maintain the same libwg codebase without copying all code and support amnesia at the same time?

This is what I tried to do first. However, I ran into trouble because the go replace directive for go.mod only allows you to replace a library with a library with the same name ("wireguard-go" != "amneziawg-go"). At the same time, the golang compiler doesn't have a pre-processor, so you can't use tags to swap library paths like you can in rust -- build tags are used to swap out entire files.

So we could use build tags and only copy the files with references to golang.zx2c4.com, or we could use a patch script and search/replace those import paths. OR we could just use the amnezia library and get rid of the wireguard-go version since it is a fork with minimal changes that by default does the exact same thing.

Only using amneziawg-go makes this PR even simpler because then the change is purely library usage, there is no build script option necessary anymore.

Edit: #1441 is an implementation of the amneziawg-go only option described above for comparison.

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 20 files at r2.
Reviewable status: 1 of 54 files reviewed, 3 unresolved discussions (waiting on @aniampio and @jmwample)


nym-vpn-core/crates/nym-vpnd/build.rs line 8 at r3 (raw file):

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let manifest_path = env::var_os("CARGO_MANIFEST_DIR").expect("manifest dir is not set");

Can we please revert these changes. nym-wg-go already does that.

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 54 files reviewed, 4 unresolved discussions (waiting on @aniampio and @jmwample)


.github/workflows/ci-nym-vpn-core-ios.yml line 49 at r3 (raw file):

          repo-token: ${{ secrets.GITHUB_TOKEN }}

      - name: Install script dependencies

I guess we no longer need it

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 54 files reviewed, 5 unresolved discussions (waiting on @aniampio and @jmwample)


wireguard/build-wireguard-go.sh line 262 at r3 (raw file):

}

LIB_DIR="libwg"

LIB_DIR is defined at the top. Rebasing issues?

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 54 files reviewed, 6 unresolved discussions (waiting on @aniampio and @jmwample)


nym-vpn-core/crates/nym-gateway-probe/README.md line 12 at r3 (raw file):

preffered platform.

Install required dependencies

Nit: this should really be a part of markdown document in the root of the nym-vpn-core because it defines general requirements for the entire project. Except maybe clang?

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 20 files at r2, 1 of 37 files at r3.
Reviewable status: 3 of 54 files reviewed, 5 unresolved discussions (waiting on @aniampio and @jmwample)

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 37 files at r3.
Reviewable status: 4 of 54 files reviewed, 5 unresolved discussions (waiting on @aniampio and @jmwample)

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 37 files at r3.
Reviewable status: 5 of 54 files reviewed, 5 unresolved discussions (waiting on @aniampio and @jmwample)

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 26 files at r1, 10 of 20 files at r2, 1 of 37 files at r3.
Reviewable status: 17 of 54 files reviewed, 7 unresolved discussions (waiting on @aniampio and @jmwample)


.github/workflows/ci-nym-vpn-core-macos.yml line 49 at r3 (raw file):

      - name: Install script dependencies
        run: brew install gnu-getopt

Same here


nym-vpn-core/crates/nym-gateway-probe/netstack_ping/go.mod line 5 at r3 (raw file):

go 1.22.3

toolchain go1.23.1

It looks like toolchain is set to go1.23 but go itself is still at 1.22. I am certain what that means in practice.

Copy link
Contributor

@pronebird pronebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 20 files at r2, 33 of 37 files at r3, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @aniampio and @jmwample)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants