opensearch-project / opensearch-go

Go Client for OpenSearch
https://opensearch.org/docs/latest/clients/go/
Apache License 2.0
188 stars 96 forks source link

[BUG] Invalid Sigv4 on requests without a body using AWS SDK v1 #492

Closed rblcoder closed 3 months ago

rblcoder commented 3 months ago

What is the bug?

Getting Error: status: [403 Forbidden] exit status 1 when connecting to OpenSearch using AWS SDK v1

How can one reproduce the bug?

The following code gives Error: status: [403 Forbidden] exit status 1

package main

import (
    "context"
    "fmt"
    "os"

    "github.com/aws/aws-sdk-go/aws/session"
    requestsigner "github.com/opensearch-project/opensearch-go/v3/signer/aws"

    "github.com/opensearch-project/opensearch-go/v3"
    "github.com/opensearch-project/opensearch-go/v3/opensearchapi"
)

const IndexName = "go-test-index1"

func main() {
    if err := example(); err != nil {
        fmt.Println(fmt.Sprintf("Error: %s", err))
        os.Exit(1)
    }
}

const endpoint = "endpoint url" // e.g. https://opensearch-domain.region.com

func example() error {
    // Create an AWS request Signer and load AWS configuration using default config folder or env vars.
    // See https://docs.aws.amazon.com/opensearch-service/latest/developerguide/request-signing.html#request-signing-go
    signer, err := requestsigner.NewSignerWithService(
        session.Options{Profile: "default"},
        requestsigner.OpenSearchService, // Use requestsigner.OpenSearchServerless for Amazon OpenSearch Serverless.
    )
    if err != nil {
        return err
    }
    // Create an opensearch client and use the request-signer.
    client, err := opensearchapi.NewClient(
        opensearchapi.Config{
            Client: opensearch.Config{
                Addresses: []string{endpoint},
                Signer:    signer,
            },
        },
    )
    if err != nil {
        return err
    }

    ctx := context.Background()

    ping, err := client.Ping(ctx, nil)
    if err != nil {
        return err
    }

    fmt.Println(ping)

    return nil
}

What is the expected behavior?

Connection should be successful

What is your host/environment?

Distributor ID: Debian Description: Debian GNU/Linux 11 (bullseye) Release: 11 Codename: bullseye

Do you have any screenshots?

golangawssdkopensearch

Do you have any additional context?

I am able to connect using AWS SDK v1 with the same credentials.

dblock commented 3 months ago

I don't see any region specified, maybe you're just missing that?

Can you try running my working sample in https://github.com/dblock/opensearch-go-client-demo, does it work or cause the same error?

rblcoder commented 3 months ago

opensearch-go-client-demo uses AWS SDK V2. The code with AWS SDK v2 works perfectly. I was trying to get the code work for AWS SDK v1. I tried adding region and am getting the same error.

package main

import (
    "context"
    "fmt"
    "os"

    "github.com/aws/aws-sdk-go/aws"
    "github.com/aws/aws-sdk-go/aws/session"
    requestsigner "github.com/opensearch-project/opensearch-go/v3/signer/aws"

    "github.com/opensearch-project/opensearch-go/v3"
    "github.com/opensearch-project/opensearch-go/v3/opensearchapi"
)

const IndexName = "go-test-index1"

func main() {
    if err := example(); err != nil {
        fmt.Println(fmt.Sprintf("Error: %s", err))
        os.Exit(1)
    }
}

const endpoint = "url" // e.g. https://opensearch-domain.region.com

func example() error {
    // Create an AWS request Signer and load AWS configuration using default config folder or env vars.
    // See https://docs.aws.amazon.com/opensearch-service/latest/developerguide/request-signing.html#request-signing-go
    signer, err := requestsigner.NewSignerWithService(
        session.Options{Profile: "default",
            Config: aws.Config{
                Region: aws.String("us-east-1"),
            }},
        requestsigner.OpenSearchService, // Use requestsigner.OpenSearchServerless for Amazon OpenSearch Serverless.
    )
    if err != nil {
        return err
    }
    // Create an opensearch client and use the request-signer.
    client, err := opensearchapi.NewClient(
        opensearchapi.Config{
            Client: opensearch.Config{
                Addresses: []string{endpoint},
                Signer:    signer,
            },
        },
    )
    if err != nil {
        return err
    }

    ctx := context.Background()

    ping, err := client.Ping(ctx, nil)
    if err != nil {
        return err
    }

    fmt.Println(ping)

    return nil
}
dblock commented 3 months ago

I can confirm that it doesn't work for requests without a body (works for PUT, POST, etc., but not GET or DELETE). Made a v1 version in https://github.com/dblock/opensearch-go-client-demo/tree/aws-sdk-v1/aws-sdk-v1 and I get an invalid signature for those as well.

It's probably something we broke in the signer in the way empty body is handled, or something that changed on the server. Maybe you can help debug/fix?

dblock commented 3 months ago

Fix in https://github.com/opensearch-project/opensearch-go/pull/496. Looks like this was introduced in https://github.com/opensearch-project/opensearch-go/commit/efe6d62ff3cb6fcaaa59f0f151f77d193da2924a when serverless support was added, I bet the empty body signature is required, but not checked in serverless on GET/DELETE, cc: @harrisonhjones.

I tested with AWS SDK v1 with https://github.com/dblock/opensearch-go-client-demo/tree/aws-sdk-v1 against both managed and serverless by adding replace "github.com/opensearch-project/opensearch-go/v3" => "...../opensearch-go/dblock-opensearch-go" in go.mod.

rblcoder commented 3 months ago

Thank you @dblock , I was trying to understand the code and it was taking time.

I can confirm that it doesn't work for requests without a body (works for PUT, POST, etc., but not GET or DELETE). Made a v1 version in https://github.com/dblock/opensearch-go-client-demo/tree/aws-sdk-v1/aws-sdk-v1 and I get an invalid signature for those as well.

It's probably something we broke in the signer in the way empty body is handled, or something that changed on the server. Maybe you can help debug/fix?

dblock commented 3 months ago

@rblcoder My PR got merged, can you try the version from HEAD? We can cut a release if it works well for you.

rblcoder commented 3 months ago

@dblock tested on the version from HEAD, works perfectly now.

harrisonhjones commented 3 months ago

Sorry for the bug on my end folks. Thanks for fixing it!

dblock commented 3 months ago

Sorry for the bug on my end folks. Thanks for fixing it!

No stress! Looks like AOSS was not enforcing the header check (I suppose it makes sense because SSL is required), so this took a while to get noticed.