fix: update proxy-init iptables rule to prevent forwarding loop (#402)

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
Azure · Mar 25, 2022 · 0a2a128 · 0a2a128
1 parent 3e85ce4
commit 0a2a128
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 89 deletions.
diff --git a/docker/proxy.Dockerfile b/docker/proxy.Dockerfile
@@ -24,6 +24,6 @@ FROM --platform=${TARGETPLATFORM:-linux/amd64} gcr.io/distroless/static:nonroot
 WORKDIR /
 COPY --from=builder /workspace/proxy .
 # Kubernetes runAsNonRoot requires USER to be numeric
-USER 65532:65532
+USER 1501:1501
 
 ENTRYPOINT [ "/proxy" ]
diff --git a/init/init-iptables.sh b/init/init-iptables.sh
@@ -3,9 +3,19 @@
 PROXY_PORT=${PROXY_PORT:-8000}
 METADATA_IP=${METADATA_IP:-169.254.169.254}
 METADATA_PORT=${METADATA_PORT:-80}
+PROXY_UID=${PROXY_UID:-1501}
 
-# Forward outbound traffic for metadata endpoint to proxy
-iptables -t nat -A OUTPUT -p tcp -d "${METADATA_IP}" --dport "${METADATA_PORT}" -j REDIRECT --to-port "${PROXY_PORT}"
+iptables -t nat -N AZWI_PROXY_OUTPUT
+iptables -t nat -N AZWI_PROXY_REDIRECT
+
+# Redirect all TCP traffic for metatadata endpoint to the proxy
+iptables -t nat -A AZWI_PROXY_REDIRECT -p tcp -j REDIRECT --to-port "${PROXY_PORT}"
+# For outbound TCP traffic to metadata endpoint on port 80 jump from OUTPUT chain to AZWI_PROXY_OUTPUT chain
+iptables -t nat -A OUTPUT -p tcp -d "${METADATA_IP}" --dport "${METADATA_PORT}" -j AZWI_PROXY_OUTPUT
+# Skip redirection of proxy traffic back to itself, return to next chain for further processing
+iptables -t nat -A AZWI_PROXY_OUTPUT -m owner --uid-owner "${PROXY_UID}" -j ACCEPT
+# For all other traffic to metadata point, jump to AZWI_PROXY_REDIRECT chain
+iptables -t nat -A AZWI_PROXY_OUTPUT -j AZWI_PROXY_REDIRECT
 
 # List all iptables rules
 iptables -t nat --list
diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go
@@ -9,12 +9,10 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/Azure/azure-workload-identity/pkg/version"
 	"github.com/Azure/azure-workload-identity/pkg/webhook"
 
-	"github.com/Azure/go-autorest/autorest/adal"
 	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/confidential"
 	"github.com/go-logr/logr"
 	"github.com/gorilla/mux"
@@ -25,11 +23,6 @@ const (
 	// "/metadata" portion is case-insensitive in IMDS
 	tokenPathPrefix = "/{type:(?i:metadata)}/identity/oauth2/token" // #nosec
 
-	// the format for expires_on in UTC with AM/PM
-	expiresOnDateFormatPM = "1/2/2006 15:04:05 PM +00:00"
-	// the format for expires_on in UTC without AM/PM
-	expiresOnDateFormat = "1/2/2006 15:04:05 +00:00"
-
 	// metadataIPAddress is the IP address of the metadata service
 	metadataIPAddress = "169.254.169.254"
 	// metadataPort is the port of the metadata service
@@ -47,6 +40,22 @@ type proxy struct {
 	logger        logr.Logger
 }
 
+// using this from https://github.com/Azure/go-autorest/blob/b3899c1057425994796c92293e931f334af63b4e/autorest/adal/token.go#L1055-L1067
+// this struct works with the adal sdks used in clients and azure-cli token requests
+type token struct {
+	AccessToken  string `json:"access_token"`
+	RefreshToken string `json:"refresh_token"`
+
+	// AAD returns expires_in as a string, ADFS returns it as an int
+	ExpiresIn json.Number `json:"expires_in"`
+	// expires_on can be in two formats, a UTC time stamp or the number of seconds.
+	ExpiresOn string      `json:"expires_on"`
+	NotBefore json.Number `json:"not_before"`
+
+	Resource string `json:"resource"`
+	Type     string `json:"token_type"`
+}
+
 // NewProxy returns a proxy instance
 func NewProxy(port int, logger logr.Logger) (Proxy, error) {
 	// tenantID is required for fetching a token using client assertions
@@ -144,7 +153,7 @@ func (p *proxy) defaultPathHandler(w http.ResponseWriter, r *http.Request) {
 	_, _ = w.Write(body)
 }
 
-func doTokenRequest(ctx context.Context, clientID, resource, tenantID, authorityHost string) (*adal.Token, error) {
+func doTokenRequest(ctx context.Context, clientID, resource, tenantID, authorityHost string) (*token, error) {
 	tokenFilePath := os.Getenv(webhook.AzureFederatedTokenFileEnvVar)
 	signedAssertion, err := readJWTFromFS(tokenFilePath)
 	if err != nil {
@@ -171,36 +180,15 @@ func doTokenRequest(ctx context.Context, clientID, resource, tenantID, authority
 		return nil, errors.Wrap(err, "failed to acquire token")
 	}
 
-	token := &adal.Token{}
-	token.AccessToken = result.AccessToken
-	token.Resource = resource
-	token.Type = "Bearer"
-	token.ExpiresOn, err = parseExpiresOn(result.ExpiresOn.UTC().Local().Format(expiresOnDateFormat))
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to parse expires_on")
-	}
-	return token, nil
-}
-
-// Vendored from https://github.com/Azure/go-autorest/blob/def88ef859fb980eff240c755a70597bc9b490d0/autorest/adal/token.go
-// converts expires_on to the number of seconds
-func parseExpiresOn(s string) (json.Number, error) {
-	// convert the expiration date to the number of seconds from now
-	timeToDuration := func(t time.Time) json.Number {
-		dur := t.Sub(time.Now().UTC())
-		return json.Number(strconv.FormatInt(int64(dur.Round(time.Second).Seconds()), 10))
-	}
-	if _, err := strconv.ParseInt(s, 10, 64); err == nil {
-		// this is the number of seconds case, no conversion required
-		return json.Number(s), nil
-	} else if eo, err := time.Parse(expiresOnDateFormatPM, s); err == nil {
-		return timeToDuration(eo), nil
-	} else if eo, err := time.Parse(expiresOnDateFormat, s); err == nil {
-		return timeToDuration(eo), nil
-	} else {
-		// unknown format
-		return json.Number(""), err
-	}
+	return &token{
+		AccessToken: result.AccessToken,
+		Resource:    resource,
+		Type:        "Bearer",
+		// There is a difference in parsing between the azure sdks and how azure-cli works
+		// Using the unix time to be consistent with response from IMDS which works with
+		// all the clients.
+		ExpiresOn: strconv.FormatInt(result.ExpiresOn.UTC().Unix(), 10),
+	}, nil
 }
 
 func parseTokenRequest(r *http.Request) (string, string) {

diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go
@@ -9,7 +9,6 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
-	"time"
 
 	"github.com/Azure/azure-workload-identity/pkg/webhook"
 
@@ -155,53 +154,6 @@ func TestRouterPathPrefix(t *testing.T) {
 	}
 }
 
-// Vendored from https://github.com/Azure/go-autorest/blob/def88ef859fb980eff240c755a70597bc9b490d0/autorest/adal/token_test.go
-func TestParseExpiresOn(t *testing.T) {
-	// get current time, round to nearest second, and add one hour
-	n := time.Now().UTC().Round(time.Second).Add(time.Hour)
-	amPM := "AM"
-	if n.Hour() >= 12 {
-		amPM = "PM"
-	}
-	testcases := []struct {
-		Name   string
-		String string
-		Value  int64
-	}{
-		{
-			Name:   "integer",
-			String: "3600",
-			Value:  3600,
-		},
-		{
-			Name:   "timestamp with AM/PM",
-			String: fmt.Sprintf("%d/%d/%d %d:%02d:%02d %s +00:00", n.Month(), n.Day(), n.Year(), n.Hour(), n.Minute(), n.Second(), amPM),
-			Value:  3600,
-		},
-		{
-			Name:   "timestamp without AM/PM",
-			String: fmt.Sprintf("%d/%d/%d %d:%02d:%02d +00:00", n.Month(), n.Day(), n.Year(), n.Hour(), n.Minute(), n.Second()),
-			Value:  3600,
-		},
-	}
-	for _, tc := range testcases {
-		t.Run(tc.Name, func(subT *testing.T) {
-			jn, err := parseExpiresOn(tc.String)
-			if err != nil {
-				subT.Error(err)
-			}
-			i, err := jn.Int64()
-			if err != nil {
-				subT.Error(err)
-			}
-			if i != tc.Value {
-				subT.Logf("expected %d, got %d", tc.Value, i)
-				subT.Fail()
-			}
-		})
-	}
-}
-
 func TestParseTokenRequest(t *testing.T) {
 	tests := []struct {
 		name             string