Copilot commented on code in PR #213:
URL:
https://github.com/apache/cloudstack-cloudmonkey/pull/213#discussion_r3341029529
##########
cmd/network.go:
##########
@@ -218,6 +219,195 @@ func encodeRequestParams(params url.Values) string {
return buf.String()
}
+func cloneRequestParams(params url.Values) url.Values {
+ cloned := make(url.Values)
+ for key, values := range params {
+ for _, value := range values {
+ cloned.Add(key, value)
+ }
+ }
+ return cloned
+}
+
+func buildAPIRequestParams(r *Request, api string, args []string) url.Values {
+ params := make(url.Values)
+ params.Add("command", api)
+ apiData := r.Config.GetCache()[api]
+ for _, arg := range args {
+ if apiData != nil {
+ skip := false
+ for _, fakeArg := range apiData.FakeArgs {
+ if strings.HasPrefix(arg, fakeArg) {
+ skip = true
+ break
+ }
+ }
+ if skip {
+ continue
+ }
+
+ }
+ parts := strings.SplitN(arg, "=", 2)
+ if len(parts) == 2 {
+ key := parts[0]
+ value := parts[1]
+ if strings.HasPrefix(value, "\"") &&
strings.HasSuffix(value, "\"") {
+ value = value[1 : len(value)-1]
+ }
+ if strings.HasPrefix(value, "@") {
+ possibleFileName := value[1:]
+ if fileInfo, err := os.Stat(possibleFileName);
err == nil && !fileInfo.IsDir() {
+ bytes, err :=
ioutil.ReadFile(possibleFileName)
+ config.Debug()
+ if err == nil {
+ value = string(bytes)
+ config.Debug("Content for
argument ", key, " read from file: ", possibleFileName, " is: ", value)
+ }
+ }
+ }
+ params.Add(key, value)
+ }
+ }
+ signatureversion := "3"
+ expiresKey := "expires"
+ params.Add("response", "json")
+ params.Add("signatureversion", signatureversion)
+ params.Add(expiresKey,
time.Now().UTC().Add(15*time.Minute).Format(time.RFC3339))
+ return params
+}
+
+func signRequest(unsignedRequest, secretKey, algorithm string) (string, error)
{
+ signatureAlgorithm, err := config.NormalizeSignatureAlgorithm(algorithm)
+ if err != nil {
+ return "", err
+ }
+
+ var signature []byte
+ switch signatureAlgorithm {
+ case config.SignatureAlgorithmHmacSHA1:
+ mac := hmac.New(sha1.New, []byte(secretKey))
+ mac.Write([]byte(strings.ToLower(unsignedRequest)))
+ signature = mac.Sum(nil)
+ case config.SignatureAlgorithmHmacSHA512:
+ mac := hmac.New(sha512.New, []byte(secretKey))
+ mac.Write([]byte(strings.ToLower(unsignedRequest)))
+ signature = mac.Sum(nil)
+ default:
+ return "", errors.New("signature algorithm must be concrete")
+ }
+ return base64.StdEncoding.EncodeToString(signature), nil
+}
+
+func executeSignedAPIRequest(r *Request, unsignedParams url.Values, algorithm
string) (*http.Response, error) {
+ params := cloneRequestParams(unsignedParams)
+ encodedParams := encodeRequestParams(params)
+
+ signature, err := signRequest(encodedParams,
r.Config.ActiveProfile.SecretKey, algorithm)
+ if err != nil {
+ return nil, err
+ }
+ if r.Config.Core.PostRequest {
+ params.Add("signature", signature)
+ } else {
+ encodedParams = encodedParams + fmt.Sprintf("&signature=%s",
url.QueryEscape(signature))
+ params = nil
+ }
+
+ requestURL := fmt.Sprintf("%s?%s", r.Config.ActiveProfile.URL,
encodedParams)
+ config.Debug("NewAPIRequest API request URL:", requestURL)
+ return executeRequest(r, requestURL, params)
+}
+
+func parseAPIResponse(body []byte) (map[string]interface{}, error) {
+ var data map[string]interface{}
+ if err := json.Unmarshal(body, &data); err != nil {
+ return nil, errors.New("failed to decode response")
+ }
+
+ if apiResponse := getResponseData(data); apiResponse != nil {
+ if _, ok := apiResponse["errorcode"]; ok {
+ return nil, fmt.Errorf("(HTTP %v, error code %v) %v",
apiResponse["errorcode"], apiResponse["cserrorcode"], apiResponse["errortext"])
+ }
+ return apiResponse, nil
+ }
+
+ return nil, errors.New("failed to decode response")
+}
+
+func isAuthenticationFailure(statusCode int, err error) bool {
+ if statusCode == http.StatusUnauthorized || statusCode ==
http.StatusForbidden {
+ return true
+ }
+ if err == nil {
+ return false
+ }
+ errText := strings.ToLower(err.Error())
+ for _, marker := range []string{"signature", "authenticate",
"authentication", "credential", "unauthoriz", "api key", "apikey"} {
+ if strings.Contains(errText, marker) {
+ return true
+ }
+ }
+ return false
+}
+
+func persistDetectedSignatureAlgorithm(r *Request, algorithm string) {
+ r.Config.ActiveProfile.SignatureAlgorithm = algorithm
+ if r.CredentialsSupplied {
+ config.Debug("Credentials supplied on command-line, not
persisting detected signature algorithm")
+ return
+ }
+ r.Config.UpdateConfig("signaturealgorithm", algorithm, true)
+}
+
+func detectSignatureAlgorithm(r *Request) (string, error) {
+ attempts := []string{config.SignatureAlgorithmHmacSHA512,
config.SignatureAlgorithmHmacSHA1}
+ var lastErr error
+
+ for _, algorithm := range attempts {
+ config.Debug("Trying API signature algorithm probe:", algorithm)
+ params := buildAPIRequestParams(r, "listApis",
[]string{"listall=true"})
+ params.Add("apiKey", r.Config.ActiveProfile.APIKey)
+ response, err := executeSignedAPIRequest(r, params, algorithm)
+ if err != nil {
+ config.Debug("API signature algorithm probe failed
before response for ", algorithm, ": ", err)
+ lastErr = err
+ continue
+ }
+
+ body, _ := ioutil.ReadAll(response.Body)
+ config.Debug("Signature algorithm probe response body:",
string(body))
+ if _, err := parseAPIResponse(body); err == nil {
Review Comment:
The signature algorithm probe reads the response body without closing it and
ignores potential read errors. This can leak HTTP connections and hide IO
failures during autodetection.
##########
cmd/network.go:
##########
@@ -384,27 +530,26 @@ func NewAPIRequest(r *Request, api string, args []string,
isAsync bool) (map[str
}
}
+ return processAPIResponse(r, response, isAsync)
+}
+
+func processAPIResponse(r *Request, response *http.Response, isAsync bool)
(map[string]interface{}, error) {
body, _ := ioutil.ReadAll(response.Body)
config.Debug("NewAPIRequest response body:", string(body))
Review Comment:
processAPIResponse reads the response body without closing it and ignores
any ReadAll error. This can leak HTTP connections and make failures harder to
diagnose.
##########
cmd/network.go:
##########
@@ -218,6 +219,195 @@ func encodeRequestParams(params url.Values) string {
return buf.String()
}
+func cloneRequestParams(params url.Values) url.Values {
+ cloned := make(url.Values)
+ for key, values := range params {
+ for _, value := range values {
+ cloned.Add(key, value)
+ }
+ }
+ return cloned
+}
+
+func buildAPIRequestParams(r *Request, api string, args []string) url.Values {
+ params := make(url.Values)
+ params.Add("command", api)
+ apiData := r.Config.GetCache()[api]
+ for _, arg := range args {
+ if apiData != nil {
+ skip := false
+ for _, fakeArg := range apiData.FakeArgs {
+ if strings.HasPrefix(arg, fakeArg) {
+ skip = true
+ break
+ }
+ }
+ if skip {
+ continue
+ }
+
+ }
+ parts := strings.SplitN(arg, "=", 2)
+ if len(parts) == 2 {
+ key := parts[0]
+ value := parts[1]
+ if strings.HasPrefix(value, "\"") &&
strings.HasSuffix(value, "\"") {
+ value = value[1 : len(value)-1]
+ }
+ if strings.HasPrefix(value, "@") {
+ possibleFileName := value[1:]
+ if fileInfo, err := os.Stat(possibleFileName);
err == nil && !fileInfo.IsDir() {
+ bytes, err :=
ioutil.ReadFile(possibleFileName)
+ config.Debug()
+ if err == nil {
+ value = string(bytes)
+ config.Debug("Content for
argument ", key, " read from file: ", possibleFileName, " is: ", value)
+ }
+ }
+ }
+ params.Add(key, value)
+ }
+ }
+ signatureversion := "3"
+ expiresKey := "expires"
+ params.Add("response", "json")
+ params.Add("signatureversion", signatureversion)
+ params.Add(expiresKey,
time.Now().UTC().Add(15*time.Minute).Format(time.RFC3339))
+ return params
+}
+
+func signRequest(unsignedRequest, secretKey, algorithm string) (string, error)
{
+ signatureAlgorithm, err := config.NormalizeSignatureAlgorithm(algorithm)
+ if err != nil {
+ return "", err
+ }
+
+ var signature []byte
+ switch signatureAlgorithm {
+ case config.SignatureAlgorithmHmacSHA1:
+ mac := hmac.New(sha1.New, []byte(secretKey))
+ mac.Write([]byte(strings.ToLower(unsignedRequest)))
+ signature = mac.Sum(nil)
+ case config.SignatureAlgorithmHmacSHA512:
+ mac := hmac.New(sha512.New, []byte(secretKey))
+ mac.Write([]byte(strings.ToLower(unsignedRequest)))
+ signature = mac.Sum(nil)
+ default:
+ return "", errors.New("signature algorithm must be concrete")
+ }
+ return base64.StdEncoding.EncodeToString(signature), nil
+}
+
+func executeSignedAPIRequest(r *Request, unsignedParams url.Values, algorithm
string) (*http.Response, error) {
+ params := cloneRequestParams(unsignedParams)
+ encodedParams := encodeRequestParams(params)
+
+ signature, err := signRequest(encodedParams,
r.Config.ActiveProfile.SecretKey, algorithm)
+ if err != nil {
+ return nil, err
+ }
+ if r.Config.Core.PostRequest {
+ params.Add("signature", signature)
+ } else {
+ encodedParams = encodedParams + fmt.Sprintf("&signature=%s",
url.QueryEscape(signature))
+ params = nil
+ }
+
+ requestURL := fmt.Sprintf("%s?%s", r.Config.ActiveProfile.URL,
encodedParams)
+ config.Debug("NewAPIRequest API request URL:", requestURL)
+ return executeRequest(r, requestURL, params)
+}
+
+func parseAPIResponse(body []byte) (map[string]interface{}, error) {
+ var data map[string]interface{}
+ if err := json.Unmarshal(body, &data); err != nil {
+ return nil, errors.New("failed to decode response")
+ }
+
+ if apiResponse := getResponseData(data); apiResponse != nil {
+ if _, ok := apiResponse["errorcode"]; ok {
+ return nil, fmt.Errorf("(HTTP %v, error code %v) %v",
apiResponse["errorcode"], apiResponse["cserrorcode"], apiResponse["errortext"])
+ }
+ return apiResponse, nil
+ }
+
+ return nil, errors.New("failed to decode response")
+}
+
+func isAuthenticationFailure(statusCode int, err error) bool {
+ if statusCode == http.StatusUnauthorized || statusCode ==
http.StatusForbidden {
+ return true
+ }
+ if err == nil {
+ return false
+ }
+ errText := strings.ToLower(err.Error())
+ for _, marker := range []string{"signature", "authenticate",
"authentication", "credential", "unauthoriz", "api key", "apikey"} {
+ if strings.Contains(errText, marker) {
+ return true
+ }
+ }
+ return false
+}
+
+func persistDetectedSignatureAlgorithm(r *Request, algorithm string) {
+ r.Config.ActiveProfile.SignatureAlgorithm = algorithm
+ if r.CredentialsSupplied {
+ config.Debug("Credentials supplied on command-line, not
persisting detected signature algorithm")
+ return
+ }
+ r.Config.UpdateConfig("signaturealgorithm", algorithm, true)
+}
+
+func detectSignatureAlgorithm(r *Request) (string, error) {
+ attempts := []string{config.SignatureAlgorithmHmacSHA512,
config.SignatureAlgorithmHmacSHA1}
+ var lastErr error
+
+ for _, algorithm := range attempts {
+ config.Debug("Trying API signature algorithm probe:", algorithm)
+ params := buildAPIRequestParams(r, "listApis",
[]string{"listall=true"})
+ params.Add("apiKey", r.Config.ActiveProfile.APIKey)
+ response, err := executeSignedAPIRequest(r, params, algorithm)
+ if err != nil {
+ config.Debug("API signature algorithm probe failed
before response for ", algorithm, ": ", err)
+ lastErr = err
+ continue
+ }
+
+ body, _ := ioutil.ReadAll(response.Body)
+ config.Debug("Signature algorithm probe response body:",
string(body))
+ if _, err := parseAPIResponse(body); err == nil {
+ config.Debug("Selected API signature algorithm:",
algorithm)
+ persistDetectedSignatureAlgorithm(r, algorithm)
+ return algorithm, nil
+ } else {
+ lastErr = err
+ if isAuthenticationFailure(response.StatusCode, err) {
+ config.Debug("API signature algorithm probe
failed authentication for ", algorithm, ": ", err)
+ } else {
+ config.Debug("API signature algorithm probe
failed with non-authentication error for ", algorithm, ": ", err)
+ }
+ }
Review Comment:
Autodetection currently falls back to trying the next algorithm even when
the probe failed for a non-authentication reason (e.g., server error, decode
error). Per the PR behavior description, fallback should only happen on
authentication/signature verification failures; otherwise the original error
should be returned immediately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
