给 Helm 提一个 PR,重温开源项目参与过程
阅读原文时间:2023年07月13日阅读:3

目录

酒足饭饱,闲来无事,打开 Helm 项目的 issues 列表,咦,这个 issue 看起来眉清目秀呀:

其实一点都不“眉清目秀”,这段 issue 描述太冗长了,没有任何排版,看起来很累。咱就看关键词吧:helm upgrade,sensitive data,error,好的,这就够了,加在一起就是说当 helm upgrade 命令执行失败的时候,可能回导致敏感数据泄露

盘它!

尽管已经有人留言认领这个任务,不过看描述似乎他并不会马上开始编码,而是希望进一步沟通方案。好办好办,转给我吧:

倒是挺干脆,分分钟就 assign 给我了。

不过,其实,这会我并不知道怎么修复这个 bug,我也不熟悉 Helm 的源码,外加已经小半年没咋写过 Golang 了,其实心里挺慌。试试看吧,究竟是宝刀未老还是老骥伏枥。

要修复 Bug,第一步自然是先看能不能稳定复现。外加 issue 描述里其实我看不出来如何复现,也不是太看清楚问题现象是什么,总之,复现问题是当务之急。

用 GPT-4 来准备 Bug 复现材料

  • 我的需求

下面就是想想自己需要啥材料,然后抛给 GPT。或许我可以这样说:

我发现 Helm 在执行 upgrade 的时候,如果出错,会导致 Secret 里的数据被明文打印出来,存在敏感数据泄露风险。现在我想复现这个问题,我设想的步骤是这样的:
1. 构造一个包含 Secret 的正常的 Helm Chart,Secret 里的数据是从 values.yaml 里读取的,values.yaml 中直接使用 base64 编码字符串,而不是在 secret.yaml 中使用 b64enc;
2. 然后执行 helm install,此刻应该没有问题异常;
3. 修改 values.yaml 中的 base64 字符串,让其非法(比如删除一个结尾的“=”);
4. 执行 helm upgrade 命令,看结果是不是存在敏感数据泄露。
请告诉我相关命令和操作等。
  • 将需求告诉 DevChat

  • DevChat 给出的完整回答

首先,我会概括你的需求:

  • 你想要复现一个问题,该问题是在使用 Helm 进行 upgrade 操作时,如果出错,会导致 Secret 中的数据被明文打印出来,存在敏感数据泄露风险。

  • 你已经设想了一些步骤,包括创建一个包含 Secret 的 Helm Chart,然后进行一些操作来触发这个问题。

    下面是我为你准备的相关命令和操作:

  1. 创建一个包含 Secret 的 Helm Chart。首先,你需要创建一个新的 Helm Chart,可以使用以下命令:

    helm create mychart

然后,你需要在 mychart/templates 目录下创建一个新的文件 secret.yaml,并添加以下内容:

apiVersion: v1
kind: Secret
metadata:
  name: mysecret
type: Opaque
data:
  mykey: {{ .Values.mysecret.mykey }}

在 mychart/values.yaml 文件中,添加以下内容:

mysecret:
  mykey: "SGVsbG8gd29ybGQ="  # "Hello world" 的 base64 编码
  1. 执行 helm install 命令。在你的 Helm Chart 目录(即 mychart)下,执行以下命令:

    helm install myrelease .

此时,应该没有任何问题或异常。

  1. 修改 values.yaml 文件中的 base64 字符串,让其非法。你可以将 values.yaml 文件中的 mykey 值修改为一个非法的 base64 字符串,例如删除一个结尾的“=”:

    mysecret:
    mykey: "SGVsbG8gd29ybGQ" # 非法的 base64 字符串

  2. 执行 helm upgrade 命令。在你的 Helm Chart 目录下,执行以下命令:

    helm upgrade myrelease .

此时,你应该能看到错误信息,包括你的 Secret 数据。

以上就是复现你所说问题的步骤。如果你在执行这些步骤时遇到任何问题,或者需要更多的帮助,欢迎随时向我提问。

对着 GPT-4 给的命令复现 Bug

DevChat 诚不我欺啊,一顿复制粘贴敲键盘,最后我看到了这个:

注意这个错误日志里的这部分:

