Skip to content

Commit 07aeecd

Browse files
committed
x509util: harden SSRF guard against DNS rebinding and HTTP redirects
rejectPrivateHost only checked literal IP addresses in the URL. Two bypasses remained: 1. A hostname resolving to a private/loopback IP (e.g. evil.com → 127.0.0.1) was not caught because net.ParseIP returns nil for hostnames. 2. An HTTP redirect from a public host to a private IP was followed by the default http.Client without re-checking the target. Fix: introduce safeTransport (custom DialContext that resolves and validates all IPs before connecting) and rejectPrivateRedirect (CheckRedirect hook that blocks redirects to private literal IPs). All three URL-fetching helpers (ReadPossiblePEMURL, ReadFileOrURL, GetIssuer) now use safeClient which combines both guards. Add tests for redirect-to-loopback and localhost-hostname scenarios.
1 parent b06af35 commit 07aeecd

2 files changed

Lines changed: 89 additions & 6 deletions

File tree

x509util/files.go

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package x509util
1616

1717
import (
18+
"context"
1819
"encoding/pem"
1920
"fmt"
2021
"io"
@@ -27,20 +28,75 @@ import (
2728
"github.com/google/certificate-transparency-go/x509"
2829
)
2930

30-
// rejectPrivateHost returns an error if the URL host is a loopback,
31-
// link-local, or private IP address.
31+
// isPrivateIP reports whether ip is loopback, link-local, or private.
32+
func isPrivateIP(ip net.IP) bool {
33+
return ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate()
34+
}
35+
36+
// rejectPrivateHost returns an error if the URL host is a literal private IP.
37+
// For hostname-based URLs the resolved addresses are checked at dial time by
38+
// safeTransport; this function catches the obvious literal-IP case early.
3239
func rejectPrivateHost(u *url.URL) error {
3340
host := u.Hostname()
3441
ip := net.ParseIP(host)
3542
if ip == nil {
3643
return nil
3744
}
38-
if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() {
45+
if isPrivateIP(ip) {
3946
return fmt.Errorf("refusing to fetch URL with private/loopback host: %q", u.String())
4047
}
4148
return nil
4249
}
4350

51+
// safeTransport returns an *http.Transport whose DialContext resolves the
52+
// hostname and rejects connections to private/loopback addresses. This
53+
// defends against DNS-rebinding and hostname-to-private-IP attacks.
54+
func safeTransport() *http.Transport {
55+
return &http.Transport{
56+
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
57+
host, port, err := net.SplitHostPort(addr)
58+
if err != nil {
59+
return nil, err
60+
}
61+
ips, err := net.DefaultResolver.LookupIPAddr(ctx, host)
62+
if err != nil {
63+
return nil, err
64+
}
65+
for _, ia := range ips {
66+
if isPrivateIP(ia.IP) {
67+
return nil, fmt.Errorf("refusing to connect: %q resolves to private/loopback address %s", host, ia.IP)
68+
}
69+
}
70+
var d net.Dialer
71+
return d.DialContext(ctx, network, net.JoinHostPort(host, port))
72+
},
73+
}
74+
}
75+
76+
// rejectPrivateRedirect is an http.Client CheckRedirect function that blocks
77+
// redirects targeting private/loopback hosts (literal IP or resolved).
78+
func rejectPrivateRedirect(req *http.Request, via []*http.Request) error {
79+
if len(via) >= 10 {
80+
return fmt.Errorf("stopped after 10 redirects")
81+
}
82+
return rejectPrivateHost(req.URL)
83+
}
84+
85+
// safeClient returns an *http.Client that blocks requests and redirects
86+
// targeting private/loopback addresses. If base is non-nil its Timeout and
87+
// Jar are preserved.
88+
func safeClient(base *http.Client) *http.Client {
89+
c := &http.Client{
90+
Transport: safeTransport(),
91+
CheckRedirect: rejectPrivateRedirect,
92+
}
93+
if base != nil {
94+
c.Timeout = base.Timeout
95+
c.Jar = base.Jar
96+
}
97+
return c
98+
}
99+
44100
// ReadPossiblePEMFile loads data from a file which may be in DER format
45101
// or may be in PEM format (with the given blockname).
46102
func ReadPossiblePEMFile(filename, blockname string) ([][]byte, error) {
@@ -68,7 +124,7 @@ func ReadPossiblePEMURL(target, blockname string) ([][]byte, error) {
68124
return nil, err
69125
}
70126

71-
rsp, err := http.Get(target)
127+
rsp, err := safeClient(nil).Get(target)
72128
if err != nil {
73129
return nil, fmt.Errorf("failed to http.Get(%q): %v", target, err)
74130
}
@@ -112,7 +168,7 @@ func ReadFileOrURL(target string, client *http.Client) ([]byte, error) {
112168
return nil, err
113169
}
114170

115-
rsp, err := client.Get(u.String())
171+
rsp, err := safeClient(client).Get(u.String())
116172
if err != nil {
117173
return nil, fmt.Errorf("failed to http.Get(%q): %v", target, err)
118174
}
@@ -140,7 +196,7 @@ func GetIssuer(cert *x509.Certificate, client *http.Client) (*x509.Certificate,
140196
return nil, err
141197
}
142198

143-
rsp, err := client.Get(issuerURL)
199+
rsp, err := safeClient(client).Get(issuerURL)
144200
if err != nil || rsp.StatusCode != http.StatusOK {
145201
return nil, fmt.Errorf("failed to get issuer from %q: %v", issuerURL, err)
146202
}

x509util/files_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
package x509util
1616

1717
import (
18+
"net/http"
19+
"net/http/httptest"
1820
"net/url"
1921
"testing"
2022
)
@@ -48,3 +50,28 @@ func TestRejectPrivateHost(t *testing.T) {
4850
})
4951
}
5052
}
53+
54+
func TestRedirectToLoopback(t *testing.T) {
55+
// Server that redirects to a loopback address.
56+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
57+
http.Redirect(w, r, "http://127.0.0.1/secret", http.StatusFound)
58+
}))
59+
defer srv.Close()
60+
61+
_, err := ReadFileOrURL(srv.URL+"/start", srv.Client())
62+
if err == nil {
63+
t.Fatal("expected error for redirect to loopback, got nil")
64+
}
65+
}
66+
67+
func TestSafeClientBlocksLocalhostHostname(t *testing.T) {
68+
// safeTransport resolves hostnames before connecting. "localhost"
69+
// resolves to 127.0.0.1 / ::1 on every platform, so a request to it
70+
// must be rejected even though rejectPrivateHost (literal-IP check)
71+
// would pass a hostname through.
72+
c := safeClient(nil)
73+
_, err := c.Get("http://localhost:1/should-not-connect")
74+
if err == nil {
75+
t.Fatal("expected error for localhost hostname, got nil")
76+
}
77+
}

0 commit comments

Comments
 (0)