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

feat: AWS public IPv4 spec #3741

Merged
merged 0 commits into from
Apr 14, 2023
Merged

feat: AWS public IPv4 spec #3741

merged 0 commits into from
Apr 14, 2023

Conversation

lzap
Copy link
Contributor

@lzap lzap commented Apr 12, 2023

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

This tries to add an AWS public IP address, that is not known to the machine itself.

Issue: #3731
Replaces: #3732

Refs ESSNTL-4588

I would like to do the same for Azure and GCP too, would you prefer a single PR or multiple PRs?

This is my first contribution, review carefully and please guide me if I miss something. Thanks!

Copy link
Contributor

@xiangce xiangce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lzap - Thanks for contributing to insights-core. As mentioned in the #3732 (here) please submit MRs to the insights-core-assets to get these newly added specs approved first.

And, There is another important piece of information need to let you know: The Insights provides IP/Hostname obfuscation feature for the customer to protect their Personal Identifiable Information (PII), see this KCS-2047593. That is to say, when the customer enabled this feature on their hosts, the IP address and hostname that appeared in all the collected files/commands will be obfuscated to a fake one, including these 2 new specs. Please check if this obfuscation feature would affect you or not.

Rather than the two comments to the spec name, this PR looks good to me.

insights/specs/default.py Outdated Show resolved Hide resolved
insights/specs/default.py Outdated Show resolved Hide resolved
insights/parsers/aws_instance_id.py Show resolved Hide resolved
* ``curl -s http://169.254.169.254/latest/dynamic/instance-identity/pkcs7``
* ``curl -s http://169.254.169.254/latest/dynamic/instance-identity/pkcs7`` and
* ``curl -s http://169.254.169.254/latest/meta-data/public-ipv4`` and
* ``curl -s http://169.254.169.254/latest/meta-data/public-hostname``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may forget to add a parser for this AWSPublicHostname?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, yep.

@xiangce
Copy link
Contributor

xiangce commented Apr 13, 2023

As mentioned in the #3732 (#3732 (comment)) please submit MRs to the insights-core-assets to get these newly added specs approved first.

Aha, I just saw your issue to the insights-core-assets, please ignore this noise.

@lzap
Copy link
Contributor Author

lzap commented Apr 13, 2023

Thanks for the review, will update the patch shortly. So it looks like IP/hostname is identified as Personal Identifiable Information, what should I change in order to allow filtering. I am assuming that this needs to be added then: RegistryPoint(filterable=True)

@xiangce
Copy link
Contributor

xiangce commented Apr 13, 2023

It's different from the filterable=True, the Obfuscation feature, per the policy, can only be controlled by the customer. Although technically we can disable it on specified files/commands, it only allows in case the file/command contains IP/hostname like strings (to prevent false positives) but not real IP/hostname. That's to say, if the customers enabled it on their side, we are not allowed to get these real IP/hostname.

@lzap
Copy link
Contributor Author

lzap commented Apr 13, 2023

I see but where do I define that these newly two added specs are in fact "obfuscte-able"? Is there some flag I need to set?

Edit: Ah I see this is sort of "automatic search and replace" filter I guess?

@lzap lzap force-pushed the aws-spec branch 3 times, most recently from 9433a22 to a1a2251 Compare April 13, 2023 09:13
@xiangce
Copy link
Contributor

xiangce commented Apr 13, 2023

A very easy way will be introduced by #3679 (see this sample to enable, disable by default), but before this PR#3679 , it's a bit complex to achieve this.

Edit: not so complex, see here, but I recommend you get it approved (not the approval of the spec itself, but to approve it not being obfuscated) before adding it to the excluded list.

@lzap
Copy link
Contributor Author

lzap commented Apr 13, 2023

Testing on EC2:

[root@ip-172-31-31-20 ~]# BYPASS_GPG=true EGG=/root/insights.zip insights-client --offline --no-upload --keep-archive --no-gpg
Starting to collect Insights data for ip-172-31-31-20.eu-central-1.compute.internal
Copying archive from /var/tmp/insights-client/insights-archive-cppjczc4/insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416.tar.gz to /var/cache/insights-client/insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416.tar.gz
Archive saved at /var/cache/insights-client/insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416.tar.gz

