[BUG] salt.utils.aws.sig4 force usage of version as GET parameters even if method is not GET

[jynolen]

Description
A clear and concise description of what the bug is.

** Setup
No Setup required

Steps to Reproduce the behavior

from salt.utils.aws import sig4
import requests

method = "POST"
body="Action=GetCallerIdentity&Version=2011-06-15"
endpoint="sts.amazonaws.com"
location="us-east-1"
params = {}
version = "2011-06-15"
product = "sts"
provider = {'id': 'use-instance-role-credentials', 'key': 'use-instance-role-credentials'}
hd = {"Content-Type": "application/x-www-form-urlencoded; charset=utf-8"}
headers, requests_url = sig4(   method=method, 
                                endpoint=endpoint,
                                prov_dict=provider,
                                params=params,
                                product="sts",
                                data=body,
                                aws_api_version=version,
                                location=location,
                                requesturl=f"https://{endpoint}/",
                                headers=hd
                            )
print(requests_url)
# https://sts.amazonaws.com?Version=2011-06-15
print(headers)
# {'Content-Type': 'application/x-www-form-urlencoded; charset=utf-8', 'X-Amz-date': '20211116T160158Z', 'host': 'sts.amazonaws.com', 'x-amz-content-sha256': 'ab821ae955788b0e33ebd34c208442ccfc2d406e2edc5e7a39bd6458fbb4f843', 'X-Amz-security-token': '<redacted>', 'Authorization': 'AWS4-HMAC-SHA256 Credential=<redacted>/20211116/us-east-1/sts/aws4_request, SignedHeaders=content-type;host;x-amz-content-sha256;x-amz-date;x-amz-security-token, Signature=<redacted>'}
r = requests.post(requests_url,headers= headers, data=body) # https://sts.amazonaws.com/?Version=2011-06-15
print(r.status_code)
# 400
print(r.text)
# ... When Content-Type:application/x-www-form-urlencoded, URL cannot include query-string parameters (after \'?\'): \'/?Version=2011-06-15\' ...
r = requests.post("https://sts.amazonaws.com/",headers= headers, data=body) # Manually remove GET parameters
print(r.status_code)
# 403
print(r.text)
# The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.
params = {"Action":"GetCallerIdentity", "Version": "2011-06-15"}
method = "GET"
hd = {}
body=""
headers, requests_url = sig4(   method=method, 
                                endpoint=endpoint,
                                prov_dict=provider,
                                params=params,
                                product="sts",
                                data=body,
                                aws_api_version=version,
                                location=location,
                                requesturl=f"https://{endpoint}/",
                                headers=hd
                            )
print(requests_url)
# https://sts.amazonaws.com/?Action=GetCallerIdentity&Version=2011-06-15
r = requests.get(requests_url,headers= headers)
print(r.status_code)
# 200

Expected behavior
request_url should not contains any params when not using GET method and then query_string should be empty in order to have a valid aws sig4 signature

Versions Report

salt --versions-report

``` Salt Version: Salt: 3004

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 2.7.3
docker-py: Not Installed
gitdb: 2.0.6
gitpython: 3.0.7
Jinja2: 2.10.1
libgit2: 0.28.3
M2Crypto: Not Installed
Mako: Not Installed
msgpack: 0.6.2
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: Not Installed
pycryptodome: 3.6.1
pygit2: 1.0.3
Python: 3.8.10 (default, Sep 28 2021, 16:10:42)
python-gnupg: 0.4.5
PyYAML: 5.3.1
PyZMQ: 18.1.1
smmap: 2.0.5
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.3.2

System Versions:
dist: ubuntu 20.04 focal
locale: utf-8
machine: x86_64
release: 5.11.0-1020-aws
system: Linux
version: Ubuntu 20.04 focal

</details>

[jynolen]

Seems I found a quick fix

diff --git a/salt/utils/aws.py b/salt/utils/aws.py
index fa92cd02dd..7245a48ee2 100644
--- a/salt/utils/aws.py
+++ b/salt/utils/aws.py
@@ -272,12 +272,14 @@ def sig4(
     if location is None:
         location = DEFAULT_LOCATION

-    params_with_headers = params.copy()
-    if product not in ("s3", "ssm"):
-        params_with_headers["Version"] = aws_api_version
-    keys = sorted(params_with_headers.keys())
-    values = list(map(params_with_headers.get, keys))
-    querystring = urllib.parse.urlencode(list(zip(keys, values))).replace("+", "%20")
+    querystring=""
+    if method =="GET":
+        params_with_headers = params.copy()
+        if product not in ("s3", "ssm"):
+            params_with_headers["Version"] = aws_api_version
+        keys = sorted(params_with_headers.keys())
+        values = list(map(params_with_headers.get, keys))
+        querystring = urllib.parse.urlencode(list(zip(keys, values))).replace("+", "%20")

     amzdate = timenow.strftime("%Y%m%dT%H%M%SZ")
     datestamp = timenow.strftime("%Y%m%d")
@@ -344,8 +346,8 @@ def sig4(
     )

     new_headers["Authorization"] = authorization_header
-
-    requesturl = "{}?{}".format(requesturl, querystring)
+    if querystring:
+        requesturl = "{}?{}".format(requesturl, querystring)
     return new_headers, requesturl

[waynew]

I'm confused - https://docs.aws.amazon.com/general/latest/gr/sigv4-signed-request-examples.html#sig-v4-examples-get-query-string suggests that auth info in the query string is valid... is that incorrect? Or not what we're doing at this point?

[jynolen]

The fact is that when you perform POST operation AWS does not accept any GET parameters in the http requests BUT the cannonical requests signature requires:

• method
• requests parameters

Right now the behavior is the following:

• if you want to perform GET request => no issue
• if you want to perform POST request, there is 2 case
  • AWS API reject your requests because it contains hardcoded GET parameters (Version)
  • if somehow you managed to get the requests and force to remove the GET parameters (Version) you still have an error because the signature doesn't match any more

So the fix is that sig4 method should add GET parameters only when method is GET