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

[Bug]: linting and testing show multiple errrors #2161

Closed
marrold opened this issue Feb 9, 2025 · 2 comments
Closed

[Bug]: linting and testing show multiple errrors #2161

marrold opened this issue Feb 9, 2025 · 2 comments
Assignees
Labels
bug needs triage Waiting for discussion / prioritization by team
Milestone

Comments

@marrold
Copy link

marrold commented Feb 9, 2025

Steps to Reproduce

  1. clone repo
  2. checkout v0.28.1
  3. run make lint && make test

Your Environment

  • OS - Debian 12
  • step-ca Version - v0.28.1

Expected Behavior

linting and testing should pass without errors to enable contributing to the code with confidence that regressions haven't been introduced

Actual Behavior

linting with make lint shows the following errors:

authority/provisioner/collection.go:213:39: G115: integer overflow conversion int -> uint32 (gosec)
        binary.BigEndian.PutUint32(bi, uint32(c.sorted.Len()))
                                             ^
authority/provisioner/controller.go:181:19: G115: integer overflow conversion uint64 -> int64 (gosec)
        if after := int64(cert.ValidAfter); after < 0 || unixNow < int64(cert.ValidAfter) {
                         ^
authority/provisioner/controller.go:184:20: G115: integer overflow conversion uint64 -> int64 (gosec)
        if before := int64(cert.ValidBefore); cert.ValidBefore != uint64(ssh.CertTimeInfinity) && (unixNow >= before || before < 0) && !p.Claimer.AllowRenewalAfterExpiry() {
                          ^
authority/provisioner/jwk.go:280:63: G115: integer overflow conversion int64 -> uint64 (gosec)
                signOptions = append(signOptions, sshCertValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix()))
                                                                            ^
authority/provisioner/jwk.go:277:62: G115: integer overflow conversion int64 -> uint64 (gosec)
                signOptions = append(signOptions, sshCertValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix()))
                                                                           ^
authority/provisioner/nebula.go:239:64: G115: integer overflow conversion int64 -> uint64 (gosec)
                        signOptions = append(signOptions, sshCertValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix()))
                                                                                    ^
authority/provisioner/nebula.go:236:63: G115: integer overflow conversion int64 -> uint64 (gosec)
                        signOptions = append(signOptions, sshCertValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix()))
                                                                                   ^
authority/provisioner/sign_ssh_options.go:109:28: G115: integer overflow conversion int64 -> uint64 (gosec)
                cert.ValidBefore = uint64(o.ValidBefore.RelativeTime(t).Unix())
                                         ^
authority/provisioner/sign_ssh_options.go:106:27: G115: integer overflow conversion int64 -> uint64 (gosec)
                cert.ValidAfter = uint64(o.ValidAfter.RelativeTime(t).Unix())
                                        ^
authority/provisioner/sign_ssh_options.go:170:20: G115: integer overflow conversion int64 -> uint64 (gosec)
                backdate = uint64(o.Backdate / time.Second)
                                 ^
authority/provisioner/sign_ssh_options.go:171:27: G115: integer overflow conversion int64 -> uint64 (gosec)
                cert.ValidAfter = uint64(now().Truncate(time.Second).Unix())
                                        ^
authority/provisioner/sign_ssh_options.go:174:46: G115: integer overflow conversion int64 -> uint64 (gosec)
                cert.ValidBefore = cert.ValidAfter + uint64(d/time.Second)
                                                           ^
authority/provisioner/sign_ssh_options.go:209:20: G115: integer overflow conversion int64 -> uint64 (gosec)
                backdate = uint64(o.Backdate / time.Second)
                                 ^
authority/provisioner/sign_ssh_options.go:210:27: G115: integer overflow conversion int64 -> uint64 (gosec)
                cert.ValidAfter = uint64(now().Truncate(time.Second).Unix())
                                        ^