cat insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416/meta_data/insights.specs.Specs.aws_public_ipv4_addresses.json | jq
{
  "name": "insights.specs.Specs.aws_public_ipv4_addresses",
  "exec_time": 0.00017786026000976562,
  "errors": [],
  "results": {
    "type": "insights.core.spec_factory.CommandOutputProvider",
    "object": {
      "rc": null,
      "cmd": "/usr/bin/curl -s -H \"X-aws-ec2-metadata-token: AQAAALD7B07-67_T570-rLMXd66RWTR_dKR2q-6GaDc4xH1YJshb_A==\" http://169.254.169.254/latest/meta-data/public-ipv4 --connect-timeout 5",
      "args": null,
      "relative_path": "insights_commands/curl_-s_-H_X-aws-ec2-metadata-token_AQAAALD7B07-67_T570-rLMXd66RWTR_dKR2q-6GaDc4xH1YJshb_A_http_..169.254.169.254.latest.meta-data.public-ipv4_--connect-timeout_5"
    }
  },
  "ser_time": 0.009093761444091797
}

cat insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416/meta_data/insights.specs.Specs.aws_public_hostnames.json | jq
{
  "name": "insights.specs.Specs.aws_public_hostnames",
  "exec_time": 0.0001914501190185547,
  "errors": [],
  "results": {
    "type": "insights.core.spec_factory.CommandOutputProvider",
    "object": {
      "rc": null,
      "cmd": "/usr/bin/curl -s -H \"X-aws-ec2-metadata-token: AQAAALD7B07-67_T570-rLMXd66RWTR_dKR2q-6GaDc4xH1YJshb_A==\" http://169.254.169.254/latest/meta-data/public-hostname --connect-timeout 5",
      "args": null,
      "relative_path": "insights_commands/curl_-s_-H_X-aws-ec2-metadata-token_AQAAALD7B07-67_T570-rLMXd66RWTR_dKR2q-6GaDc4xH1YJshb_A_http_..169.254.169.254.latest.meta-data.public-hostname_--connect-timeout_5"
    }
  },
  "ser_time": 0.009180307388305664
}

[root@ip-172-31-31-20 ~]# cat insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416/data/insights_commands/curl_-s_-H_X-aws-ec2-metadata-token_AQAAALD7B07-67_T570-rLMXd66RWTR_dKR2q-6GaDc4xH1YJshb_A_http_..169.254.169.254.latest.meta-data.public-ipv4_--connect-timeout_5
18.194.237.99[root@ip-172-31-31-20 ~]# 
[root@ip-172-31-31-20 ~]# cat insights-ip-172-31-31-20.eu-central-1.compute.internal-20230413100416/data/insights_commands/curl_-s_-H_X-aws-ec2-metadata-token_AQAAALD7B07-67_T570-rLMXd66RWTR_dKR2q-6GaDc4xH1YJshb_A_http_..169.254.169.254.latest.meta-data.public-hostname_--connect-timeout_5
ec2-18-194-237-99.eu-central-1.compute.amazonaws.com[root@ip-172-31-31-20 ~]#

Data seems fine.

@xiangce
Copy link
Contributor

xiangce commented Apr 13, 2023

To enable it, edit the following two options in the /etc/insights-client/insights-client.conf

# Obfuscate IP addresses
obfuscate=True

# Obfuscate hostname. Requires obfuscate=True.
obfuscate_hostname=True

@lzap
Copy link
Contributor Author

lzap commented Apr 13, 2023

I don’t think we want to exclude these new specs from being obfuscated, so I think we can proceed.

@xiangce xiangce merged commit 2cbd09e into RedHatInsights:master Apr 14, 2023
xiangce pushed a commit that referenced this pull request Apr 14, 2023
Signed-off-by: Lukas Zapletal <[email protected]>
(cherry picked from commit 2cbd09e)
@lzap lzap deleted the aws-spec branch April 14, 2023 07:00
@lzap lzap mentioned this pull request Apr 14, 2023
3 tasks
@lzap lzap mentioned this pull request Apr 24, 2023
3 tasks
xiangce pushed a commit that referenced this pull request Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants