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

x/crypto: cryptobyte: encode int64 with context-specific tags #24973

Closed
mirtchovski opened this issue Apr 20, 2018 · 5 comments
Closed

x/crypto: cryptobyte: encode int64 with context-specific tags #24973

mirtchovski opened this issue Apr 20, 2018 · 5 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone
Unreleased

Comments

@mirtchovski
Copy link
Contributor

We frequently have to deal with asn.1 formatted with context-specific
tags even for things like integers, for example:

http://lapo.it/asn1js/#A0108004010203048102050682040708090A

To do that efficiently with cryptobyte I've added two additional
functions to marshal such values to and from int64:

func (b *Builder) AddInt64(tag asn1.Tag, v int64)
func (s *String) ReadASN1Int64(out *int64, tag asn1.Tag) bool

I assume versions for uint64 will also be required.

Does it make sense to add those to the cryptobyte API or is there a workaround?
@gopherbot gopherbot added this to the Unreleased milestone Apr 20, 2018
@agl
Copy link
Contributor

agl commented Apr 20, 2018

These are implicitly tagged values, right? (Which is why the existing functions aren't so great?)

If so, then adding some more helper functions seems perfectly reasonable. (Most of the stuff I come across is explicitly tagged, thus I haven't hit this much.)

But I'd name the Builder method something like AddASN1Int64WithTag (all the other ASN.1 methods are AddASN1… and we already have AddASN1Int64).

Likewise with String: ReadASN1Int64WithTag (or ReadASN1IntegerWithTag if you want to mirror the existing function that handles all the different types).

@FiloSottile FiloSottile added Proposal Proposal-Crypto Proposal related to crypto packages or other security issues labels Apr 20, 2018
@mirtchovski
Copy link
Contributor Author

Yes, they are implicitly tagged. I'll add the changes. Do you want me to do the same for uint64?

@agl
Copy link
Contributor

agl commented Apr 20, 2018

If you expect to need uint64 too, then sure.

@mirtchovski
Copy link
Contributor Author

I will leave uints until the need arises to keep things simple.

Thanks for the quick response. I've created https://go-review.googlesource.com/c/crypto/+/108456

@gopherbot
Copy link

Change https://golang.org/cl/108456 mentions this issue: x/crypto: cryptobyte: manage integers with implicit tags

@golang golang locked and limited conversation to collaborators Apr 20, 2019
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@mirtchovski @agl
x/crypto: cryptobyte: manage integers with implicit tags
9a10200 
This change adds two functions to cryptobyte to encode and decode
int64 with tags supplied by the user.

This change also modifies a documentation comment which was outdated.

Fixes golang/go#24973

Change-Id: I2e3ca475891ba62df902f33085719f94e87a27cc
GitHub-Last-Rev: cd0300d9bfbd4e38f22503a046ea18f9ddd37676
GitHub-Pull-Request: golang/crypto#42
Reviewed-on: https://go-review.googlesource.com/108456
Reviewed-by: Adam Langley <agl@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@mirtchovski @agl
x/crypto: cryptobyte: manage integers with implicit tags
3d5461b 
This change adds two functions to cryptobyte to encode and decode
int64 with tags supplied by the user.

This change also modifies a documentation comment which was outdated.

Fixes golang/go#24973

Change-Id: I2e3ca475891ba62df902f33085719f94e87a27cc
GitHub-Last-Rev: cd0300d9bfbd4e38f22503a046ea18f9ddd37676
GitHub-Pull-Request: golang/crypto#42
Reviewed-on: https://go-review.googlesource.com/108456
Reviewed-by: Adam Langley <agl@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@mirtchovski @c-expert-zigbee
x/crypto: cryptobyte: manage integers with implicit tags
ac7a2a9 
This change adds two functions to cryptobyte to encode and decode
int64 with tags supplied by the user.

This change also modifies a documentation comment which was outdated.

Fixes golang/go#24973

Change-Id: I2e3ca475891ba62df902f33085719f94e87a27cc
GitHub-Last-Rev: cd0300d9bfbd4e38f22503a046ea18f9ddd37676
GitHub-Pull-Request: golang/crypto#42
Reviewed-on: https://go-review.googlesource.com/108456
Reviewed-by: Adam Langley <agl@golang.org>
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@mirtchovski @agl
x/crypto: cryptobyte: manage integers with implicit tags
973ae5e 
This change adds two functions to cryptobyte to encode and decode
int64 with tags supplied by the user.

This change also modifies a documentation comment which was outdated.

Fixes golang/go#24973

Change-Id: I2e3ca475891ba62df902f33085719f94e87a27cc
GitHub-Last-Rev: cd0300d9bfbd4e38f22503a046ea18f9ddd37676
GitHub-Pull-Request: golang/crypto#42
Reviewed-on: https://go-review.googlesource.com/108456
Reviewed-by: Adam Langley <agl@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@mirtchovski @agl
x/crypto: cryptobyte: manage integers with implicit tags
e5c9cd3 
This change adds two functions to cryptobyte to encode and decode
int64 with tags supplied by the user.

This change also modifies a documentation comment which was outdated.

Fixes golang/go#24973

Change-Id: I2e3ca475891ba62df902f33085719f94e87a27cc
GitHub-Last-Rev: cd0300d
GitHub-Pull-Request: golang#42
Reviewed-on: https://go-review.googlesource.com/108456
Reviewed-by: Adam Langley <agl@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@agl @FiloSottile @mirtchovski @gopherbot