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.

Sign up for GitHub

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

Jump to bottom

crypto/elliptic: bake 88 kB p256Precomputed table into binary on iOS #44992

Closed
bradfitz opened this issue Mar 14, 2021 · 9 comments
Closed

crypto/elliptic: bake 88 kB p256Precomputed table into binary on iOS #44992

bradfitz opened this issue Mar 14, 2021 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Go1.17

Comments

@bradfitz
Copy link
Contributor

The crypto/elliptic package, on amd64 and arm64, has an 88kB table that's lazily computed via crypto/elliptic/p256_asm.go:

func (p *p256Point) p256BaseMult(scalar []uint64) {
        precomputeOnce.Do(initTable)
...

func initTable() {
        p256Precomputed = new([43][32 * 8]uint64)

That 43 * 32 * 8 * 8 bytes == 88 KB, which resides in the heap.

On iOS, in certain contexts (App Extensions: https://developer.apple.com/app-extensions/), the memory limits can be quite low, like 15 MB total non-pageable memory.

88 KB is enough to show up in profiles.

If the 88 KB were baked into the binary, however, it wouldn't count against the limit.

Can we just always bake it into the binary on iOS at least?
@gopherbot gopherbot added this to the Proposal milestone Mar 14, 2021
@ianlancetaylor ianlancetaylor added the Proposal-Crypto Proposal related to crypto packages or other security issues label Mar 16, 2021
@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

/cc @FiloSottile

@FiloSottile
Copy link
Contributor

From my side, there's no problem with compiling it in. In general, this is a binary size vs memory use concern. It would get linked into anything that uses crypto/tls or net/http.

@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

If most uses of TLS are going to compute this table anyway, then precomputing is a net win.

Do most TLS uses end up needing this table?

@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc mentioned this issue Apr 7, 2021
Open
@FiloSottile
Copy link
Contributor

Do most TLS uses end up needing this table?

Yes, a pretty good portion of TLS connections will use it.

Do we want to special-case iOS where this is a net win, or everywhere even if it will make binaries bigger? I would prefer not to have the complexity of supporting both modes, but it would not be particularly hard.

@rsc
Copy link
Contributor

rsc commented Apr 14, 2021

If most TLS connections need it anyway, then sure let's try doing the table on all platforms and see how it goes.

@rsc
Copy link
Contributor

rsc commented Apr 14, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 21, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: crypto/elliptic: bake 88 kB p256Precomputed table into binary on iOS crypto/elliptic: bake 88 kB p256Precomputed table into binary on iOS Apr 21, 2021
@rsc rsc modified the milestones: Proposal, Backlog Apr 21, 2021
@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. and removed Proposal Proposal-FinalCommentPeriod labels Apr 29, 2021
@FiloSottile FiloSottile modified the milestones: Backlog, Go1.17 Apr 29, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/315189 mentions this issue: crypto/elliptic: store p256 table in source

@golang golang locked and limited conversation to collaborators Apr 29, 2022
@rsc rsc moved this to Accepted in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
5 participants
@bradfitz @rsc @FiloSottile @ianlancetaylor @gopherbot