Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Datadog trace headers when using AWS IAM authentication #3719

Closed
ryan-breidenbach opened this issue Oct 13, 2023 · 5 comments · Fixed by #3796 or #3836
Closed

Allow Datadog trace headers when using AWS IAM authentication #3719

ryan-breidenbach opened this issue Oct 13, 2023 · 5 comments · Fixed by #3796 or #3836

Comments

@ryan-breidenbach
Copy link

Currently, dd-trace explicitly omits trace HTTP headers if a request uses AWS IAM authentication. This behavior was introduced by this change that was made in 2018.

The comments in the change suggest that adding Datadog headers will break the authentication of the signature and cause 403 errors. However, this is not the case. When the signature is created, the headers used to calculate the signature are captured in the SignedHeaders component of the Authorization header. Any additional headers that are added after the signature is generated will be ignored.

We work with many applications that use IAM authentication for service-to-service communication. Not being able to see traces span all of our applications is hindering our ability to support and troubleshoot these applications. I would like the ability to at least "opt-in" for allowing headers when using IAM authentication.

@tlhunter
Copy link
Member

I'm reverting the commit that I made to re-enable these headers on AWS requests:
#3799

Both @rochdev and @astuyve believe the change will cause a lot of failures. Will need to build out better tests to figure this out.

@astuyve
Copy link
Collaborator

astuyve commented Nov 15, 2023

Hey I'm sorry there was some confusion here on my end, unless the headers are explicitly part of the signature (present in SignedHeaders), it should be possible to add additional headers after signing without causing issues. I think #3796 would be okay, and we can test this in an integration test in something like Lambda to verify. But it seems like we had some reported issues, so I'd be curious to see what those are before making any further changes.

@rochdev
Copy link
Member

rochdev commented Nov 15, 2023

But it seems like we had some reported issues

That's the main concern, we used to not handle this at all and we received several reports that applications were starting to error. Since there hasn't been any changes to the signature version, I would expect a regression of any cases that were not working before we introduced the check.

tlhunter added a commit that referenced this issue Nov 15, 2023
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
tlhunter added a commit that referenced this issue Nov 15, 2023
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
pull bot pushed a commit to astradot/dd-trace-js that referenced this issue Nov 15, 2023
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts DataDog#205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes DataDog#3719
@tlhunter
Copy link
Member

Here's a temporary workaround I had provided for a user. In short, it's possible to install a version of the tracer between 4.18 and 4.19 that has this change included in it:

"dependencies": {
  "dd-trace": "git://github.com/DataDog/dd-trace-js.git#14c1eb0ba7bb1648881affb1f1aa520f94bc5fbc"
}

This isn't something we recommend long term though. One user tried it and everything apparently worked out fine. If anyone else tries this please report back with your results!

Due to the risky nature of this change we might release it as a feature locked behind a configuration flag. But the more feedback we get on if this works or breaks the better.

@janos-kasa
Copy link

We were able to use this version and all requests correctly went through the IAM Authentication between our services, and DD was able to connect the trace.
I also support the configuration flag idea.

tlhunter added a commit that referenced this issue Nov 30, 2023
tlhunter added a commit that referenced this issue Nov 30, 2023
tlhunter added a commit that referenced this issue Dec 1, 2023
tlhunter added a commit that referenced this issue Dec 1, 2023
khanayan123 pushed a commit that referenced this issue Jan 2, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
khanayan123 pushed a commit that referenced this issue Jan 2, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
khanayan123 pushed a commit that referenced this issue Jan 2, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
khanayan123 pushed a commit that referenced this issue Jan 2, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
khanayan123 pushed a commit that referenced this issue Jan 2, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
khanayan123 pushed a commit that referenced this issue Jan 2, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts #205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes #3719
erikvanbrakel pushed a commit to mazedesignhq/dd-trace-js that referenced this issue Jul 5, 2024
- removes the code that disabled tracing headers when amazon signature headers were present
- essentially reverts DataDog#205
- was originally done to prevent breaking AWS signatures
- apparently the presence of our headers does not break amazon signatures after all
- fixes DataDog#3719
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants