pires / go-proxyproto

A Go library implementation of the PROXY protocol, versions 1 and 2.
Apache License 2.0
478 stars 107 forks source link

tests and fixes for issue #69 #71

Closed isedev closed 3 years ago

isedev commented 3 years ago

2As suggested in PR#70, a new pull request which captures just the tests proving issue #69, with some refinements:

The second issue above can be used for DoS by consuming server process sockets.

Will then submit fixes - further testing shows that pull request #70 does not solve the problem properly.

Fixes #69

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-94.2%) to 0.0% when pulling a368adcec57657ab34c0b53c8f771f2224cc5d30 on isedev:feature/issue-69-tests into c4bcea2ffbdf5f83639a15da7cd3b3e1438a29cc on pires:main.

isedev commented 3 years ago

Added the new fix. Let me know what you think.

pires commented 3 years ago

cc @emersion @mschneider82 @TimWolla @jefferai (I wish I remembered everyone's name/handle). Someone at Snyk is also willing to help getting a CVE for this which will be a first for me. Thanks everybody out there paying attention and willing to help.

TimWolla commented 3 years ago

Thanks for the ping! I'm afraid I won't be much of a help here, though. I'm not even a go programmer. My contribution regarding the unique ID TLV was motivated by the fact that I was the person that added this TLV to the PROXYv2 spec (I'm somewhat involved with upstream HAProxy) and I just searched for proxy protocol libraries to make sure they are up to date.

jefferai commented 3 years ago

@pires Thanks for pinging me! This generally looks good, left a couple of comments.

isedev commented 3 years ago

The code mostly LGTM. Other than two comments I kindly request you tidy up the commit log.

Any suggestions how I tidy up the commit log? I agree needs to be done, just no idea how to without new pull request. If that's what it takes, more than happy to.

TimWolla commented 3 years ago

Any suggestions how I tidy up the commit log?

Use an interactive rebase git rebase -i and then force push to isedev:feature/issue-69-tests.

isedev commented 3 years ago

Any suggestions how I tidy up the commit log?

Use an interactive rebase git rebase -i and then force push to isedev:feature/issue-69-tests.

Thanks. Learn something new everyday :)

mschneider82 commented 3 years ago

@pires can you tag a new release?

pires commented 3 years ago

Working on it. Want to get Go 1.16 in and some fixes to tests I spotted while doing so.

mschneider82 commented 3 years ago

i do a pull req for traefik, where i used this lib to provide proxy protocol support, but traefik still depends on go 1.15, so requiring 1.16 here would break it. I will stick on last commit for now

pires commented 3 years ago

There's no such thing as enforcing Go 1.16 requirement. CI compiles and tests for Go 1.14 and Go 1.15, and now want to add 1.16, that's all.

mschneider82 commented 3 years ago

Ok fine. The traefik maintainers requested a new version tag

pires commented 3 years ago

OK the tests fixing will have to wait as I need more time to find a reasonable strategy while also refactoring to reduce lots of duplicated code.

pires commented 3 years ago

https://github.com/pires/go-proxyproto/releases/tag/v0.5.0