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

32-bit platform failure: cannot use 100000000000000000 #1239

Closed
jas4711 opened this issue Nov 21, 2024 · 4 comments
Closed

32-bit platform failure: cannot use 100000000000000000 #1239

jas4711 opened this issue Nov 21, 2024 · 4 comments
Assignees

Comments

@jas4711
Copy link

jas4711 commented Nov 21, 2024

Hi

I am reviewing this package for Debian and noticed self-checks fail on 32-bit i386 platform:

https://salsa.debian.org/jas/golang-github-lestrrat-go-jwx-v2/-/jobs/6632052/viewer

# github.com/lestrrat-go/jwx/jwe_test [github.com/lestrrat-go/jwx/jwe.test]
src/github.com/lestrrat-go/jwx/jwe/jwe_test.go:940:37: cannot use 100000000000000000 (untyped int constant) as int value in argument to jwe.WithMaxPBES2Count (overflows)
FAIL	github.com/lestrrat-go/jwx/jwe [build failed]

Thoughts?

Thanks,
Simon

@lestrrat
Copy link
Collaborator

lestrrat commented Nov 21, 2024

@jas4711 Thanks!

I don't have a testing environment handy, but this should work yeah? This number just needs to be large enough to allow messages containing a big number on the p2c field, so anything large-ish should work, and it worked when I used math.MaxInt32 so it should work with the general case of math.MaxInt

===

git diff
diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go
index 3581974..e6ddf23 100644
--- a/jwe/jwe_test.go
+++ b/jwe/jwe_test.go
@@ -10,6 +10,7 @@ import (
        "crypto/rsa"
        "encoding/base64"
        "fmt"
+       "math"
        "os"
        "strings"
        "testing"
@@ -937,7 +938,7 @@ func TestGHSA_7f9x_gw85_8grf(t *testing.T) {
 
        // NOTE: HAS GLOBAL EFFECT
        // Should allow for timeout to occur
-       jwe.Settings(jwe.WithMaxPBES2Count(100000000000000000))
+       jwe.Settings(jwe.WithMaxPBES2Count(math.MaxInt))
 
        // put it back to normal after the test
        defer jwe.Settings(jwe.WithMaxPBES2Count(10000))

@jas4711
Copy link
Author

jas4711 commented Nov 22, 2024

Thank you! I can confirm that the patch fixes the i386 problem.

https://salsa.debian.org/jas/golang-github-lestrrat-go-jwx-v2/-/jobs/6634068

The package is now ready for upload into Debian, but we have to work out what to do with the minio/pkg reverse dependency that still is stuck on your v1 branch. We'd prefer to only ship your v2 branch, and not both v1 and v2. They have closed their issue tracker, so it is hard to report the problem. Are you familiar with that package? Any reason they couldn't move to v2? Any help on this is appreciated. https://github.com/minio/pkg

/Simon

@lestrrat
Copy link
Collaborator

I'll merge and make a release this weekend, so after that you should be able to just import from v2 mainline without a patch. Please wait just a bit more.


re: minio, I know its existence and I know they use jwx, but I'm not really familiar. I don't think there's anything stopping them from migrating to v2 either. But if I had to guess, I don't think there's anything compelling them to migrate either, and I think that's the reason why they haven't moved on.

@lestrrat
Copy link
Collaborator

@jas4711 https://github.com/lestrrat-go/jwx/releases/tag/v2.1.3

I'll close this for now. Please feel free to reopen if there are further issues.

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

No branches or pull requests

2 participants