From f0caa97e8667ce9c31ecc78c08b5ca9a7af96634 Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Fri, 25 Oct 2024 17:04:05 -0700 Subject: [PATCH 1/3] patching GetSignedRequestSignature to cover edge cases with the signature --- .DS_Store | Bin 0 -> 8196 bytes aws/signer/v4/middleware.go | 3 +- aws/signer/v4/middleware_test.go | 57 +++++++++++++++++++++++++++++++ service/.DS_Store | Bin 0 -> 8196 bytes 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 .DS_Store create mode 100644 service/.DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..43cddbbf63dc5d8ed761acd8eaebee58ca334e87 GIT binary patch literal 8196 zcmeHMU2GIp6u#fI(3yeCEznYS0&HmHrxa-mlwZU44~sw`-7PHzTz7W{m@u6wJF~kW zAvPLc6fw~lpTz%1NsQ4%BN3h?zG*7Ti}5G%MIQ|MsL^xh&Jx=f0H2Z5)9(0uN*a#Qu<^DrhRA6Ox8o2Q}dpfT*|v zgoU2z9uV3jBbtingruP`)UDK=dcqP~LL))|z zsG6!}`O68wk0=i?mFL=Jh!d0y)C(EW7qlfn%da1VaH(hlr!dfFGx=i?FOuV znr9bgy5;!g-n=8+s%fga^tU#i5FA1}6{a z{DS40wtHf@Z2NZJu`+IM%xb`1O`EK5TGG5UVHmTij+FBD>5?6s}!%a<&)3rr6hRZUxd~ejbW2bJ66uhx9a?x)YvONtpgPTS=76Sdczf?eqq#a+JP4eDIojFk;CG3-KH73(cHA8-WW78D*sxg z#j)M8O~9r_m-tp&Qmt}*!pOFy+9jvaakZ2^LWk1;J4(qj&d#z6>~(gDz0W>o*Vq^A zD|VB8&wgY-v7gy*>=yeAwW!Aea5Nx+)mV#GtivXx(2XAK#X$@rjSP+<4-*!S!$S!b zoWWTq@8UgN#ua>ytGI?Q@eOX^Tl|1q_yd39Z>3IA6-`;FEK(Yk zCS|SCs%%uYD&2DGN;PLjrvgvuA%5CcKsiI7@7()LUm%0bO1vwUhcv zl~B*;NcLfUwMuyBb0yoFXj3V}xDvAZhD3)(dBkfZ+nU&{sT`8os&CgQqj;@kAJunh zBFQ-m`9x2mPo=Ej>b=AD1NJGq%C56Xg6l8rSN1#m2P%QI2}!IXm8%6jCP{z}kAkaRG=LoVd;bpvnS8)k%;4Qozf%ijv zHVuxCPlY3{%!Fe(U34AKJwcKxh^HFG+`UAgMk+Up`+xuJ-~aFBm9cIdfj9#Hvk0KJ zFWo0j|JgQGg=9&pPf`^{=uJo(nouKDM1Oc4Cwlu2LwZg|vQ$JTBqa%z|NKKheE#40 Nb2UEyLli!E&A%2OCf)!5 literal 0 HcmV?d00001 diff --git a/aws/signer/v4/middleware.go b/aws/signer/v4/middleware.go index a10ee510afe..ed24eaf530e 100644 --- a/aws/signer/v4/middleware.go +++ b/aws/signer/v4/middleware.go @@ -372,8 +372,9 @@ func GetSignedRequestSignature(r *http.Request) ([]byte, error) { const authHeaderSignatureElem = "Signature=" if auth := r.Header.Get(authorizationHeader); len(auth) != 0 { - ps := strings.Split(auth, ", ") + ps := strings.Split(auth, ",") for _, p := range ps { + p = strings.Trim(p, " ") if idx := strings.Index(p, authHeaderSignatureElem); idx >= 0 { sig := p[len(authHeaderSignatureElem):] if len(sig) == 0 { diff --git a/aws/signer/v4/middleware_test.go b/aws/signer/v4/middleware_test.go index 9b56ecefda5..6d7f85c1d54 100644 --- a/aws/signer/v4/middleware_test.go +++ b/aws/signer/v4/middleware_test.go @@ -3,6 +3,7 @@ package v4 import ( "bytes" "context" + "encoding/hex" "errors" "fmt" "io" @@ -372,6 +373,62 @@ func TestUseDynamicPayloadSigningMiddleware(t *testing.T) { } } +func TestGetSignedRequestSignature(t *testing.T) { + testCases := map[string]struct { + authHeader string + expectedSig string + expectedErrMsg string + }{ + "Valid signature": { + authHeader: "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024", + expectedSig: "fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024", + }, + "Whitespace after Signature": { + authHeader: "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024 ", + expectedSig: "fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024", + }, + "Whitespaces before Signature": { + authHeader: "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024 ", + expectedSig: "fe5f80f77d5fa3beca038a248ff027d0445342fe2855ddc963176630326f1024", + }, + "Empty signature": { + authHeader: "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date, Signature=", + expectedErrMsg: "invalid request signature authorization header", + }, + "Missing signature": { + authHeader: "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20150830/us-east-1/service/aws4_request, SignedHeaders=host;x-amz-date", + expectedErrMsg: "request not signed", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + r, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + r.Header.Set("Authorization", tc.authHeader) + + sig, err := GetSignedRequestSignature(r) + + if tc.expectedErrMsg != "" { + if err == nil { + t.Errorf("Expected error with message '%s', but got no error", tc.expectedErrMsg) + } else if err.Error() != tc.expectedErrMsg { + t.Errorf("Expected error message '%s', but got '%s'", tc.expectedErrMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if hex.EncodeToString(sig) != tc.expectedSig { + t.Errorf("Expected signature '%s', but got '%s'", tc.expectedSig, hex.EncodeToString(sig)) + } + } + }) + } +} + type nonSeeker struct{} func (nonSeeker) Read(p []byte) (n int, err error) { diff --git a/service/.DS_Store b/service/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..ccf5a4c129c9a84b9d5493cfaa3814468be1a653 GIT binary patch literal 8196 zcmeHMU2GIp6u#fIzziMe07Vwui3^1wWP$#mZTT_VKjpu`wsc#5mff9^4oqjt&hEBQ z8XFT|KrlXO{O3udkp}}xeDOg=^ik0SV|*ZL)E7+jMdiVB=gtynsZYiz%uVK=d(J)Q z-Z|%f=ia@uj4`woje5o+j4_!WPPHnk?ofC?e-{*|V(eW~qHKJ7imvv_nA{ zfiMDL1i}b}5eOsjUx)ym*}TY8ocp3TtiuR|5x6HK!1sq3J)9;2IxeWcbx`GB0f_P| zfM2Ms@dnXYKobES7nFLaK$sF0rUJD9Y!FGz~l(<^pTjy8neUR(5?4(uTWR_6{L#FY13!OGvq3He{#SbP8KqSqSuzm z@8w*#>)P3JJCogGnZs$VrrmW48Ot&AzJX2K4K3#?n&|b2(|z=Yl1tc%D&iMsa^O| zk>uYZxG!IqeNkpssk8LnRA2vIHLcAp(Mxv?6)e~B_LzB(Xj-fG$hplq*V&mjd4F%t zwTpv}m)7RwGIp+LX9{MkZ6~ecCdsP3T;8?qovvr`Vfs=9bELrgCUvFyJvU!y_XkT^ zG``T%ik`XKq(QBc*+D~lH4_|za&Gp52bMm#s-bB^qI2{1U015LIdkXLsA@k&(J5Gu zTV}pz*fI(OeHkxjI)>#O>MdEGm9@=oCo^cyL0nO;R7Vyrx<9JxQ>mtkS?g%gDjd^l zW!ax2wpds98T3xuU4zeJXcuG}hh|^4Sl9RRp7y*+wGkMsSgEVs9yzaN0>#y9RJHpE zEx+dRS0K~IwW{7@XR@ZP8H$*vxkc4`OCtW-)5|!^?L)EyxBEO@C&3{Na`;s$+=OdVHI4zlcKSMyy)Y@wHk)+!=lK- zYCWm<$lB*}TGK42WD(?PSq1Xt+Qiv5e|2gTE5$0+mKVzpbZw$e@CqGhI%{AZY#Tes z@@$lyX3w*;>^%FJU1DFdZ`n`mI{O{KbSRjKD%4;RmLiUa(1d2RpbcHvh22PE9|mCI zFg%RlI7V>-PvIn<#%Y|vb9foA;x(Mbn|KH3aRKk*16; zhIH=EX%>BK)8;K(|37JVb+$;X`S&e|EnB%}ZS%&~n*bDuEraw{==*rTElYg7`*@gu z)hi+c)d$MiIWZM`C{fH2ndhrELufBim&%kV+BCr~S69fCBHDDp#-pocN=i-gS+zde zs8H%@6@raNn-p1tVAiP{6iPy^Qn0$3P!x^%Otd}PDN`b8@;`;}tLz&4k^RDM5W;6; z9wJzdI>PlM*n}N~Y>w+596%ZwWMN@*m>i_@% literal 0 HcmV?d00001 From b12c8cf885a2aae18cce0b8530f1cf610ff5f11a Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Fri, 25 Oct 2024 17:47:54 -0700 Subject: [PATCH 2/3] adding changelog --- .changelog/f63b26b19e2e4330b4240f38088453ea.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changelog/f63b26b19e2e4330b4240f38088453ea.json diff --git a/.changelog/f63b26b19e2e4330b4240f38088453ea.json b/.changelog/f63b26b19e2e4330b4240f38088453ea.json new file mode 100644 index 00000000000..24318e02648 --- /dev/null +++ b/.changelog/f63b26b19e2e4330b4240f38088453ea.json @@ -0,0 +1,8 @@ +{ + "id": "f63b26b1-9e2e-4330-b424-0f38088453ea", + "type": "bugfix", + "description": "Adding logic to omit whitespaces in the signature, and test coverage", + "modules": [ + "." + ] +} \ No newline at end of file From 803614d34f44ac114759b8c1decc393bd14db55a Mon Sep 17 00:00:00 2001 From: Ran Vaknin Date: Mon, 28 Oct 2024 09:24:46 -0700 Subject: [PATCH 3/3] Fixing changelog description and implementation to use TrimSpace --- .changelog/f63b26b19e2e4330b4240f38088453ea.json | 2 +- aws/signer/v4/middleware.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changelog/f63b26b19e2e4330b4240f38088453ea.json b/.changelog/f63b26b19e2e4330b4240f38088453ea.json index 24318e02648..ef398bf401e 100644 --- a/.changelog/f63b26b19e2e4330b4240f38088453ea.json +++ b/.changelog/f63b26b19e2e4330b4240f38088453ea.json @@ -1,7 +1,7 @@ { "id": "f63b26b1-9e2e-4330-b424-0f38088453ea", "type": "bugfix", - "description": "Adding logic to omit whitespaces in the signature, and test coverage", + "description": "Improve handling of whitespace (or lack thereof) in sigv4 GetSignedRequestSignature.", "modules": [ "." ] diff --git a/aws/signer/v4/middleware.go b/aws/signer/v4/middleware.go index ed24eaf530e..8a46220a37b 100644 --- a/aws/signer/v4/middleware.go +++ b/aws/signer/v4/middleware.go @@ -374,7 +374,7 @@ func GetSignedRequestSignature(r *http.Request) ([]byte, error) { if auth := r.Header.Get(authorizationHeader); len(auth) != 0 { ps := strings.Split(auth, ",") for _, p := range ps { - p = strings.Trim(p, " ") + p = strings.TrimSpace(p) if idx := strings.Index(p, authHeaderSignatureElem); idx >= 0 { sig := p[len(authHeaderSignatureElem):] if len(sig) == 0 {