bump viper to remove CVE found in github.com/miekg/dns v1.0.14

[umarcor]

Close #1538.

As reported in #1538, this addresses a security issue. However, github.com/miekg/dns v1.0.14 is still found in the go.sum.

Ref: GHSA-44r7-7p62-q3fr

/cc @cboitel

[cboitel]

Will check but did u run go mod tidy ?

does go mod graph still reports the wrong version of the dns module ?

[umarcor]

Will check but did u run go mod tidy ?

I did. Actually, I double checked by removing go.sum, and then running go mod tidy using container golang, in order to ensure that it was not my local golang installation the one adding stuff.

does go mod graph still reports the wrong version of the dns module ?

See:

# go mod graph | grep miekg/[email protected]
github.com/hashicorp/[email protected] github.com/miekg/[email protected]

EDIT

# go mod graph | gomodtree github.com/miekg/[email protected]
github.com/miekg/[email protected]
|   github.com/hashicorp/[email protected]
|   |   github.com/hashicorp/[email protected]
|   |   |   github.com/hashicorp/consul/[email protected]
|   |   |   |   github.com/sagikazarmark/[email protected]
|   |   |   |   |   github.com/spf13/[email protected]
|   |   |   |   |   |   github.com/spf13/cobra
|   |   |   github.com/sagikazarmark/[email protected] ^^

[umarcor]

It seems that sagikazarmark/crypt#4 should be merged, then it should be updated in viper, and last we might bump it here.

/cc @sagikazarmark

EDIT

See also bketelsen/crypt#42.

/cc @bketelsen @mmorel-35

[sagikazarmark]

Let me take care of the viper side of things.

[bketelsen]

crypt was updated, tag v0.0.5 released.

[umarcor]

As explained in bketelsen/crypt#42, I think we need hashicorp to publish a new verison of github.com/hashicorp/consul/api.

@sagikazarmark @bketelsen, is there any technical reason to maintain two forks of crypt instead of having a single one? Or, alternatively, one being rebased on top of the other.

[bketelsen]

what's the 2nd fork? I'm only aware of mine. bketelsen/crypt is the only one mentioned in the go.sum file, too.

[bketelsen]

oh I understand now, @sagikazarmark doesn't have a "forked" version of crypt, that was a pull request against mine.

[umarcor]

@bketelsen according to https://github.com/spf13/viper/blob/master/go.mod#L11, spf13/viper is using https://github.com/sagikazarmark/crypt/tree/v0.2.0.

[bketelsen]

well if someone forked crypt and added it to viper, why am I here? :)
You're welcome to continue to use the fork of crypt if you prefer to. I wrote the original library at a previous job, and the company doesn't exist anymore, which is why it was forked to my personal account. I don't have a preference either way, I'm not using crypt anywhere else.

[bketelsen]

it's worse than that because viper and cobra are going to be out of sync. Cobra is using mine.
https://github.com/spf13/cobra/blob/master/go.sum#L47

[umarcor]

well if someone forked crypt and added it to viper, why am I here? :)

"forked from bketelsen/crypt" is shown at the top of https://github.com/sagikazarmark/crypt. Since I don't know the background, sagikazarmark/crypt might have been a subset of bketelsen/crypt, which is kept in sync with it.
The main difference seems to be that the CLI was removed from sagikazarmark/crypt. Maybe you two had talked about it. That's why I asked.

Moreover, the user who reported the issue here (#1538), did mention viper and he opened an issue in your fork (bketelsen/crypt#42), not in sagikazarmark/crypt.

Therefore, I think it makes sense for you to be in this dialogue.

it's worse than that because viper and cobra are going to be out of sync. Cobra is using mine.
https://github.com/spf13/cobra/blob/master/go.sum#L47

That's the indirect dependency through viper:

github.com/bketelsen/[email protected]
|   github.com/spf13/[email protected]
|   |   github.com/spf13/cobra

Hence, that'll be replaced with sagikazarmark/crypt when this PR is merged, unless viper reverts back to using bketelsen/crypt.

@sagikazarmark, is there any discussion/explanation about spf13/viper#1218?

[sagikazarmark]

I decided to fork crypt a while ago, so we have a little bit more control over some of the dependencies. The crypt dependency will eventually disappear from Viper, so I consider it an "internal" dependency at the moment (implementation detail).

I think crypt is only a transitive dependency of cobra, so once Cobra updates its Viper, everything should use my fork.

I also upgraded crypt in Viper. Will tag a new version shortly.

[sagikazarmark]

Tagged Viper 1.10.0

[umarcor]

@sagikazarmark see https://github.com/spf13/viper/blob/a4bfcd9ea04475e70535476efda42c9757b86c18/go.sum#L295 https://github.com/sagikazarmark/crypt/blob/1e72b2159e0bc52b259cbdf78bac5f9e43a0af12/go.sum#L305, and and bketelsen/crypt#42 (comment).

In short, github.com/hashicorp/consul/[email protected] was tagged in June. The dependency was fixed in main a month ago: https://github.com/hashicorp/consul/blame/b50ef696c67117af8cbd32c854aa04c334273dab/api/go.mod#L13. I believe that a new api/v1* tag is required in https://github.com/hashicorp/consul/. That'd allow crypt to update the dependency, then viper to update the dependency, then cobra to update viper.

[sagikazarmark]

Hashicorp API v1.12.0 was released yesterday. Tagged Viper v1.10.1: https://github.com/spf13/viper/releases/tag/v1.10.1

[umarcor]

@sagikazarmark thanks! ❤️

@jpmcb I think this is ready to merge.

[umarcor]

@jpmcb since this is a security issue, can we have it merged and a bugfix release published?

[jpmcb]

Now that we have dependabot in place, I prefer to use that. If there's no objections, we should close this PR and move any other discussion to:

#1567

[umarcor]

Now that we have dependabot in place, I prefer to use that. If there's no objections, we should close this PR and move any other discussion to:

#1567

Done. I suggest that you edit the title/body of #1567 to add a ref to this PR. Alternatively, you might do it when merging.