feat: add User/Pass Authentication to Sock5 Dialer with Tests #189
Conversation
This reverts commit e793764.
|
@fortuna I will create a separate PR for config package changes to accept config URL with username/password for socks5 after this PR is merged otherwise the config package PR breaks the build. |
transport/socks5/stream_dialer.go
Outdated
| } | ||
|
|
||
| // SetUsername sets the username field, ensuring it doesn't exceed 255 bytes in length and is at least 1 byte. | ||
| func (c *Credentials) SetUsername(username string) error { |
There was a problem hiding this comment.
Let's nave a NewCredentials(username, password) (Credentials, error) instead. It's easier to use.
transport/socks5/stream_dialer.go
Outdated
| // +----+--------+ | ||
| // | 1 | 1 | | ||
| // +----+--------+ | ||
| var subNegotiation [2]byte |
transport/socks5/stream_dialer.go
Outdated
| return nil, fmt.Errorf("invalid protocol version %v. Expected 5", methodResponse[0]) | ||
| } | ||
| if methodResponse[1] == 2 { | ||
| // 2. Read sub-negotiation version and status |
There was a problem hiding this comment.
Sub-negotiation is a bad name.
Use authentication response/reply instead.
transport/socks5/stream_dialer.go
Outdated
| // +----+-----+-------+------+----------+----------+ | ||
| // | 1 | 1 | X'00' | 1 | Variable | 2 | | ||
| // +----+-----+-------+------+----------+----------+ | ||
| var connectResponse [4]byte |
Co-authored-by: Vinicius Fortuna <[email protected]>
|
@fortuna Thanks again for the feedback! I applied your comments; Could you please review this again for me? Btw, |
|
@fortuna also windows build failed again but I recall you mentioned it is due to some race condition. It sometimes pass for me and sometimes fail even though I am not making any breaking changes... |
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
Co-authored-by: Vinicius Fortuna <[email protected]>
fortuna
changed the title
transport/socks5/stream_dialer.go
Outdated
| @@ -23,26 +23,54 @@ import ( | |||
| "github.com/Jigsaw-Code/outline-sdk/transport" | |||
| ) | |||
|
|
|||
| // https://datatracker.ietf.org/doc/html/rfc1929 | |||
| // Credentials can be nil, and that means no authentication. | |||
| type Credentials struct { | |||
This PR adds username/password authentication to Socks5 dialer. Changes to config package will come in a separate PR to avoid breaking build checks. The config URL for socks5 with authentication would like this:
For testing I am currently using this socks5 server implementation.
TODO