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

Fix CVE feed: comply with the JSON feed specifications and add the full JSON feed object in the script output to add last_updated root fields #76

Merged
merged 4 commits into from
Feb 27, 2023

Conversation

mtardy
Copy link
Member

@mtardy mtardy commented Dec 20, 2022

This PR bundles another PR #75. All the changes were included in this one to make it simpler.

This is making modifications to the output of the CVE feed scripts and thus should be synchronized with the merge of an according PR on the k/website side. This is the PR on the k/website site: kubernetes/website#38579.

This is the content of PR #75:

Fixes kubernetes/website#36808.

I simplified the code logically and added documentation on why it's parsing the way it's parsing.
Anecdotally I change all double quotes to single quotes.

I took some arbitrary decisions:

  • transform cve_id into existing field id;
  • transform issue_url into existing field url;
  • transform cve_url into existing field external_url;
  • use _kubernetes_io as custom extension;
  • transform number into issue_number for clarity.

I didn't add it because it was maybe too much but it would be trivial to add optional fields date_published and date_modified using GitHub issues metadata.

The CVE feed looks similar to this after the fix (I only used the two last CVEs for the example, note that the indention is not exact):

{
  "version": "https://jsonfeed.org/version/1.1",
  "title": "Auto-refreshing Official CVE Feed",
  "home_page_url": "https://kubernetes.io",
  "feed_url": "https://kubernetes.io/docs/reference/issues-security/official-cve-feed/index.json",
  "description": "Auto-refreshing official CVE feed for Kubernetes repository",
  "authors": [
    {
      "name": "Kubernetes Community",
      "url": "https://www.kubernetes.dev"
    }
  ],
  "items": [
        {
            "_kubernetes_io": {
                "google_group_url": "https://groups.google.com/g/kubernetes-announce/search?q=CVE-2022-3294",
                "issue_number": 113757
            },
            "content_text": "CVSS Rating: [CVSS:3.1/AV:N/AC:H/PR:H/UI:N/S:U/C:H/I:H/A:H](https://www.first.org/cvss/calculator/3.1#CVSS:3.1/AV:N/AC:H/PR:H/UI:N/S:U/C:H/I:H/A:H)\r\n\r\nA security issue was discovered in Kubernetes where users may have access to secure endpoints in the control plane network. Kubernetes clusters are only affected if an untrusted user can modify Node objects and send proxy requests to them.\r\n\r\nKubernetes supports node proxying, which allows clients of kube-apiserver to access endpoints of a Kubelet to establish connections to Pods, retrieve container logs, and more. While Kubernetes already validates the proxying address for Nodes, a bug in kube-apiserver made it possible to bypass this validation. Bypassing this validation could allow authenticated requests destined for Nodes to to the API server's private network.\r\n\r\n### Am I vulnerable?\r\n\r\nClusters are affected by this vulnerability if there are endpoints that the kube-apiserver has connectivity to that users should not be able to access. This includes:\r\n\r\n- kube-apiserver is in a separate network from worker nodes\r\n- localhost services\r\n\r\nmTLS services that accept the same client certificate as nodes may be affected. The severity of this issue depends on the privileges & sensitivity of the exploitable endpoints.\r\n\r\nClusters that configure the egress selector to use a proxy for cluster traffic may not be affected.\r\n\r\n\r\n#### Affected Versions\r\n\r\n- Kubernetes kube-apiserver <= v1.25.3\r\n- Kubernetes kube-apiserver <= v1.24.7\r\n- Kubernetes kube-apiserver <= v1.23.13\r\n- Kubernetes kube-apiserver <= v1.22.15\r\n\r\n### How do I mitigate this vulnerability?\r\n\r\nUpgrading the **kube-apiserver** to a fixed version mitigates this vulnerability.\r\n\r\nAside from upgrading, configuring an [egress proxy for egress to the cluster network](https://kubernetes.io/docs/tasks/extend-kubernetes/setup-konnectivity/) can mitigate this vulnerability.\r\n\r\n#### Fixed Versions\r\n\r\n- Kubernetes kube-apiserver v1.25.4\r\n- Kubernetes kube-apiserver v1.24.8\r\n- Kubernetes kube-apiserver v1.23.14\r\n- Kubernetes kube-apiserver v1.22.16\r\n\r\n**Fix impact:** In some cases, the fix can break clients that depend on the nodes/proxy subresource, specifically if a kubelet advertises a localhost or link-local address to the Kubernetes control plane.\r\n\r\n### Detection\r\n\r\nNode create & update requests may be included in the Kubernetes audit log, and can be used to identify requests for IP addresses that should not be permitted. Node proxy requests may also be included in audit logs.\r\n\r\nIf you find evidence that this vulnerability has been exploited, please contact security@kubernetes.io\r\n\r\n#### Acknowledgements\r\n\r\nThis vulnerability was reported by Yuval Avrahami of Palo Alto Networks.\r\n\r\n<!-- labels -->\r\n/area security\r\n/kind bug\r\n/committee security-response\r\n/label official-cve-feed\r\n/sig api-machinery\r\n/area apiserver",
            "external_url": "https://www.cve.org/cverecord?id=CVE-2022-3294",
            "id": "CVE-2022-3294",
            "summary": "Node address isn't always verified when proxying",
            "url": "https://github.com/kubernetes/kubernetes/issues/113757"
        },
        {
            "_kubernetes_io": {
                "google_group_url": "https://groups.google.com/g/kubernetes-announce/search?q=CVE-2022-3162",
                "issue_number": 113756
            },
            "content_text": "CVSS Rating: [CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N](https://www.first.org/cvss/calculator/3.0#CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N)\r\n\r\nA security issue was discovered in Kubernetes where users authorized to list or watch one type of namespaced custom resource cluster-wide can read custom resources of a different type in the same API group without authorization.\r\n\r\n### Am I vulnerable?\r\n\r\nClusters are impacted by this vulnerability if all of the following are true:\r\n- There are 2+ CustomResourceDefinitions sharing the same API group\r\n- Users have cluster-wide list or watch authorization on one of those custom resources.\r\n- The same users are not authorized to read another custom resource in the same API group.\r\n\r\n#### Affected Versions\r\n\r\n- Kubernetes kube-apiserver <= v1.25.3\r\n- Kubernetes kube-apiserver <= v1.24.7\r\n- Kubernetes kube-apiserver <= v1.23.13\r\n- Kubernetes kube-apiserver <= v1.22.15\r\n\r\n### How do I mitigate this vulnerability?\r\n\r\nUpgrading the kube-apiserver to a fixed version mitigates this vulnerability.\r\n\r\nPrior to upgrading, this vulnerability can be mitigated by avoiding granting cluster-wide list and watch permissions.\r\n\r\n#### Fixed Versions\r\n\r\n- Kubernetes kube-apiserver v1.25.4\r\n- Kubernetes kube-apiserver v1.24.8\r\n- Kubernetes kube-apiserver v1.23.14\r\n- Kubernetes kube-apiserver v1.22.16\r\n\r\n### Detection\r\n\r\nRequests containing `..` in the request path are a likely indicator of exploitation. Request paths may be captured in API audit logs, or in kube-apiserver HTTP logs.\r\n\r\nIf you find evidence that this vulnerability has been exploited, please contact security@kubernetes.io\r\n\r\n#### Acknowledgements\r\n\r\nThis vulnerability was reported by Richard Turnbull of NCC Group as part of the Kubernetes Audit.\r\n\r\n<!-- labels -->\r\n/area security\r\n/kind bug\r\n/committee security-response\r\n/label official-cve-feed\r\n/sig api-machinery\r\n/area apiserver\r\n",
            "external_url": "https://www.cve.org/cverecord?id=CVE-2022-3162",
            "id": "CVE-2022-3162",
            "summary": "Unauthorized read of Custom Resources",
            "url": "https://github.com/kubernetes/kubernetes/issues/113756"
        }
    ]
}

On a side and useless note, is it useful that the scripts output the JSON dumps? Otherwise, I think it would be more elegant that it just prints to STDOUT the result and that the other bash scripts python3 script.py > the_file.json. But this one is really a nit!

This is the old content of this PR:

This resolves #72.

I don't know if it's the best way to proceed but this PR includes the commits from #75, so this should be merged into this PR or better, after!
Identically, it needs some changes on the k/website side.
/hold

So the unique commit this PR brings is e61ffa9.
I took the liberty to name it updated_at, but we have plenty of possibilities here and maybe we can find something better? Or more in line with the Kubernetes API? I don't know.

/cc @PushkarJ @nehaLohia27

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 20, 2022
@PushkarJ
Copy link
Member

PushkarJ commented Jan 3, 2023

@mtardy thank you so much for the updates!!!

If you have resolved the suggestions from #75 already in this PR, we can close the #75 without merging and then include all the changes from that PR in this one which we will merge along with kubernetes/website#38579

Does that sound like a reasonable next step?

Copy link
Member

@PushkarJ PushkarJ left a comment

Choose a reason for hiding this comment

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

Some really minor suggestions otherwise looks good.

I will review kubernetes/website#38579 later next when I get a chance

@mtardy mtardy force-pushed the full-json-feed-envelope branch 2 times, most recently from 071e7bd to ef13af4 Compare January 22, 2023 17:00
@mtardy mtardy changed the title Add the full JSON feed object in the script output to add last_updated root fields Fix CVE feed: comply with the JSON feed specifications and add the full JSON feed object in the script output to add last_updated root fields Jan 22, 2023
@PushkarJ
Copy link
Member

/lgtm
/approve
/hold

(@mtardy please unhold when k/website PR is ready to merge)

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 20, 2023
@PushkarJ
Copy link
Member

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 20, 2023
@sftim
Copy link
Contributor

sftim commented Feb 20, 2023

I think it might be helpful not to squash merge this one. Instead, @mtardy, you can squash this locally and force-push.

@sftim
Copy link
Contributor

sftim commented Feb 20, 2023

I also think it's helpful to keep fac4ebd (Comply with JSON feed specifications) as its own commit.

@mtardy
Copy link
Member Author

mtardy commented Feb 20, 2023

I think it might be helpful not to squash merge this one. Instead, @mtardy, you can squash this locally and force-push.

Yes sure, It seems that some commits might be better on their own.

Simplify the output by printing to STDOUT and using tee to duplicate the
STDOUT in a file in the bash script. Fix typos in comments and add
explanations about usage of tee

Co-authored-by: Pushkar Joglekar <3390906+PushkarJ@users.noreply.github.com>
@mtardy
Copy link
Member Author

mtardy commented Feb 20, 2023

Honestly, I would just remove the last commit ef13af4 and squash it.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2023
@mtardy
Copy link
Member Author

mtardy commented Feb 20, 2023

/remove-label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot removed the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 20, 2023
@PushkarJ
Copy link
Member

/lgtm
/approve
/hold

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtardy, PushkarJ

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mtardy
Copy link
Member Author

mtardy commented Feb 27, 2023

🥳
/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 27, 2023
@k8s-ci-robot k8s-ci-robot merged commit 83570c8 into kubernetes:main Feb 27, 2023
@mtardy mtardy deleted the full-json-feed-envelope branch February 27, 2023 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CVE Feed: Add lastUpdatedAt as a metadata field CVE Feed: JSON feed should pass jsonfeed spec validator
4 participants