Skip to content

Switch to faraday_middleware-aws-sigv4 for signing Elasticsearch request

Changzheng Liu requested to merge 37931-upgrade-faraday-aws-sigv4 into master

What does this MR do?

  • Replace faraday_middleware-aws-signers-v4 with faraday_middleware-aws-sigv4
  • Upgrade aws-sdk to v3, and because v3 contains too many stuff, only pick the ones that are being used
    • aws-sdk-core
    • aws-sdk-cloudformation
    • aws-sdk-s3
  • Change arguments names of fmid.request API call
    • aws_signers_v4 -> aws_sigv4. This one is required by the gem change
    • credentials -> credentails_provider. This one is less obvious. Please see explanation below.
Change credentials to credentails_provider

There was another try before to upgrade the gems. However, there was an unhandled exception that caused omnibus-gitlab#5029 (closed) . From the surface, it's due to the newer aws-sdk (v3) does not have deprecated method used in v2.

The v2 version is at, https://github.com/aws/aws-sdk-ruby/blob/version-2/aws-sdk-core/lib/aws-sdk-core/credential_provider.rb#L18 .

And in v3, you can't fetch AWS credentials id and secret directly. https://github.com/aws/aws-sdk-ruby/blob/master/gems/aws-sdk-core/lib/aws-sdk-core/credential_provider.rb

The fix for that is based on the logic at, https://github.com/aws/aws-sdk-ruby/blob/master/gems/aws-sigv4/lib/aws-sigv4/signer.rb#L607-L615

Before we passed in options with credentials: creds, and aws-sigv4 code above creates a wrap StaticCredentialsProvider outside. When fetching the credentials at https://github.com/aws/aws-sdk-ruby/blob/master/gems/aws-sigv4/lib/aws-sigv4/signer.rb#L661-L669 , it returned provider, not the credentials themselves.

Instead, we can use credentials_provider: creds to bypass that wrapper.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Closes #37931 (closed)

Edited by Changzheng Liu

Merge request reports

Loading