Error: UPGRADE FAILED: cannot patch "mysecret" with kind Secret:  "" is invalid: patch: Invalid value: "{\"apiVersion\":\"v1\",\"data\":{\"mykey\":\"SGVsbG8gd29ybGQ\"}……

这里的 mykey: SGVsbG8gd29ybGQ 看起来似乎不太敏感,不过它其实意味着假如你存了10个密码在 Secret 里,当更新的时候写错了一个,其他正确的9个会被无差别打印出来。这个日志里包含的是一个未脱敏的完整的 Secret 资源实例内容。

从日志里来看,JSON 部分大概率是 K8s 相关的库返回的内容,Helm 里直接拼接到自己日志里打印出来了。我们需要找到最接近这个“K8s 相关调用”的地方,然后找到这串日志怎么来的,再加一层“脱敏”。

  1. 搜索关键字“UPGRADE FAILED”

太幸运了,只有一个结果(明显是 upgrade.go 这里),那就长刀直入吧:

rel, err := client.RunWithContext(ctx, args[0], ch, vals)
if err != nil {
    return errors.Wrap(err, "UPGRADE FAILED")
}
  1. 继续跟 RunWithContext() 方法里哪里返回了错误日志

进去 RunWithContext() 方法,基本从名字上就能判断是“执行升级过程”的时候出的错,因为一开始的错误日志里有一句“cannot patch "mysecret" with kind Secret”,这显然也不是 Helm 本身的行为,而是某个 K8s 相关的库返回的,因此肯定是开始执行的时候出的错,也就是 performUpgrade() 这个方法返回的 err 里包含了我们所寻找的错误日志。

func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
    //……
    res, err := u.performUpgrade(ctx, currentRelease, upgradedRelease)
    if err != nil {
        return res, err
    }
    //……

    return res, nil
}
  1. 继续跟 performUpgrade()

这个方法有点长,仔细看看,大概率是在结尾的时候 result.e 里包含了我们寻找的 err,而这个 result 是一个 Channel,在 releasingUpgrade 这个协程里完成了数据写入。行,下一步,继续看 releasingUpgrade()

func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedRelease *release.Release) (*release.Release, error) {
    // ……
    go u.releasingUpgrade(rChan, upgradedRelease, current, target, originalRelease)
    go u.handleContext(ctx, doneChan, ctxChan, upgradedRelease)
    select {
    case result := <-rChan:
        return result.r, result.e
    case result := <-ctxChan:
        return result.r, result.e
    }
  1. 继续看 releasingUpgrade()

到这里就差不多了,这里有关键的一行代码:u.cfg.KubeClient.Update(current, target, u.Force)

func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *release.Release, current kube.ResourceList, target kube.ResourceList, originalRelease *release.Release) {
    // ……
    results, err := u.cfg.KubeClient.Update(current, target, u.Force)
    if err != nil {
        u.cfg.recordRelease(originalRelease)
        u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err)
        return
    }
    //……
}

到这里就能猜到很快就应该接近 Helm 和 K8s 相关库的交界点了,“KubeClient”这个名字看起来就不像是 Helm 自身逻辑里的代码。我们继续来看 Update 就行:

  1. 继续跟 Update() 接口

这里比较直观,我们要找到的实现肯定是 client.go 里面那个 Update。

  1. Update() 接口的实现

这个方法看起来也挺复杂,里面藏了一个关键的 updateResource() 函数:

func (c *Client) Update(original, target ResourceList, force bool) (*Result, error) {
    // ……
    err := target.Visit(func(info *resource.Info, err error) error {
        //……
        if err := updateResource(c, info, originalInfo.Object, force); err != nil {
            c.Log("error updating the resource %q:\n\t %v", info.Name, err)
            updateErrors = append(updateErrors, err.Error())
        }
        // ……
    })
    // ……
}
  1. 继续看 updateResource() 函数

    func updateResource(c *Client, target *resource.Info, currentObj runtime.Object, force bool) error {
    // ……
    if force {
    // ……
    } else {
    // ……
    // send patch to server
    c.Log("Patch %s %q in namespace %s", kind, target.Name, target.Namespace)
    obj, err = helper.Patch(target.Namespace, target.Name, patchType, patch, nil)
    if err != nil {
    return errors.Wrapf(err, "cannot patch %q with kind %s", target.Name, kind)
    }
    }
    // ……
    }

