[refactor] - GCP detector

[ahrav]

Description:

This PR refactors the validation logic for GCP service account keys. Instead of directly calling the Google API to verify the credentials using google.CredentialsFromJSON, we now perform the validation by comparing the public keys.

Key changes:

• Renamed the validation function to isValidGCPServiceAccountKey to better reflect its purpose.
• Updated the function to retrieve the public key certificate from the ClientX509CertURL field of the gcpKey struct.
• Extracted the public key from the retrieved certificate and compared it with the public key derived from the PrivateKey field of the gcpKey struct.
• If the public keys match, the function returns true, indicating a valid service account key. Otherwise, it returns false.
• Updated the function comment to provide a clear explanation of the validation logic.

Benefits:

• Improved efficiency by avoiding the need to make a direct API call to verify the credentials.
• Enhanced security by validating the service account key based on the public key comparison.
• Increased clarity and readability of the code by renaming the function and updating the comment.

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

[rgmz]

Wouldn't the API call still be necessary? A genuine key does not necessarily equate to a valid key (e.g., revocation.)

[ahrav]

Wouldn't the API call still be necessary? A genuine key does not necessarily equate to a valid key (e.g., revocation.)

I totally understand your concern about the necessity of making an API call to verify the validity of the credential, considering factors like revocation, I had the same concerns initially.

I have actually tested this scenario by disabling the service account, and I can confirm that the approach of comparing the public keys works as expected. When a service account is expired or disabled, the request to fetch the certificate from the ClientX509CertURL returns a 404 status code. This indicates that the certificate is no longer accessible, and the service account is correctly considered invalid.

By relying on the availability of the certificate at the ClientX509CertURL, we can effectively validate the status of the service account. If the certificate is not found (404 status code), it means the service account is no longer valid, regardless of the public key comparison.

This approach aligns with the recommendation provided to us by the Google team. They suggested that if we can successfully retrieve the certificate and find a matching public key corresponding to the private key in the credential, it can be considered live and verified.

Let ma update the comments to document that.

[rosecodym]

I think I see a way to support indeterminacy (commented inline) - is that possible?

(And out of curiosity, what prompted this change?)

[ahrav]

I think I see a way to support indeterminacy (commented inline) - is that possible?

(And out of curiosity, what prompted this change?)

The team at Google requested we use this approach instead.

[rgmz]

Are both string and bool return types necessary? If bool is only true when string != "", you could return (string, error) and just check whether string != "".

(Idk, the three return values confused me while reading this.)

[rgmz]

Is it worth handling these errors? The input is coming from a trusted API in a known format; I'd think that any failures would be highly unusual and something worth looking at.

[rgmz]

Would it make sense to run this prior to the verify block? That way, if the key isn't valid (e.g., ---- BEGIN PRIVATE KEY ----- \n<YOUR-KEY-HERE>\n---- END PRIVATE KEY -----) it doesn't need to make a network request. It would also filter out invalid results if verification is disabled/unverified results are being shown; right now, there is no false positive check.