-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Description
This issue was reported in the Kubernetes Security Audit Report
Description
Kubernetes includes a linkcheck command, intended to check the correctness of links within Kubernetes documentation. The command uses the strconv.Atoi function to parse the Retry-After response header. However, the backoff value that is later used to wait between making another linkcheck attempt is updated only if strconv.Atoi returns an error. In other words, a valid value of Retry-After header value is not used by linkcheck:
if resp.StatusCode == http.StatusTooManyRequests {
retryAfter := resp.Header.Get("Retry-After")
if seconds, err := strconv.Atoi(retryAfter); err != nil {
backoff = seconds + 10
}
fmt.Fprintf(os.Stderr, "Got %d visiting %s, retry after %d seconds.\n", resp.StatusCode, string(URL), backoff)
time.Sleep(time.Duration(backoff) * time.Second)
backoff *= 2
retry++
Figure 34.1: Retry-After header parsing in linkcheck command
Furthermore, note that:
- strconv.Atoi uses strconv.ParseInt, which has edge cases for parser failures, which will return MAXINT for values too large and zero for values that fail to parse as integers.
- linkcheck does not set an explicit redirect policy via CheckRedirect, meaning that Go’s core HTTP library will follow redirects. While linkcheck has a whitelist for URLs, redirects are not checked against the whitelist.
- The Retry-After response header is used only in the case of “Too many Requests” (HTTP status code 429). However, the Retry-After header may also be returned with both 301 (Redirect) and 503 (Service Unavailable) status codes as well.
Exploit Scenario
Alice wishes to build a copy of Kubernetes. Unbeknownst to Alice, Eve has inserted a link into the documentation that returns a redirect to a URL that was initially blocked by the linkcheck link regex whitelist.
Recommendation
Short term, use the retryAfter value and always include reasonable minimum and maximum values for all untrusted data. These should meet the roughly-expected timelines of an operation. For example, if a server responds with a Retry-After header that is longer than 1 minute, mark the link as inactive and continue on. Define a CheckRedirect policy for HTTP clients, and ensure that developers and end-users may control if they want redirects to be followed.
Long term, ensure that all timeouts and similar operations have both a maximum and a minimum value. This will prevent events from happening more frequently than developers expect, or from taking too long under imperfect situations. Additionally, ensure that any code that relies on standards such as HTTP adequately follows the standard.
Anything else we need to know?:
See #81146 for current status of all issues created from these findings.
The vendor gave this issue an ID of TOB-K8S-003 and it was finding 35 of the report.
The vendor considers this issue Informational Severity.
To view the original finding, begin on page 83 of the Kubernetes Security Review Report
Environment:
- Kubernetes version: 1.13.4