Skip to content

Commit d39a98c

Browse files
author
observatory
committed
x509util: fix SSRF hostname bypass in rejectPrivateHost()
PR google#1761 introduced rejectPrivateHost() to block SSRF via private IP addresses, but only checked net.ParseIP() which returns nil for hostnames. DNS hostnames resolving to private or loopback addresses bypassed the fix entirely. This change resolves hostnames via net.LookupHost() before IP validation, blocking bypasses such as: - localhost → 127.0.0.1 (loopback) - metadata.google.internal → 169.254.169.254 (GCP) - kubernetes.default.svc → 10.x.x.x (k8s) Also adds: - rejectPrivateHost() to ReadFileOrURL() and GetIssuer() - defer rsp.Body.Close() to prevent resource leaks - Tests for hostname bypass and SSRF protection Fixes incomplete fix in PR google#1761 (Issue google#1759)
1 parent dc95553 commit d39a98c

2 files changed

Lines changed: 113 additions & 0 deletions

File tree

x509util/files.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"encoding/pem"
1919
"fmt"
2020
"io"
21+
"net"
2122
"net/http"
2223
"net/url"
2324
"os"
@@ -26,6 +27,38 @@ import (
2627
"github.com/google/certificate-transparency-go/x509"
2728
)
2829

30+
// rejectPrivateHost returns an error if the URL host resolves
31+
// to a loopback, link-local, or private IP address.
32+
func rejectPrivateHost(u *url.URL) error {
33+
host := u.Hostname()
34+
ip := net.ParseIP(host)
35+
if ip == nil {
36+
addrs, err := net.LookupHost(host)
37+
if err != nil {
38+
return fmt.Errorf("failed to resolve host %q: %v", host, err)
39+
}
40+
for _, addr := range addrs {
41+
resolved := net.ParseIP(addr)
42+
if resolved == nil {
43+
continue
44+
}
45+
if resolved.IsLoopback() || resolved.IsLinkLocalUnicast() ||
46+
resolved.IsLinkLocalMulticast() || resolved.IsPrivate() {
47+
return fmt.Errorf(
48+
"refusing URL: hostname %q resolves to private/loopback IP %q",
49+
host, addr)
50+
}
51+
}
52+
return nil
53+
}
54+
if ip.IsLoopback() || ip.IsLinkLocalUnicast() ||
55+
ip.IsLinkLocalMulticast() || ip.IsPrivate() {
56+
return fmt.Errorf(
57+
"refusing to fetch URL with private/loopback host: %q", u.String())
58+
}
59+
return nil
60+
}
61+
2962
// ReadPossiblePEMFile loads data from a file which may be in DER format
3063
// or may be in PEM format (with the given blockname).
3164
func ReadPossiblePEMFile(filename, blockname string) ([][]byte, error) {
@@ -45,10 +78,18 @@ func ReadPossiblePEMURL(target, blockname string) ([][]byte, error) {
4578
return ReadPossiblePEMFile(target, blockname)
4679
}
4780

81+
u, err := url.Parse(target)
82+
if err != nil {
83+
return nil, fmt.Errorf("failed to parse URL %q: %v", target, err)
84+
}
85+
if err := rejectPrivateHost(u); err != nil {
86+
return nil, err
87+
}
4888
rsp, err := http.Get(target)
4989
if err != nil {
5090
return nil, fmt.Errorf("failed to http.Get(%q): %v", target, err)
5191
}
92+
defer rsp.Body.Close()
5293
data, err := io.ReadAll(rsp.Body)
5394
if err != nil {
5495
return nil, fmt.Errorf("failed to io.ReadAll(%q): %v", target, err)
@@ -84,10 +125,14 @@ func ReadFileOrURL(target string, client *http.Client) ([]byte, error) {
84125
return os.ReadFile(target)
85126
}
86127

128+
if err := rejectPrivateHost(u); err != nil {
129+
return nil, err
130+
}
87131
rsp, err := client.Get(u.String())
88132
if err != nil {
89133
return nil, fmt.Errorf("failed to http.Get(%q): %v", target, err)
90134
}
135+
defer rsp.Body.Close()
91136
return io.ReadAll(rsp.Body)
92137
}
93138

@@ -99,6 +144,16 @@ func GetIssuer(cert *x509.Certificate, client *http.Client) (*x509.Certificate,
99144
return nil, nil
100145
}
101146
issuerURL := cert.IssuingCertificateURL[0]
147+
u, err := url.Parse(issuerURL)
148+
if err != nil {
149+
return nil, fmt.Errorf("invalid issuer URL %q: %v", issuerURL, err)
150+
}
151+
if u.Scheme != "http" && u.Scheme != "https" {
152+
return nil, fmt.Errorf("unsupported scheme in issuer URL %q", issuerURL)
153+
}
154+
if err := rejectPrivateHost(u); err != nil {
155+
return nil, err
156+
}
102157
rsp, err := client.Get(issuerURL)
103158
if err != nil || rsp.StatusCode != http.StatusOK {
104159
return nil, fmt.Errorf("failed to get issuer from %q: %v", issuerURL, err)

x509util/files_ssrf_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package x509util
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"net/url"
7+
"testing"
8+
)
9+
10+
func TestRejectPrivateHost(t *testing.T) {
11+
tests := []struct {
12+
url string
13+
wantErr bool
14+
}{
15+
{"http://169.254.169.254/latest/meta-data/", true},
16+
{"http://127.0.0.1/secret", true},
17+
{"http://192.168.1.1/admin", true},
18+
{"http://10.0.0.1/internal", true},
19+
{"http://[::1]/secret", true},
20+
{"http://localhost/secret", true},
21+
{"http://example.com/public", false},
22+
{"http://8.8.8.8/dns", false},
23+
}
24+
25+
for _, tt := range tests {
26+
u, _ := url.Parse(tt.url)
27+
err := rejectPrivateHost(u)
28+
if (err != nil) != tt.wantErr {
29+
t.Errorf("rejectPrivateHost(%q) error=%v wantErr=%v",
30+
tt.url, err, tt.wantErr)
31+
}
32+
}
33+
}
34+
35+
func TestReadFileOrURL_SSRFBypass(t *testing.T) {
36+
srv := httptest.NewServer(
37+
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
38+
w.Write([]byte("SECRET_INTERNAL_DATA"))
39+
}))
40+
defer srv.Close()
41+
42+
client := &http.Client{}
43+
localhostURL := "http://localhost:" +
44+
srv.Listener.Addr().String()[len("127.0.0.1:"):] + "/secret"
45+
46+
_, err := ReadFileOrURL(localhostURL, client)
47+
if err == nil {
48+
t.Errorf("ReadFileOrURL should reject localhost URL: %s", localhostURL)
49+
}
50+
}
51+
52+
func TestReadFileOrURL_PublicURL(t *testing.T) {
53+
client := &http.Client{}
54+
_, err := ReadFileOrURL("https://example.com/", client)
55+
if err != nil {
56+
t.Skipf("network unavailable: %v", err)
57+
}
58+
}

0 commit comments

Comments
 (0)