-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
Implement support for Wireguard in PIA #1836
base: master
Are you sure you want to change the base?
Conversation
I'm willing to test this. Already built the image but what should I put in my compose file? |
You should be able to set your account credentials in the openvpn user credentials, and set |
ERROR VPN settings: provider settings: VPN provider name is not valid for Wireguard: value is not one of the possible choices: private internet access must be one of airvpn, custom, ivpn, mullvad, nordvpn, surfshark or windscrib. This is what I get. If I put custom instead of PIA I get the error saying no endpoint set |
I've fixed this in the latest commit, please build and try it again. |
Now I get this: ERROR [vpn] finding a VPN server: filtering servers: no server found: for VPN wireguard; protocol udp; region dk copenhagen; encryption preset strong |
Hi, please try again, this should now be fixed! |
How do I know the right thing to put in SERVER_REGIONS= ? |
Anything that works with OpenVPN should most likely work with Wireguard too. |
It doesn't. Must likely it's searching on servers.json for a wireguard server but PIA only has ovpn server in the built in server list. https://raw.githubusercontent.com/qdm12/gluetun/master/internal/storage/servers.json (builtin server list) |
Could you give it a go again? I managed to reproduce those errors without an account, and each of them should be fixed now. You may need to disable the firewall with |
almost there. ERROR [vpn] getting Wireguard connection: HTTP status code is not OK: https://www.privateinternetaccess.com/gtoken/generateToken: 403 403 Forbidden: response received: <title>403 Forbidden</title> 403 Forbiddencloudflare </html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
Unfortunately this requires connectivity before the VPN is up, and that's a deal breaker for Gluetun. I'm slowly working towards a solution but this is far from done. The PR won't be useless though, it will be used at some point in the future 👍 !
result := replaceInString(testCase.s, testCase.substitutions) | ||
result := ReplaceInString(testCase.s, testCase.substitutions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please keep this unexported (lowercase first letter), there is no need to export it
// replaceInErr is used to remove sensitive information from errors. | ||
func ReplaceInErr(err error, substitutions map[string]string) error { | ||
s := ReplaceInString(err.Error(), substitutions) | ||
return errors.New(s) //nolint:goerr113 | ||
} | ||
|
||
// replaceInString is used to remove sensitive information. | ||
func ReplaceInString(s string, substitutions map[string]string) string { | ||
for old, new := range substitutions { | ||
s = strings.ReplaceAll(s, old, new) | ||
} | ||
return s | ||
} | ||
|
||
func makeNOKStatusError(response *http.Response, substitutions map[string]string) (err error) { | ||
url := response.Request.URL.String() | ||
url = ReplaceInString(url, substitutions) | ||
|
||
b, _ := io.ReadAll(response.Body) | ||
shortenMessage := string(b) | ||
shortenMessage = strings.ReplaceAll(shortenMessage, "\n", "") | ||
shortenMessage = strings.ReplaceAll(shortenMessage, " ", " ") | ||
shortenMessage = ReplaceInString(shortenMessage, substitutions) | ||
|
||
return fmt.Errorf("%w: %s: %d %s: response received: %s", | ||
ErrHTTPStatusCodeNotOK, url, response.StatusCode, | ||
response.Status, shortenMessage) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move this back to original location; you can do a subsequent PR to move these around after for cleanup if necessary, but let's keep this PR focused on what it's supposed to do.
|
||
func (p *Provider) GetWireguardConnection(ctx context.Context, connection models.Connection, wireguardSettings settings.Wireguard, ipv6Supported bool) (settings wireguard.Settings, err error) { | ||
// create http client to make requests to the server's API | ||
client, err := newHTTPClient(connection.Hostname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handle the error
client, err := newHTTPClient(connection.Hostname) | |
client, err := newHTTPClient(connection.Hostname) | |
if err != nil { | |
return fmt.Errorf("creating http client: %w", err) | |
} |
) | ||
|
||
func (p *Provider) GetWireguardConnection(ctx context.Context, connection models.Connection, wireguardSettings settings.Wireguard, ipv6Supported bool) (settings wireguard.Settings, err error) { | ||
// create http client to make requests to the server's API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit comment unneeded
// create http client to make requests to the server's API |
// create http client to make requests to the server's API | ||
client, err := newHTTPClient(connection.Hostname) | ||
|
||
// generate a new private key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit comment unneeded
// generate a new private key |
@@ -1 +1,2 @@ | |||
scratch.txt | |||
.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove before merging
@@ -0,0 +1,72 @@ | |||
package privateinternetaccess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename file to token.go
, avoid utils
in filename or directory, it's rather meaningless
func fetchToken(ctx context.Context, client *http.Client, | ||
tokenType string, authFilePath string) (token string, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is used outside openvpn-only, we should inject the username and password and not use the openvpn auth file anymore (especially if it's for Wireguard, that makes it super strange)
case "gtoken": | ||
path = "/gtoken/generateToken" | ||
default: | ||
return "", fmt.Errorf("token type %q is not supported", tokenType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could even panic here, since that would be a programming error:
return "", fmt.Errorf("token type %q is not supported", tokenType) | |
panic(fmt.Sprintf("token type %q is not supported", tokenType)) |
publicKey := privateKey.PublicKey() | ||
|
||
// fetch token from PIA's API | ||
token, err := fetchToken(ctx, client, "gtoken", p.authFilePath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's also why I did not implement this yet, but don't worry, your code will be used at some point.
I'm working on a few other moving parts, and the idea is that connecting to privateinternetaccess.com over https (and its dns resolution) would be the only one allowed outside the vpn, temporarily. But this is far from done, so expect this pull request to stay opened for a few months unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theoretically, if the agent were to connect via openvpn over a sideband, download the necessary wireguard configs over the ovpn tunnel, then bring up the primary wireguard tunnel using the downloaded wireguard credentials, would that address the leakage risk?
Obviously the scenario assumes all tunnelled traffic is only routed over wireguard, and thus is blocked till the wireguard tunnel is up. I would assume this should be the default behaviour anyways in the event of the PIA dropping the wireguard tunnel to rotate IPs.
7793495
to
a194906
Compare
any way I could help to get support added for PIA wireguard? |
57a0645
to
059b128
Compare
bba6a9f
to
23b0320
Compare
Is there a chance that this will be implemented somewhen? I would love to have that feature. |
Note you can already use it with the custom provider and wireguard. Not super easy, but it still relatively easy to do, and quite a few users are already doing this. This is still blocked by #137 which will get merged after release v3.39.0 |
Closes #612
Untested, as I don't have a PIA account
Warning: I don't really write a lot of Go code, so the structure may be bad!