From 439da9c09f89805087164ed87b4d57800e10de80 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala <joohoi@users.noreply.github.com> Date: Thu, 15 Mar 2018 00:23:55 +0200 Subject: [PATCH] Properly parse r.RemoteAddr (#50) * Properly parse r.RemoteAddr * Add tests, and fix net.ParseCIDR issues with IPv6 addresses enclosed in brackets --- acmetxt.go | 4 ++-- auth.go | 8 +++++++- auth_test.go | 33 +++++++++++++++++++++++++++++++++ util.go | 6 ++++++ 4 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 auth_test.go diff --git a/acmetxt.go b/acmetxt.go index 7b20c02..95719d1 100644 --- a/acmetxt.go +++ b/acmetxt.go @@ -33,9 +33,9 @@ func (c *cidrslice) JSON() string { func (c *cidrslice) ValidEntries() []string { valid := []string{} for _, v := range *c { - _, _, err := net.ParseCIDR(v) + _, _, err := net.ParseCIDR(sanitizeIPv6addr(v)) if err == nil { - valid = append(valid, v) + valid = append(valid, sanitizeIPv6addr(v)) } } return valid diff --git a/auth.go b/auth.go index 99c3035..162f927 100644 --- a/auth.go +++ b/auth.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "net" "net/http" "github.com/julienschmidt/httprouter" @@ -83,5 +84,10 @@ func updateAllowedFromIP(r *http.Request, user ACMETxt) bool { ips := getIPListFromHeader(r.Header.Get(Config.API.HeaderName)) return user.allowedFromList(ips) } - return user.allowedFrom(r.RemoteAddr) + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + log.WithFields(log.Fields{"error": err.Error(), "remoteaddr": r.RemoteAddr}).Error("Error while parsing remote address") + host = "" + } + return user.allowedFrom(host) } diff --git a/auth_test.go b/auth_test.go new file mode 100644 index 0000000..911b4f7 --- /dev/null +++ b/auth_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "net/http" + "testing" +) + +func TestUpdateAllowedFromIP(t *testing.T) { + userWithAllow := newACMETxt() + userWithAllow.AllowFrom = cidrslice{"192.168.1.2/32", "[::1]/128"} + userWithoutAllow := newACMETxt() + + for i, test := range []struct { + remoteaddr string + expected bool + }{ + {"192.168.1.2:1234", true}, + {"192.168.1.1:1234", false}, + {"invalid", false}, + {"[::1]:4567", true}, + } { + newreq, _ := http.NewRequest("GET", "/whatever", nil) + newreq.RemoteAddr = test.remoteaddr + ret := updateAllowedFromIP(newreq, userWithAllow) + if test.expected != ret { + t.Errorf("Test %d: Unexpected result for user with allowForm set", i) + } + + if !updateAllowedFromIP(newreq, userWithoutAllow) { + t.Errorf("Test %d: Unexpected result for user without allowForm set", i) + } + } +} diff --git a/util.go b/util.go index 46cf64d..dcd1d15 100644 --- a/util.go +++ b/util.go @@ -38,6 +38,12 @@ func sanitizeString(s string) string { return re.ReplaceAllString(s, "") } +func sanitizeIPv6addr(s string) string { + // Remove brackets from IPv6 addresses, net.ParseCIDR needs this + re, _ := regexp.Compile("[\\[\\]]+") + return re.ReplaceAllString(s, "") +} + func generatePassword(length int) string { ret := make([]byte, length) const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890-_" -- GitLab