Implement reuse detection for cipher

[oif]

Use bloom filter to withstand reuse 'attack'.

Same implementation as https://github.com/shadowsocks/shadowsocks-libev/blob/v3.3.4/src/ppbloom.c

[oif]

@riobard re-implement saltfilter and support change filter entries and FPS via ldflags(but saltfilter slot count fixed). And wait riobard/go-bloom#1 merged to modify go.mod

[oif]

pardon me, I should post the general purpose of this PR.

1. Implements a shared Bloom filter in saltfilter package and expose necessary function(Check and Test);
2. Ciphers use this shared filter directly without initialize it manually or manage it alone.

[riobard]

Please make salfilter an internal package. I don't want to export any of its members to avoid outside use.

[oif]

@riobard How about create a github.com/shadowsocks/go-shadowsocks2/internal package and put filter inside?

[riobard]

I guess that's the way to do it.

[riobard]

Is there anyway to change the value from the command line flags?

[oif]

Absolutely, but I thought about this: low-level flags maybe more suitable to expose to people who known this what to do and how to modify(to make it works as expect), and keep user-end flags as Shadowsocks well known provides.

[riobard]

Makes sense. Maybe we could change it via environment variables?

[oif]

A good idea as well, with SHADOWSOCKS_ prefix or not?

[oif]

• SHADOWSOCKS_SF_CAPACITY
• SHADOWSOCKS_SF_FPR
• SHADOWSOCKS_SF_SLOT

[riobard]

Looks good to me. I'm debating if we should default the filter on or off. Maybe off by default?

[oif]

In my view, it's better to be enabled by default as it can 'protects' connection by withstand reuse detection.

[riobard]

Makes sense.

[oif]

and also suppose public errors like var ErrShortPacket = errors.New("short packet") should define in one place(maybe internal or errors)

[riobard]

What do you mean? It's already exported.

[oif]

errors declared both in aead and steam package, it’s not necessary and hard to handle outside as well

[riobard]

We will remove shadowstream package soon (I've already removed it from my fork), so there's no need to worry about that.

[oif]

@riobard OK, then just keep those declaration

[riobard]

We should add the necessary documentation.

[oif]

@riobard yep, I'll create a new PR about this soon.

[fortuna]

Because this large allocation is done on init(), anyone depending on go-shadowsocks2 will have this allocation done, even client code on memory constrained devices. This caused an issue on the Outline iOS client, where the VpnExtension has a very tight memory limit.

Instead, this could be done on main and injected into the code that needs it, or initialized on demand.

[oif]

Good point, client needs fewer capacity so maybe provide a manual initialization by giving two kind of capacity, slot and FPR which are identitified by running mode (client/server) to reduce large allocations is a good chooice.

What do you think? @riobard

[oif]

And on the other hand, saltfilter provide enough ENV to let caller or executor to controll related args while initializing, it's another way to controll allocation directly.

[fortuna]

I sent #189 to fix it.

The ENV is not appropriate. Who sets it? Setting it on main leaks implementation details. Setting in a library is messy, since libraries can compete. For example, if client code disables it, we'll have a problem with an integration test that runs both the client and the server code.

The ideal solution would be to inject dependencies instead of using globals, but the code is not designed for injection (doesn't use classes). I implemented a compromise where I get a singleton object when needed. That can be injected later if wiring is made easier.

[riobard]

I agree with @fortuna that the filter should be optional. It was an oversight that the filter is created in init(), causing problems for importing go-ss2 as a library.

Please move to #189 for further discussion about the fix.