Skip to content

Update golang crypto library - #503

Draft
ohnorobo wants to merge 4 commits into
mainfrom
update-crypto
Draft

ohnorobo wants to merge 4 commits into
mainfrom
update-crypto

Conversation

@ohnorobo

@ohnorobo ohnorobo commented Aug 21, 2025 •

Copy link
Copy Markdown
Collaborator

run go get -u golang.org/x/crypto in sdk and in /x

This is to fix the error

E  panic: tls: ConnectionState is not equal to tls.ConnectionState: struct field count mismatch: 17 vs 16

Someone was seeing this while trying to use mobileproxy in an android app. Due to a new field being added in crypto/tls Conn.ConnectionState

@fortuna fortuna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's good to upgrade the dependencies, but the explanation does not make sense to me. We build a static binary, why would there be any conflict?

Were you able to reproduce the bug? Does this fix it?

(Also, kind of crazy how many new libraries it's pulling)

@ohnorobo

Copy link
Copy Markdown
Collaborator Author

Yeah, I'm not yet convinced this fixes the issue (it's still in draft) and I'm suspicious of how dramatic the changes it makes are. We would want to test something like this very carefully.

The user found that compiling with go 1.24.6 fixed their issue, so it's definitely some kind of library version mismatch, presumably of x/crypto. Where the seam is exactly (psiphon and mobileproxy? the mobileproxy aar and the user's app?) I'm not sure.

By the error message, this unsafe reflection code in psiphon is also involved.

@fortuna

fortuna commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

It seems like they are hacking the TLS library. Perhaps we need to update the psiphon dependencies instead.

The reflection is concerning. I hope they are not doing any function calls with reflection (field lookup is ok), because that would disable dead code removal and bloat any code we build.

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.

2 participants