v4.Signer handles query param encoding incorrectly

[staffan-einarsson]

Acknowledgements

• I have searched (https://github.com/aws/aws-sdk/issues?q=is%3Aissue) for past instances of this issue
• I have verified all of my SDK modules are up-to-date (you can perform a bulk update with go get -u github.com/aws/aws-sdk-go-v2/...)

Describe the bug

When assembling the canonical query string for the canonical request, the AWS docs specify that query parameters should first be percent-encoded to cover for all special characters, and after that the parameters should be sorted. However, v4.Signer currently does the sorting first, and then the percent-encoding. This seems to cause a signature mismatch when the query parameters contain special characters in such a way that it affects their position after sorting.

CanonicalQueryString – The URI-encoded query string parameters. You URI-encode each name and value individually. You must also sort the parameters in the canonical query string alphabetically by key name. The sorting occurs after encoding.

https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv-create-signed-request.html

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

Using a URL query a=1&🍺=2, the signer should use the following canonical query string

%F0%9F%8D%BA=2&a=1

First the query param names and values should be percent-encoded, then sorted as above since "%" comes before "a".

Current Behavior

Using a URL query a=1&🍺=2, the signer uses the following canonical query string

a=1&%F0%9F%8D%BA=2

This indicates that the sorting happens before encoding, and "a" comes before "🍺".

Full log output:

SDK 2025/01/17 12:03:45 DEBUG Request Signature:
---[ CANONICAL STRING  ]-----------------------------
GET
/
a=1&%F0%9F%8D%BA=2
host:example.com
x-amz-date:20200101T000000Z

host;x-amz-date
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
---[ STRING TO SIGN ]--------------------------------
AWS4-HMAC-SHA256
20200101T000000Z
20200101/eu-west-1/lambda/aws4_request
fa4028d7a8c7230bdfbf6458bf02b74ae940ddaecd42f1619a49776848cbc97d
-----------------------------------------------------

Reproduction Steps

package main

import (
	"context"
	"crypto/sha256"
	"encoding/hex"
	"fmt"
	"net/http"
	"net/url"
	"os"
	"time"

	"github.com/aws/aws-sdk-go-v2/aws"
	v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
	"github.com/aws/smithy-go/logging"
)

func main() {
	signer := v4.NewSigner(func(signer *v4.SignerOptions) {
		signer.LogSigning = true
		signer.Logger = logging.NewStandardLogger(os.Stderr)
	})

	request := http.Request{
		Method: "GET",
		URL: &url.URL{
			Scheme:   "https",
			Host:     "example.com",
			Path:     "/",
			RawQuery: "a=1&🍺=2",
		},
		Header: map[string][]string{},
	}

	payloadHash := sha256.Sum256([]byte{})

	err := signer.SignHTTP(
		context.Background(),
		aws.Credentials{
			AccessKeyID:     "id",
			SecretAccessKey: "secret",
		},
		&request,
		hex.EncodeToString(payloadHash[:]),
		"lambda",
		"eu-west-1",
		time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC),
	)
	if err != nil {
		panic(err)
	}

	fmt.Fprintln(os.Stderr, request.Header)
}

Possible Solution

Looking at the implementation, it seems the issue is that query params uses the built in url.URL and writes it back using url.Values.Encode(). This automatically handles param encoding and sorting, but in the opposite order. There might be need for a custom encoder here.

Additional Information/Context

The example above is contrived. We are not using emoji in parameters, but we do have a real production cases where there are special characters in the parameters that do affect the sorting and does break the signature.

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2@v1.33.0 github.com/aws/smithy-go@v1.22.1
github.com/aws/aws-sdk-go-v2@v1.33.0 github.com/jmespath/go-jmespath@v0.4.0
github.com/aws/aws-sdk-go-v2@v1.33.0 go@1.21

Compiler and Version used

go version go1.23.2 darwin/arm64

Operating System and version

macOS Sonoma 14.7.2

[adev-code]

Hello @staffan-einarsson, thanks for reaching out and finding out the bug. I have brought this with our team and have added this to our queue.