到这里就比较明确了,是 helper.Patch(target.Namespace, target.Name, patchType, patch, nil) 这个方法调用返回了一个包含敏感数据的 err。如果继续看一眼 Patch 方法的定义,就能找到:

这是 k8s.io/cli-runtime 里的代码,已经离开了 Helm 的“管辖范围”。

很明显,敏感数据来自于这几行代码:

obj, err = helper.Patch(target.Namespace, target.Name, patchType, patch, nil)
if err != nil {
    return errors.Wrapf(err, "cannot patch %q with kind %s", target.Name, kind)
}

我们希望尽脱敏这些数据,自然是在当前函数/方法内完成这个过程,不能把锅留给调用方(上层函数)。所以下一步就是在 if err != nil { 之后,return 之前加入一个日志过滤函数,在这个函数内实现日志脱敏。

  • 增加一个函数来完成脱敏逻辑:

    // desensitizeLog replaces the data in a Secret with {"key": ""}. // e.g. "data": {"username": "admin", "password": "password"} becomes "data": {"username": "", "password": "***"}
    func desensitizeLog(errLog string) string {
    // Find the start and end index of the JSON string
    start := strings.Index(errLog, "{\\"apiVersion")
    end := strings.LastIndex(errLog, ",\\"kind\\":\\"Secret\\"")

    // Extract the JSON string and unescape it
    jsonStr := strings.ReplaceAll(errLog[start:end], "\\\"", "\"")+ "}"
    
    // Unmarshal the JSON string into a Secret struct
    var secret Secret
    json.Unmarshal([]byte(jsonStr), &secret)
    
    // Desensitize the data in the Secret struct
    for key := range secret.Data {
        secret.Data[key] = "***"
    }
    
    // Marshal the Secret struct back into a JSON string
    desensitizedJson, _ := json.Marshal(secret)
    
    // Escape the JSON string and replace it in the original log
    fixedDesensitizedJson := strings.ReplaceAll(string(desensitizedJson), "\"", "\\\"")
    desensitizedLog := errLog[:start] + fixedDesensitizedJson[:len(fixedDesensitizedJson)-1] + errLog[end:]
    
    return desensitizedLog

    }

  • 把函数调用加到老代码里:

    c.Log("Patch %s %q in namespace %s", kind, target.Name, target.Namespace)
    obj, err = helper.Patch(target.Namespace, target.Name, patchType, patch, nil)
    if err != nil {
    sanitizeLog:=err.Error()
    if kind == "Secret" {
    sanitizeLog = desensitizeLog(err.Error())
    }
    return errors.Wrapf(errors.New(sanitizeLog), "cannot patch %q with kind %s", target.Name, kind)
    }

当 kind 为 Secret 的时候,就走一遍脱敏过程。

  • 再加一段 UT 吧:

    func TestDesensitizeLog(t *testing.T) {
    // Define a test error log
    errLog := cannot patch "test-release-secret" with kind Secret: "" is invalid: patch: Invalid value: "{\"apiVersion\":\"v1\",\"data\":{\"secretKey\":\"hello\", \"anotherKey\":\"world\"},\"kind\":\"Secret\",

    // Call the function
    result := desensitizeLog(errLog)
    
    // Check if the "data" field has been correctly sanitized
    if !strings.Contains(result, "\\\"secretKey\\\":\\\"***\\\"") ||
        !strings.Contains(result, "\\\"anotherKey\\\":\\\"***\\\"") {
        t.Errorf("The function did not correctly sanitize the error log")
    }

    }

UT 需要能通过,这就不用赘述了。UT 过了之后,手动测试下:

酷,看起来挺和谐了。

最后一步,提个 PR:

关于如何参与开源项目,如何在 GitHub 上提 PR,我这有一篇非常详细的文章:

ok,不啰嗦了。可能你已经发现,结尾有点仓促,没错,我写到一半的时候,到饭点了,赶紧收个尾,填肚子去咯~

手机扫一扫

移动阅读更方便

阿里云服务器
腾讯云服务器
七牛云服务器

你可能感兴趣的文章