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

Bump golang 1.12.10 (CVE-2019-9512, CVE-2019-9514, CVE-2019-16276) #1485

Merged
merged 3 commits into from
Oct 17, 2019

Conversation

thaJeztah
Copy link
Contributor

@thaJeztah thaJeztah commented Aug 14, 2019

Bump golang 1.12.10 (CVE-2019-16276)

go1.12.10 (released 2019/09/25) includes security fixes to the net/http and net/textproto
packages. See the Go 1.12.10 milestone on our issue tracker for details.

https://github.com/golang/go/issues?q=milestone%3AGo1.12.10

Bump golang 1.12.9

go1.12.9 (released 2019/08/15) includes fixes to the linker, and the os and math/big packages.
See the Go 1.12.9 milestone on our issue tracker for details.

https://github.com/golang/go/issues?q=milestone%3AGo1.12.9

Bump golang 1.12.8 (CVE-2019-9512, CVE-2019-9514)

go1.12.8 (released 2019/08/13) includes security fixes to the net/http and net/url packages.
See the Go 1.12.8 milestone on our issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.12.8

  • net/http: Denial of Service vulnerabilities in the HTTP/2 implementation
    net/http and golang.org/x/net/http2 servers that accept direct connections from untrusted
    clients could be remotely made to allocate an unlimited amount of memory, until the program
    crashes. Servers will now close connections if the send queue accumulates too many control
    messages.
    The issues are CVE-2019-9512 and CVE-2019-9514, and Go issue golang.org/issue/33606.
    Thanks to Jonathan Looney from Netflix for discovering and reporting these issues.
    This is also fixed in version v0.0.0-20190813141303-74dc4d7220e7 of golang.org/x/net/http2.
    net/url: parsing validation issue
  • url.Parse would accept URLs with malformed hosts, such that the Host field could have arbitrary
    suffixes that would appear in neither Hostname() nor Port(), allowing authorization bypasses
    in certain applications. Note that URLs with invalid, not numeric ports will now return an error
    from url.Parse.
    The issue is CVE-2019-14809 and Go issue golang.org/issue/29098.
    Thanks to Julian Hector and Nikolai Krein from Cure53, and Adi Cohen (adico.me) for discovering
    and reporting this issue.

Update and pin golang-migrate to v4.6.2

v4.6.2

  • Removed unnecessary debug output
  • Improved error messages when no migrations are found

v4.6.1

  • Fix issue parsing MySQL DSNs with custom query parameters

v4.6.0

  • Updated MongoDB driver to v1.1.0
  • Missing migrate CLI commands will now return a non-zero exit
  • Go 1.12.8 fixed a security issue where invalid URIs were being parsed.
    The fix broke migrate when used with MySQL.
  • Update NewDockerContainer in unused/deprecated migrate/testing package .

Use golang-migrate/migrate, because mattes/migrate was archived

@thaJeztah
Copy link
Contributor Author

ping @justincormack @HuKeping

@justincormack
Copy link
Contributor

CI issue looks like it could be transient - can you try repushing?

@thaJeztah
Copy link
Contributor Author

Looks like CodeCov had issues; https://circleci.com/gh/theupdateframework/notary/5567?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

==> Uploading
    .url https://codecov.io
    .query pr=1485&service=circleci&package=py2.0.15&yaml=codecov.yml&job=5567.0&build=5567.0&branch=pull%2F1485&commit=6a1bfb66d63ba75871b7232ebc4f7949faa9715b&slug=theupdateframework%2Fnotary
    Pinging Codecov...
Error: HTTPSConnectionPool(host='codecov.io', port=443): Max retries exceeded with url: /upload/v4?pr=1485&service=circleci&package=py2.0.15&yaml=codecov.yml&job=5567.0&build=5567.0&branch=pull%2F1485&commit=6a1bfb66d63ba75871b7232ebc4f7949faa9715b&slug=theupdateframework%2Fnotary (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7efc90263c90>: Failed to establish a new connection: [Errno 110] Connection timed out',))

Seeing networking issues as well; wondering: when you updated to Go 1.12.7 was the image already switched to debian buster? If not, this could be related. I can push a commit to pin it to -stretch for now

@thaJeztah
Copy link
Contributor Author

Yes, I'll try a repush first

@thaJeztah
Copy link
Contributor Author

Oh! failure actually looks because of the Golang change;

error: parse mysql://signer@tcp(mysql:3306)/notarysigner: invalid port ":3306)" after host

There's a trailing ) that it's parsing

@justincormack
Copy link
Contributor

Ah, yes the parsing changes look like they are going to break a few things... there are more coming in Go 1.14

@thaJeztah
Copy link
Contributor Author

Looks like they had this regression before in Golang; golang/go#12023

@justincormack
Copy link
Contributor

Well the suggestion in that issue is that DSNs are not necessarily URIs and you shouldn't parse them...

@thaJeztah
Copy link
Contributor Author

Need to look where it hits net/url; looks like it's not coming from here; as that would produce a different error (and handles the DSN's with custom code); https://github.com/theupdateframework/notary/blob/7e330fa39b5ddbd9dc081f3effd410ebb6bc24c4/utils/configuration.go#L114-L120

@thaJeztah
Copy link
Contributor Author

Guess it could be in an external binary;

go get -tags 'mysql postgres file' github.com/mattes/migrate/cli && mv /go/bin/cli /go/bin/migrate

@thaJeztah
Copy link
Contributor Author

Ah; that one has been archived (https://github.com/mattes/migrate/) and has moved to https://github.com/golang-migrate/migrate

@justincormack
Copy link
Contributor

issue now open at golang-migrate/migrate#264

@thaJeztah
Copy link
Contributor Author

New version still has the problem;

migrate -source "mysql://signer@tcp(mysql:3306)/notarysigner?parseTime=True" -database "mysql://signer@tcp(mysql:3306)/notarysigner?parseTime=True" up
error: source: parse mysql://signer@tcp(mysql:3306)/notarysigner?parseTime=True: invalid port ":3306)" after host

@justincormack
Copy link
Contributor

Should be fixed by golang-migrate/migrate#265

@thaJeztah
Copy link
Contributor Author

rebased

@thaJeztah
Copy link
Contributor Author

thaJeztah commented Aug 17, 2019

golang-migrate/migrate#265 was merged, but not yet tagged, pending golang-migrate/migrate#254 (trying to address that with golang-migrate/migrate#270)

I disabled go modules in the dockerfile to force it using "master", which may help getting CI green here. that didn't work; trying a workaround now

@thaJeztah thaJeztah force-pushed the bump_golang_1.12.8 branch 4 times, most recently from ec253c6 to 5181f20 Compare August 17, 2019 15:38
@thaJeztah
Copy link
Contributor Author

Well, I guess go mod is an improvement .. if you like your images to be bigger

REPOSITORY          TAG                     IMAGE ID            CREATED             SIZE
notary-server       latest                  01541277765c        17 seconds ago      953MB
notary-server       previous                adeee5f21478        9 minutes ago       590MB

@ernado
Copy link

ernado commented Aug 20, 2019

@thaJeztah regarding big images: you can use multi-stage builds to reduce size as described in recent article if it is important 🙂 (oh, you probably already know about it :D)

@thaJeztah
Copy link
Contributor Author

@thaJeztah regarding big images: you can use multi-stage builds to reduce size as described in recent article if it is important 🙂 (oh, you probably already know about it :D)

@ernado thanks! Yes, I'm aware of ways to reduce the final image size; the "minimal" Dockerfiles already use multi-stage to reduce the final image size, and for the other Dockerfile, it's not super important (as it's mainly for testing/building locally).

It's a pity though that go mod pulls in a lot more data than before; I haven't looked in-depth, but I suspect it's fetching dependencies for all packages (not just for the cli), and possibly also dependencies that are only used for testing (and not needed at all to build the binary).

After #1486 is merged, we can make more improvements (such as using RUN --mount=type=cache for the Go mod cache), which should make the situation better again.

Strictly we don't need the fixed version of migrate (go get "master" should work again now), but pinning to a fixed version makes the builds / tests more reproducible; I could work around the size regression by manually Git-cloning / pinning to a fixed version, but thought it wasn't important for now, and better left for future improvements

@thaJeztah
Copy link
Contributor Author

@HuKeping @endophage ptal 🤗

Copy link
Contributor

@HuKeping HuKeping left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @thaJeztah for tracking this !

rebase needed :)

and would you mind if squashing the last two commits, it seems the last but one was broken.

@thaJeztah thaJeztah changed the title Bump golang 1.12.8 (CVE-2019-9512, CVE-2019-9514) Bump golang 1.12.10 (CVE-2019-9512, CVE-2019-9514, CVE-2019-16276) Oct 16, 2019
@thaJeztah
Copy link
Contributor Author

thaJeztah commented Oct 16, 2019

CI is currently failing because of #1506 (also see #1505)

The reason this was now failing is that gosec recently added a new check ("G108": securego/gosec#383), which produced a false positive on the pprof endpoint being enabled by default.

However, there was a bug in the makefile, causing that output to not be printed, so make lint failed, but didn't print why.

I opened a PR to suppress the gosec issue; #1506
and a PR to fix the make lint output: #1505.

That last one currentlyfails, which allows verifying it does what it should do. After #1506 is merged, we can rebase it, and it should go green (to verify it's also doing the right thing if there's no failures 😂 )

@thaJeztah
Copy link
Contributor Author

This was updated to use Go 1.12.10 (which fixes another CVE), and golang-migrate v4.6.2

and would you mind if squashing the last two commits, it seems the last but one was broken.

They were broken when testing with Go > 1.12.8, but worked on Go 1.12.7. I re-ordered the commits so that the Go 1.12 bump is last, which should keep the repository git bisectable

@HuKeping
Copy link
Contributor

HuKeping commented Oct 17, 2019

Thanks @thaJeztah , let's get to #1505 and #1506 first.

They were broken when testing with Go > 1.12.8, but worked on Go 1.12.7. I re-ordered the commits so that the Go 1.12 bump is last, which should keep the repository git bisect able

yeahhhhhh, git bisect always means a lot for me, thanks!


RUN apk add --update git gcc libc-dev

# Pin to the specific v3.0.0 version
RUN go get -tags 'mysql postgres file' github.com/mattes/migrate/cli && mv /go/bin/cli /go/bin/migrate
ARG MIGRATE_VER=v4.6.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

missed updating this dockerfile; let me fix that one to be consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍

v4.6.2

- Removed unnecessary debug output
- Improved error messages when no migrations are found

v4.6.1

- Fix issue parsing MySQL DSNs with custom query parameters

v4.6.0

- Updated MongoDB driver to v1.1.0
- Missing migrate CLI commands will now return a non-zero exit
- Go 1.12.8 fixed a security issue where invalid URIs were being parsed.
  The fix broke migrate when used with MySQL.
- Update NewDockerContainer in unused/deprecated migrate/testing package .

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Bump golang 1.12.10 (CVE-2019-16276)
=====================================================

go1.12.10 (released 2019/09/25) includes security fixes to the net/http and net/textproto
packages. See the Go 1.12.10 milestone on our issue tracker for details.

https://github.com/golang/go/issues?q=milestone%3AGo1.12.10

Bump golang 1.12.9
=====================================================

go1.12.9 (released 2019/08/15) includes fixes to the linker, and the os and math/big packages.
See the Go 1.12.9 milestone on our issue tracker for details.

https://github.com/golang/go/issues?q=milestone%3AGo1.12.9

Bump golang 1.12.8 (CVE-2019-9512, CVE-2019-9514)
=====================================================

go1.12.8 (released 2019/08/13) includes security fixes to the net/http and net/url packages.
See the Go 1.12.8 milestone on our issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.12.8

- net/http: Denial of Service vulnerabilities in the HTTP/2 implementation
  net/http and golang.org/x/net/http2 servers that accept direct connections from untrusted
  clients could be remotely made to allocate an unlimited amount of memory, until the program
  crashes. Servers will now close connections if the send queue accumulates too many control
  messages.
  The issues are CVE-2019-9512 and CVE-2019-9514, and Go issue golang.org/issue/33606.
  Thanks to Jonathan Looney from Netflix for discovering and reporting these issues.
  This is also fixed in version v0.0.0-20190813141303-74dc4d7220e7 of golang.org/x/net/http2.
  net/url: parsing validation issue
- url.Parse would accept URLs with malformed hosts, such that the Host field could have arbitrary
  suffixes that would appear in neither Hostname() nor Port(), allowing authorization bypasses
  in certain applications. Note that URLs with invalid, not numeric ports will now return an error
  from url.Parse.
  The issue is CVE-2019-14809 and Go issue golang.org/issue/29098.
  Thanks to Julian Hector and Nikolai Krein from Cure53, and Adi Cohen (adico.me) for discovering
  and reporting this issue.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@justincormack
Copy link
Contributor

Going to ignore codecov change as this is not actually introducing new code.

@justincormack justincormack merged commit 66d40e4 into notaryproject:master Oct 17, 2019
@thaJeztah thaJeztah deleted the bump_golang_1.12.8 branch October 17, 2019 22:05
@thaJeztah
Copy link
Contributor Author

I'll start working on the Go1.12.11 update; which just was released (will probably do a round to a bunch of repositories)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants