https://github.com/osquery/osquery logo
Powered by
Title
s

suppandi

04/20/2021, 3:12 AM
Hi, Are there any plans to support IMDSV2 instead of IMDSV1 for aws instance metadata. Most of accounts today disable the use of IMDSV1 due to security issues
s

seph

04/20/2021, 3:59 AM
I don’t know that anyone is working on that, but I’d be happy to review a PR.
w

WS

04/20/2021, 5:31 AM
off topic but, for someone who knows no CPP, but knows how to program. how long do you think it would take to learn CPP to the point where i can submit a PR and you don’t cringe
s

seph

04/20/2021, 12:08 PM
A couple of answers -- we try to be kind to PRs, no matter the content they usually represent work that someone did from love. Personally, and I think I speak for several of us, we're happy with people stepping in. That's about where I am -- I'm not great at c++, but I have enough other background to fake it. At least for small things. I know folks will have suggestions about how I can improve my PRs to modern style, but I've also been able to fix bugs and make things I think of as real improvements. For this fix, it seems straightforward. Where there's one network call, there should be two. I'd be happy to chat about it
n

nyanshak

04/20/2021, 3:53 PM
https://github.com/osquery/osquery/issues/6165 ^ Issue for this btw. I mentioned about ~2 weeks ago that it looks like IMDSv2 should be supported as of 4.6.0. But I'm looking for someone to confirm that. My initial testing seems to confirm that osquery can use metadata v2 (e.g., by fully disabling v1 with aws cli). However, I have another user who mentioned some problem with it to me. I'm following up with him to see if it's actually an issue with metadata or something else.
w

WS

04/20/2021, 3:55 PM
thanks seph, i really appreciate your response and im sure if others saw it that would encourage them to submit PR’s if they might have been on the fence for whatever reason really.
that is 100% the vibe i’ve gotten from folks about contributing here.
g

Gabriel Munoz

04/20/2021, 6:29 PM
@nyanshak I noticed the ping on that issue for a request to confirm and took a look at this just now. I'm running osquery version 
4.6.0-1
. I ran a query against the 
ec2_instance_metadata
table and dumping the http headers from tcpdump didn't show the 
x-aws-ec2-metadata-token
header with a token value as I would have expected. I used the test case example on the ticket to confirm `curl`'ing the endpoint directly would function to know I had a sane test case. I'm wondering if there is a flag to tell osquery to use imdsv2? My test instance is in a mode that allows both imdsv1 and imdsv2.
n

nyanshak

04/20/2021, 6:30 PM
I'm not sure, I don't think so.
g

Gabriel Munoz

04/20/2021, 6:31 PM
Do you know if the test instance you used was in IMDSv2 only mode? That is, I'm wondering if the AWS SDK will default to imdsv1 but use imdsv2 if it is exclusive to imdsv2 mode.
n

nyanshak

04/20/2021, 6:32 PM
I didn't do a tcpdump to see headers in my example. But I had an instance in v2 only mode.
Seemed to work fine for me, but I had someone else that had some issues that I'm looking into. They may or may not be related.
But would help to have some other data 🤷‍♀️ @Gabriel Munoz
g

Gabriel Munoz

04/20/2021, 6:35 PM
Ah, gotcha. Ok, that could explain my test results then. It would make sense to me that the AWS SDK would default to imdsv1 when available, which it is for my test case. I can read up on the aws docs and see if I can put an instance into imdsv2-only mode and see if that works. That sounds like the use case by the original commenter in this thread.
n

nyanshak

04/20/2021, 6:37 PM
metadata client is an instance of `AWSHttpResourceClient`: https://github.com/aws/aws-sdk-cpp/blob/bb1fdce01cc7e8ae2fe7162f24c8836e9d3ab0a2/a[…]-cpp-sdk-core/include/aws/core/internal/AWSHttpResourceClient.h not very familiar with this code, but looks to set token if 
m_tokenRequired
is set: https://github.com/aws/aws-sdk-cpp/blob/bb1fdce01cc7e8ae2fe7162f24c8836e9d3ab0a2/aws-cpp-sdk-core/source/internal/AWSHttpResourceClient.cpp#L312
ah yeah, okay I see it
g

Gabriel Munoz

