zgrab2
zgrab2 copied to clipboard
Upgrade to go 1.23
Following upon #461, updating the go version to latest, the 1.23 release gives us many new improvements in both the compiler land and tooling land to utilize:
- Profile Guided Optimizations (PGOs) straight with the compiler allows for 2-7% perf improvements. GA in 1.21.
- Built-in
min, max, so we don't have to constantly implement our own, see:utility.go:129,modules/mssql/connection.go:749,modules/oracle/types_test.go:455,lib/ssh/channel.go:134 - Structured logging in the standard library!
- Slices and maps helpers in the standard library.
- ...and many other improvements all around that we would get for free with all the new tooling to improve zgrab2 further.
How to Test
CI/CD involved should handle it. As Go team noted in all release notes since 1.20 (current version in this project) to now,
As always, the release maintains the Go 1 promise of compatibility. We expect almost all Go programs to continue to compile and run as before.
Compiling and running the program yields no issues.
Notes & Caveats
Only one "breaking" compile-time issue found in modules/amqp091/scanner.go:250, where ChannelMax was expecting an int but got an uint16. Casting here is safe as this is a type promotion with no requirements for this variable to be exactly 16 bits.
Issue Tracking
Add a link to the relevant GitHub issue(s) if the pull request resolves it.
Hey @thecsw, thanks for this!
Off-hand, though, I'm hesitant to want to bump this to Go 1.23 for backwards-compatibility. We have users using older OS's and don't want to preclude them from being able to compile from source. All the compiler-driven enhancements you mention can still be obtained today by using the latest Go compiler on the existing source code. I don't think the other built-ins standard library enhancements can justify decreasing backwards compatibility, IMO.
Sure thing and totally valid! With this bump, it would require, at least on major systems, to run at least Big Sur for Mac and windows 10 (some new ports for bsd/etc. introduced)—makes sense for compat
We could possibly leave this open/archived for whenever time comes to upgrade to go1.23, at least
After the discussion on https://github.com/zmap/zdns/pull/495, I think we can go ahead with this!
@phillip-stephens wonderful—I noticed there are some new test failures since the last good head—I'll review those and make changes to get this through!
Okay, rebased onto the upstream's master and should be good to go
The failures seem to be coming from the python integration tests,
One or more schema validations failed: schema failure@ftp/authtls.json, schema failure@http/https.json, schema failure@ipp/cups-tls.json, schema failure@mssql/2017-linux.json, schema failure@mysql/5.7.json, schema failure@mysql/8.0.json, schema failure@postgres/10.1-ssl.json, schema failure@postgres/9.3-ssl.json, schema failure@postgres/9.4-ssl.json, schema failure@postgres/9.5-ssl.json, schema failure@postgres/9.6-ssl.json
Not super familiar with them or how these are checked, may be transient or coming from upstream
Looks like all the failures are in the exact same place:
ERROR:root:data.ftp.result.tls.handshake_log.server_hello.compression_method: class mismatch for compression_method: expected (<class 'future.types.newint.newint'>,), {u'hex': u'0x00', u'name': u'NULL', u'value': 0} has class dict
This does look like a zcrypto change, where compression_method was changed from a uint to a struct here: https://github.com/zmap/zcrypto/commit/c1c1128be414acd7fa84e87733380908944e0829
Though that commit is 2 years old!