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 #75

Closed
wants to merge 3 commits into from

Conversation

mtardy
Copy link
Member

@mtardy mtardy commented Dec 19, 2022

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!

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 19, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mtardy
Once this PR has been reviewed and has the lgtm label, please assign tabbysable for approval by writing /assign @tabbysable in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 19, 2022
@mtardy
Copy link
Member Author

mtardy commented Dec 19, 2022

/cc @PushkarJ @nehaLohia27

@mtardy
Copy link
Member Author

mtardy commented Dec 19, 2022

It seems that the fields are already translated at some points on the part that creates the real feed with the header, I'm not sure why it's useful to have these multiple versions and not have directly the whole header here. But anyway, it would require modifying (notice the commit hash starting with cafe ☕️):

I don't know the surely good reasons behind everything already being translated, but maybe we can simplify that?

@sftim
Copy link
Contributor

sftim commented Dec 19, 2022

If we make this change, does the website then break?

@sftim
Copy link
Contributor

sftim commented Dec 19, 2022

https://github.com/kubernetes/website/blob/cafe6d258c91c3814d83c0655c8c6354e3eade1c/layouts/_default/cve-feed.json is localized (using T()). For JSON it may well not need to be.

I reckon we could drop:

[cve_json_external_url]
other = "external_url"

[cve_json_id]
other = "id"

[cve_json_summary]
other = "summary"

[cve_json_url]
other = "url"

@mtardy
Copy link
Member Author

mtardy commented Dec 19, 2022

If we make this change, does the website then break?

I think so! Then:
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 19, 2022
@mtardy
Copy link
Member Author

mtardy commented Dec 20, 2022

I added the date_published field with the date of the issue creation in 215c89c.

In theory, there is the date_modified field that could fit with the GitHub issue's updated_at but this date is updated when any message or reaction is posted to the issue, so I don't think it's relevant or even correct to include it since the CVE and the CVE message were a priori not modified.

@sftim
Copy link
Contributor

sftim commented Dec 20, 2022

Optional enhancement: find out when the issue title or issue description were last modified.

Definitely something for a future PR (ie, not this one). I think it's possible.

@mtardy
Copy link
Member Author

mtardy commented Dec 20, 2022

Optional enhancement: find out when the issue title or issue description were last modified.

Definitely something for a future PR (ie, not this one). I think it's possible.

Good idea! Btw I think this fix was okay to include in this PR because date_published is part of the specification, so it might make sense to bundle all of this together. Or I could still cut it into two PRs but the second one would be dependent on the first one.

@mtardy mtardy changed the title Fix CVE feed: comply with the JSON stream specifications Fix CVE feed: comply with the JSON feed specifications Dec 20, 2022
@mtardy
Copy link
Member Author

mtardy commented Jan 22, 2023

This PR was closed in favor of PR #76. All the modifications and fixes were merged into that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. 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: JSON feed should pass jsonfeed spec validator
3 participants