authority/provisioner/sign_ssh_options.go:213:35: G115: integer overflow conversion uint64 -> int64 (gosec)
        certValidAfter := time.Unix(int64(cert.ValidAfter), 0)
                                         ^
authority/provisioner/sign_ssh_options.go:224:28: G115: integer overflow conversion int64 -> uint64 (gosec)
                cert.ValidBefore = uint64(certValidBefore.Unix())
                                         ^
authority/provisioner/sign_ssh_options.go:226:37: G115: integer overflow conversion uint64 -> int64 (gosec)
                certValidBefore := time.Unix(int64(cert.ValidBefore), 0)
                                                  ^
authority/provisioner/sign_ssh_options.go:280:32: G115: integer overflow conversion int64 -> uint64 (gosec)
        case cert.ValidBefore < uint64(now().Unix()):
                                      ^
authority/provisioner/sign_ssh_options.go:302:22: G115: integer overflow conversion uint64 -> int64 (gosec)
        dur := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
                            ^
authority/provisioner/sign_ssh_options.go:335:32: G115: integer overflow conversion int64 -> uint64 (gosec)
        case cert.ValidBefore < uint64(now().Unix()):
                                      ^
authority/provisioner/sign_ssh_options.go:465:11: G115: integer overflow conversion int -> uint32 (gosec)
        if uint32(len(in)) < length {
                 ^
authority/provisioner/sshpop.go:121:20: G115: integer overflow conversion uint64 -> int64 (gosec)
                if after := int64(sshCert.ValidAfter); after < 0 || unixNow < int64(sshCert.ValidAfter) {
                                 ^
authority/provisioner/sshpop.go:124:21: G115: integer overflow conversion uint64 -> int64 (gosec)
                if before := int64(sshCert.ValidBefore); sshCert.ValidBefore != uint64(ssh.CertTimeInfinity) && (unixNow >= before || before < 0) {
                                  ^
authority/provisioner/x5c.go:338:63: G115: integer overflow conversion int64 -> uint64 (gosec)
                signOptions = append(signOptions, sshCertValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix()))
                                                                            ^
authority/provisioner/x5c.go:335:62: G115: integer overflow conversion int64 -> uint64 (gosec)
                signOptions = append(signOptions, sshCertValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix()))
                                                                           ^
scripts/badger-migration/main.go:309:18: G115: integer overflow conversion int -> uint16 (gosec)
                length = uint16(len(bk))
                               ^
scripts/badger-migration/main.go:311:11: G115: integer overflow conversion int -> uint16 (gosec)
        if uint16(len(bk)) < start {
                 ^
acme/challenge.go:230:16: G115: integer overflow conversion uint64 -> uint8 (gosec)
                        return uint8(v.Uint())
                                    ^
acme/challenge.go:1105:32: G115: integer overflow conversion int64 -> int32 (gosec)
        switch coseAlgorithmIdentifier(alg) {
                                      ^
acme/linker.go:89:77: G602: slice index out of range (gosec)
                return fmt.Sprintf("/%s/%s/%s/%s", provisionerName, typ, inputs[0], inputs[1])
                                                                                          ^
db/db.go:468:21: G115: integer overflow conversion uint64 -> int64 (gosec)
                if time.Unix(int64(data.Expiry), 0).After(time.Now()) {
                                  ^
cas/cloudcas/certificate.go:253:25: G115: integer overflow conversion int -> int32 (gosec)
                        maxPathLength = int32(cert.MaxPathLen)
                                             ^
cas/cloudcas/certificate.go:307:17: G115: integer overflow conversion int -> int32 (gosec)
                ret[i] = int32(v)
                              ^
test/integration/scep/internal/x509/oid.go:83:22: G115: integer overflow conversion int -> uint (gosec)
                o := byte(n >> uint(i*7))
                                   ^
test/integration/scep/internal/x509/parser.go:279:11: SA1019: elliptic.Unmarshal has been deprecated since Go 1.21: for ECDH, use the crypto/ecdh package. This function accepts an encoding equivalent to that of the NewPublicKey methods in crypto/ecdh. (staticcheck)
                x, y := elliptic.Unmarshal(namedCurve, der)
                        ^
test/integration/scep/internal/x509/parser.go:10:2: SA1019: "crypto/dsa" has been deprecated since Go 1.16 because it shouldn't be used: DSA is a legacy algorithm, and modern alternatives such as Ed25519 (implemented by package crypto/ed25519) should be used instead. Keys with 1024-bit moduli (L1024N160 parameters) are cryptographically weak, while bigger keys are not widely supported. Note that FIPS 186-5 no longer approves DSA for signature generation. (staticcheck)
        "crypto/dsa"
        ^
authority/linkedca.go:339:48: G115: integer overflow conversion int -> int32 (gosec)
                ReasonCode:     linkedca.RevocationReasonCode(rci.ReasonCode),
                                                             ^
authority/linkedca.go:353:45: G115: integer overflow conversion int -> int32 (gosec)
                ReasonCode:  linkedca.RevocationReasonCode(rci.ReasonCode),
                                                          ^
authority/linkedca.go:406:34: G115: integer overflow conversion int -> int32 (gosec)
                Type: linkedca.Provisioner_Type(p.GetType()),
                                               ^
authority/provisioners.go:1259:43: G115: integer overflow conversion int -> int32 (gosec)
                                                MinimumPublicKeyLength:        int32(p.MinimumPublicKeyLength),
                                                                                    ^
authority/provisioners.go:1262:43: G115: integer overflow conversion int -> int32 (gosec)
                                                EncryptionAlgorithmIdentifier: int32(p.EncryptionAlgorithmIdentifier),
                                                                                    ^
authority/ssh.go:359:27: G115: integer overflow conversion uint64 -> int64 (gosec)
        duration := time.Duration(oldCert.ValidBefore-oldCert.ValidAfter) * time.Second
                                 ^
authority/ssh.go:373:26: G115: integer overflow conversion int64 -> uint64 (gosec)
                ValidAfter:      uint64(va.Unix()),
                                       ^
authority/ssh.go:374:26: G115: integer overflow conversion int64 -> uint64 (gosec)
                ValidBefore:     uint64(vb.Unix()),
                                       ^
authority/ssh.go:439:27: G115: integer overflow conversion uint64 -> int64 (gosec)
        duration := time.Duration(oldCert.ValidBefore-oldCert.ValidAfter) * time.Second
                                 ^
authority/ssh.go:453:26: G115: integer overflow conversion int64 -> uint64 (gosec)
                ValidAfter:      uint64(va.Unix()),
                                       ^
authority/ssh.go:454:26: G115: integer overflow conversion int64 -> uint64 (gosec)
                ValidBefore:     uint64(vb.Unix()),
                                       ^
ca/tls.go:135:2: SA1019: tr.DialTLS has been deprecated since Go 1.14: Use DialTLSContext instead, which allows the transport to cancel dials as soon as they are no longer needed. If both are set, DialTLSContext takes priority. (staticcheck)
        tr.DialTLS = c.buildDialTLS(tlsCtx)
        ^
ca/tls.go:182:2: SA1019: tr.DialTLS has been deprecated since Go 1.14: Use DialTLSContext instead, which allows the transport to cancel dials as soon as they are no longer needed. If both are set, DialTLSContext takes priority. (staticcheck)
        tr.DialTLS = c.buildDialTLS(tlsCtx)
        ^
api/api.go:598:39: G115: integer overflow conversion uint64 -> int64 (gosec)
                        "valid-from":       time.Unix(int64(cert.ValidAfter), 0).Format(time.RFC3339),
                                                           ^
api/api.go:599:39: G115: integer overflow conversion uint64 -> int64 (gosec)
                        "valid-to":         time.Unix(int64(cert.ValidBefore), 0).Format(time.RFC3339),
                                                           ^
api/ssh.go:334:30: G115: integer overflow conversion uint64 -> int64 (gosec)
                        NotBefore: time.Unix(int64(cert.ValidAfter), 0),
                                                  ^
api/ssh.go:335:30: G115: integer overflow conversion uint64 -> int64 (gosec)
                        NotAfter:  time.Unix(int64(cert.ValidBefore), 0),
                                                  ^
api/sshRekey.go:83:30: G115: integer overflow conversion uint64 -> int64 (gosec)
        notBefore := time.Unix(int64(oldCert.ValidAfter), 0)
                                    ^
api/api.go:7:2: SA1019: "crypto/dsa" has been deprecated since Go 1.16 because it shouldn't be used: DSA is a legacy algorithm, and modern alternatives such as Ed25519 (implemented by package crypto/ed25519) should be used instead. Keys with 1024-bit moduli (L1024N160 parameters) are cryptographically weak, while bigger keys are not widely supported. Note that FIPS 186-5 no longer approves DSA for signature generation. (staticcheck)
        "crypto/dsa" // support legacy algorithms
        ^
make: *** [Makefile:141: lint] Error 1

testing with make test shows:

=== Skipped
=== SKIP: acme Test_parseAndVerifyWireAccessToken (0.00s)
    challenge_wire_test.go:2126: skip until we can retrieve public key from e2e test, so that we can actually verify the token

=== Failed
=== FAIL: ca/identity TestWriteDefaultIdentity/fail_certificate (0.00s)
    identity_test.go:263: WriteDefaultIdentity() error = <nil>, wantErr true

=== FAIL: ca/identity TestWriteDefaultIdentity/fail_write_identity (0.00s)
    identity_test.go:263: WriteDefaultIdentity() error = <nil>, wantErr true

=== FAIL: ca/identity TestWriteDefaultIdentity (0.00s)

=== FAIL: ca/identity TestIdentity_Renew/fail_write_identity (0.00s)
    identity_test.go:449: Identity.Renew() error = <nil>, wantErr true

=== FAIL: ca/identity TestIdentity_Renew (0.01s)

=== FAIL: templates TestOutput_Write/snippetErr (0.00s)
    templates_test.go:412: Output.Write() error = <nil>, wantErr true

=== FAIL: templates TestOutput_Write/fileErr (0.00s)
    templates_test.go:412: Output.Write() error = <nil>, wantErr true

=== FAIL: templates TestOutput_Write/directoryErr (0.00s)
    templates_test.go:412: Output.Write() error = <nil>, wantErr true

=== FAIL: templates TestOutput_Write (0.00s)

DONE 4745 tests, 1 skipped, 9 failures in 237.259s
make: *** [Makefile:112: testdefault] Error 1

Additional Context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@marrold marrold added bug needs triage Waiting for discussion / prioritization by team labels Feb 9, 2025
@hslatman hslatman self-assigned this Feb 11, 2025
@hslatman
Copy link
Member

hslatman commented Feb 11, 2025

Thanks for opening the issue, @marrold. It looks like there's a mismatch between versions used for golangci-lint in the Makefile vs. what's in CI. I'm working on fixing the issues in #2167.

W.r.t. make test: I'm not seeing that same behavior on my machine. It's possible that the directory can't be written to on your machine.

@hslatman hslatman added this to the v0.28.2 milestone Feb 20, 2025
@hslatman
Copy link
Member

The PR was merged, fixing the linter issues. Our CI was updated to make use of the latest golangci-lint at all times. There could still be a mismatch between versions used, but at least our CI will catch new issues before make lint.

I can't reproduce the failures you see with make test, so I consider this done for now. Feel free to reopen if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs triage Waiting for discussion / prioritization by team
Projects
None yet
Development

No branches or pull requests

2 participants