From 706c9c15a85cac0721ab1c773631a3981e93a576 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 12 Oct 2021 14:48:08 -0700 Subject: [PATCH 1/3] Update JWT library to golang-jwt/jwt (#2074) --- cmd/nginx-ingress/aws.go | 53 +++++++++++---------- cmd/nginx-ingress/aws_test.go | 89 +++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 5 +- 4 files changed, 120 insertions(+), 29 deletions(-) create mode 100644 cmd/nginx-ingress/aws_test.go diff --git a/cmd/nginx-ingress/aws.go b/cmd/nginx-ingress/aws.go index 1d7e80d08ef..9bb937932fe 100644 --- a/cmd/nginx-ingress/aws.go +++ b/cmd/nginx-ingress/aws.go @@ -1,4 +1,4 @@ -// +build aws +//go:build aws package main @@ -15,7 +15,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/marketplacemetering" "github.com/aws/aws-sdk-go-v2/service/marketplacemetering/types" - "github.com/dgrijalva/jwt-go/v4" + "github.com/golang-jwt/jwt/v4" ) var ( @@ -61,49 +61,52 @@ func checkAWSEntitlement() error { return err } - pk, err := base64.StdEncoding.DecodeString(pubKeyString) - if err != nil { - return fmt.Errorf("error decoding Public Key string: %w", err) - } - pubKey, err := jwt.ParseRSAPublicKeyFromPEM(pk) - if err != nil { - return fmt.Errorf("error parsing Public Key: %w", err) - } + token, err := jwt.ParseWithClaims(*out.Signature, &claims{}, func(token *jwt.Token) (interface{}, error) { + if _, ok := token.Method.(*jwt.SigningMethodRSAPSS); !ok { + return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) + } - token, err := jwt.ParseWithClaims(*out.Signature, &claims{}, jwt.KnownKeyfunc(jwt.SigningMethodPS256, pubKey)) - if err != nil { - return fmt.Errorf("error parsing the JWT token: %w", err) - } + pk, err := base64.StdEncoding.DecodeString(pubKeyString) + if err != nil { + return nil, fmt.Errorf("error decoding Public Key string: %w", err) + } + pubKey, err := jwt.ParseRSAPublicKeyFromPEM(pk) + if err != nil { + return nil, fmt.Errorf("error parsing Public Key: %w", err) + } + + return pubKey, nil + }) if claims, ok := token.Claims.(*claims); ok && token.Valid { if claims.ProductCode != productCode || claims.PublicKeyVersion != pubKeyVersion || claims.Nonce != nonce { return fmt.Errorf("the claims in the JWT token don't match the request") } } else { - return fmt.Errorf("something is wrong with the JWT token") + return fmt.Errorf("something is wrong with the JWT token: %w", err) } - return nil } type claims struct { - ProductCode string `json:"productCode,omitempty"` - PublicKeyVersion int32 `json:"publicKeyVersion,omitempty"` - IssuedAt *jwt.Time `json:"iat,omitempty"` - Nonce string `json:"nonce,omitempty"` + ProductCode string `json:"productCode,omitempty"` + PublicKeyVersion int32 `json:"publicKeyVersion,omitempty"` + Nonce string `json:"nonce,omitempty"` + jwt.RegisteredClaims } -func (c claims) Valid(h *jwt.ValidationHelper) error { +func (c claims) Valid() error { if c.Nonce == "" { - return &jwt.InvalidClaimsError{Message: "the JWT token doesn't include the Nonce"} + return jwt.NewValidationError("token doesn't include the Nonce", jwt.ValidationErrorClaimsInvalid) } if c.ProductCode == "" { - return &jwt.InvalidClaimsError{Message: "the JWT token doesn't include the ProductCode"} + return jwt.NewValidationError("token doesn't include the ProductCode", jwt.ValidationErrorClaimsInvalid) } if c.PublicKeyVersion == 0 { - return &jwt.InvalidClaimsError{Message: "the JWT token doesn't include the PublicKeyVersion"} + return jwt.NewValidationError("token doesn't include the PublicKeyVersion", jwt.ValidationErrorClaimsInvalid) } - if err := h.ValidateNotBefore(c.IssuedAt); err != nil { + + if err := c.RegisteredClaims.Valid(); err != nil { return err } diff --git a/cmd/nginx-ingress/aws_test.go b/cmd/nginx-ingress/aws_test.go new file mode 100644 index 00000000000..1c151f0b5d9 --- /dev/null +++ b/cmd/nginx-ingress/aws_test.go @@ -0,0 +1,89 @@ +//go:build aws + +package main + +import ( + "errors" + "testing" + "time" + + "github.com/golang-jwt/jwt/v4" +) + +func TestValidClaims(t *testing.T) { + iat := *jwt.NewNumericDate(time.Now().Add(time.Hour * -1)) + + c := claims{ + "test", + 1, + "nonce", + jwt.RegisteredClaims{ + IssuedAt: &iat, + }, + } + if err := c.Valid(); err != nil { + t.Fatalf("Failed to verify claims, wanted: %v got %v", nil, err) + } +} + +func TestInvalidClaims(t *testing.T) { + badClaims := []struct { + c claims + expectedError error + }{ + { + claims{ + "", + 1, + "nonce", + jwt.RegisteredClaims{ + IssuedAt: jwt.NewNumericDate(time.Now().Add(time.Hour * -1)), + }, + }, + errors.New("token doesn't include the ProductCode"), + }, + { + claims{ + "productCode", + 1, + "", + jwt.RegisteredClaims{ + IssuedAt: jwt.NewNumericDate(time.Now().Add(time.Hour * -1)), + }, + }, + errors.New("token doesn't include the Nonce"), + }, + { + claims{ + "productCode", + 0, + "nonce", + jwt.RegisteredClaims{ + IssuedAt: jwt.NewNumericDate(time.Now().Add(time.Hour * -1)), + }, + }, + errors.New("token doesn't include the PublicKeyVersion"), + }, + { + claims{ + "test", + 1, + "nonce", + jwt.RegisteredClaims{ + IssuedAt: jwt.NewNumericDate(time.Now().Add(time.Hour * +2)), + }, + }, + errors.New("token used before issued"), + }, + } + + for _, badC := range badClaims { + + err := badC.c.Valid() + if err == nil { + t.Errorf("Valid() returned no error when it should have returned error %q", badC.expectedError) + } else if err.Error() != badC.expectedError.Error() { + t.Errorf("Valid() returned error %q when it should have returned error %q", err, badC.expectedError) + } + } +} diff --git a/go.mod b/go.mod index e2a9414bac5..d06aea6bc8c 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.17 require ( github.com/aws/aws-sdk-go-v2/config v1.8.2 github.com/aws/aws-sdk-go-v2/service/marketplacemetering v1.5.1 - github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1 + github.com/golang-jwt/jwt/v4 v4.1.0 github.com/golang/glog v1.0.0 github.com/google/go-cmp v0.5.6 github.com/nginxinc/nginx-plus-go-client v0.8.0 diff --git a/go.sum b/go.sum index 16ce2c54efe..e23cb3270fe 100644 --- a/go.sum +++ b/go.sum @@ -145,10 +145,7 @@ github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= -github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1 h1:CaO/zOnF8VvUfEbhRatPcwKVWamvbYd8tQGRWacE9kU= -github.com/dgrijalva/jwt-go/v4 v4.0.0-preview1/go.mod h1:+hnT3ywWDTAFrW5aE+u2Sa/wT555ZqwoCS+pk3p6ry4= github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no= github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE= github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= @@ -220,6 +217,8 @@ github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zV github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= +github.com/golang-jwt/jwt/v4 v4.1.0 h1:XUgk2Ex5veyVFVeLm0xhusUTQybEbexJXrvPNOKkSY0= +github.com/golang-jwt/jwt/v4 v4.1.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/glog v1.0.0 h1:nfP3RFugxnNRyKgeWd4oI1nYvXpxrx8ck8ZrcizshdQ= github.com/golang/glog v1.0.0/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4= From 29c2e2c8843d40f4ddb9e02f9712265e0fed4080 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Mon, 11 Oct 2021 11:14:44 -0700 Subject: [PATCH 2/3] Update packages for CVE-2021-37750 --- build/Dockerfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build/Dockerfile b/build/Dockerfile index fa47c3dffe3..3bc267d71b4 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -9,6 +9,8 @@ FROM nginx:1.21.3 AS debian RUN apt-get update \ && apt-get install --no-install-recommends --no-install-suggests -y libcap2-bin \ + # temporary fix for CVE-2021-37750 + && apt-get install -y libkrb5-3 libk5crypto3 \ && rm -rf /var/lib/apt/lists/* \ && echo $NGINX_VERSION > nginx_version From 92f2bd84f9b6819d66a13852fd5c2ca0c0131da2 Mon Sep 17 00:00:00 2001 From: Luca Comellini Date: Tue, 12 Oct 2021 14:39:04 -0700 Subject: [PATCH 3/3] Use release specific repo for NAP on Debian --- build/Dockerfile | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/build/Dockerfile b/build/Dockerfile index 3bc267d71b4..5362e03f4bd 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -47,11 +47,10 @@ RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/ssl/nginx/nginx-repo.crt,mode && apt-get install --no-install-recommends --no-install-suggests -y ca-certificates gnupg curl apt-transport-https libcap2-bin \ && curl -sSL https://cs.nginx.com/static/keys/nginx_signing.key | gpg --dearmor > /etc/apt/trusted.gpg.d/nginx_signing.gpg \ && curl -sSL -o /etc/apt/apt.conf.d/90pkgs-nginx https://cs.nginx.com/static/files/90pkgs-nginx \ - && printf "%s\n" "Acquire::https::pkgs.nginx.com::User-Agent \"k8s-ic-$IC_VERSION-apt\";" >> /etc/apt/apt.conf.d/90pkgs-nginx \ + && printf "%s\n" "Acquire::https::pkgs.nginx.com::User-Agent \"k8s-ic-$IC_VERSION${BUILD_OS##debian-plus}-apt\";" >> /etc/apt/apt.conf.d/90pkgs-nginx \ && printf "%s\n" "deb https://pkgs.nginx.com/plus/${NGINX_PLUS_VERSION^^}/debian buster nginx-plus" > /etc/apt/sources.list.d/nginx-plus.list \ && apt-get update \ - && apt-get install --no-install-recommends --no-install-suggests -y \ - nginx-plus-${NGINX_PLUS_VERSION} nginx-plus-module-njs-${NGINX_PLUS_VERSION} \ + && apt-get install --no-install-recommends --no-install-suggests -y nginx-plus nginx-plus-module-njs \ && apt-get purge --auto-remove -y apt-transport-https gnupg curl \ && rm -rf /var/lib/apt/lists/* @@ -66,14 +65,11 @@ RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/ssl/nginx/nginx-repo.crt,mode apt-get update \ && apt-get install --no-install-recommends --no-install-suggests -y gnupg curl apt-transport-https \ && curl -sSL https://cs.nginx.com/static/keys/app-protect-security-updates.key | gpg --dearmor > /etc/apt/trusted.gpg.d/nginx_app_signing.gpg \ - && sed -i "s/$IC_VERSION/$IC_VERSION-nap/g" /etc/apt/apt.conf.d/90pkgs-nginx \ - && printf "%s\n" "deb https://pkgs.nginx.com/app-protect/debian buster nginx-plus" \ + && printf "%s\n" "deb https://pkgs.nginx.com/app-protect/${NGINX_PLUS_VERSION^^}/debian buster nginx-plus" \ "deb https://pkgs.nginx.com/app-protect-security-updates/debian buster nginx-plus" > /etc/apt/sources.list.d/nginx-app-protect.list \ && apt-get update \ - # searching apt-cache for the latest version of NAP package compatible with the $NGINX_PLUS_VERSION - && module_version=$(apt-cache showpkg nginx-plus-module-appprotect | awk -v ver="nginx-plus-$NGINX_PLUS_VERSION" '{ if ($6 == ver) {print $1; exit}}') \ - && apt-get install --no-install-recommends --no-install-suggests -y nginx-plus-module-appprotect=${module_version} app-protect=${module_version} \ - && apt-get install --no-install-recommends --no-install-suggests -y app-protect-attack-signatures app-protect-threat-campaigns \ + && apt-get install --no-install-recommends --no-install-suggests -y \ + nginx-plus-module-appprotect app-protect app-protect-attack-signatures app-protect-threat-campaigns \ && apt-get purge --auto-remove -y apt-transport-https gnupg curl \ && rm -rf /var/lib/apt/lists/* \ && rm /etc/apt/sources.list.d/nginx-app-protect.list