04/20/2021, 6:37 PM
Oh yeah, I bet that's on the right track.
n

nyanshak

04/20/2021, 6:39 PM
Aws::String EC2MetadataClient::GetDefaultCredentials() const
        {
            std::unique_lock<std::recursive_mutex> locker(m_tokenMutex);
            if (m_tokenRequired)
            {
                return GetDefaultCredentialsSecurely();
            }
...
so yeah, I think if you have v1 enabled, it will use that, but if not, add v2 headers
g

Gabriel Munoz

04/20/2021, 6:41 PM
Yep, that's how I'm reading it too.
That would work for the original commenter of this thread. But I'm wondering if it would be possible to have a way to influence that behavior with an osquery setting or environment variable?
I can try to look at the code base too. I pinged the folks involved on the creation of the ticket too offline. Figure one of us can take a look at some point 🙂
n

nyanshak

04/20/2021, 6:45 PM
OP:
Are there any plans to support IMDSV2 instead of IMDSV1 for aws instance metadata. Most of accounts today disable the use of IMDSV1 due to security issues
Based on my reading of how osquery 4.6.0+ and aws-sdk-cpp work, I think this is already supported. Disabling v1 should just work going forward.
g

Gabriel Munoz

04/20/2021, 6:45 PM
Right, I meant having osquery use imdsv2 even when imdsv1 is present. It's a corner case for sure.
n

nyanshak

04/20/2021, 6:47 PM
I would suggest that there's no tangible benefit to doing so. The end goal of disabling v1 is to prevent SSRF attacks against metadata services. Osquery is not stopping you from doing that, so long as you're on 4.6.0+.
g

Gabriel Munoz

04/20/2021, 6:51 PM
It's more about a transition plan for infrastructure in general. Leaving osquery on imdsv1 will still show up in the cloud metrics as not doing the right thing, even though it should work when the environment is modified. It makes it a little bit of a pain to track down, which is covered in the AWS docs about how to transition. There really should be a imdsv2 default option as part of that transition plan.
But I'm happy to take a look into this aspect of the behavior.
n

nyanshak

04/20/2021, 6:54 PM
👍 ❤️
btw - should be able to enable cloudtrail logs, then use cw search to list out sdk versions looking for 
userAgent
field, example: 
"userAgent": "aws-sdk-go/1.35.23 (go1.13.14; linux; amd64) amazon-ssm-agent/",
g

Gabriel Munoz

04/20/2021, 7:06 PM
I was just looking into that too 🙂 Agreed on that.
n

nyanshak

04/20/2021, 7:07 PM
Yeah, bit familiar as I've also recently been doing some investigation into this on my end 🤷‍♀️
g

Gabriel Munoz

04/20/2021, 7:07 PM
I'm leaning towards to how the behavior is right now. We could push some code but I'm not sure it is worth it tbh. My own team(s) work around the issue by disabling the tables entirely, which we are able to do, but if folks are dependent on the tables it would be a pain point.
And if folks need the table we can just reference tracking the userAgent like you mentioned.
n

nyanshak

04/20/2021, 7:09 PM
Agreed that it's not 100% straightforward though. In our specific uses, aws-sdk-cpp is pretty much entirely osquery though, so we could quickly just make sure it was updated. Other SDKs are more complicated as we need to identify different applications using it 🤷‍♀️
g

Gabriel Munoz

04/20/2021, 7:10 PM
Yeah, another fair point.
s

suppandi

04/21/2021, 1:24 AM
@nyanshak @Gabriel Munoz - The table here https://github.com/osquery/osquery/blob/d2be385d71f401c85872f00d479df8f499164c5a/osquery/tables/cloud/aws/ec2_instance_metadata.cpp is not initializing the AWS SDK for it to get the token
https://github.com/osquery/osquery/blob/d2be385d71f401c85872f00d479df8f499164c5a/osquery/tables/cloud/aws/ec2_instance_tags.cpp
Whereas this table is using AWS SDK
Now which table is reliable in terms of ec2_instance id
n

nyanshak

04/21/2021, 3:19 PM
https://github.com/osquery/osquery/issues/6165#issuecomment-824145956 - did a bit more testing
9 Views
#generalJoin Slack
Powered by