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

encoding/asn1: roll back CL 16517? #13563

Closed
rsc opened this issue Dec 10, 2015 · 9 comments
Closed

encoding/asn1: roll back CL 16517? #13563

rsc opened this issue Dec 10, 2015 · 9 comments
Assignees
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Dec 10, 2015

This CL broke a test at Google in the Go port of keyczar. The test vectors check that signatures with non-minimal integer encodings are still accepted, and after CL 16517 they are not. We need to decide whether to roll back the CL or declare that that usage is not important.

@rsc rsc self-assigned this Dec 10, 2015
@rsc rsc added this to the Go1.6 milestone Dec 10, 2015
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 10, 2015

keyczar wants to accept bogus encodings?

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 10, 2015

/cc @agl

@rsc
Copy link
Contributor Author

@rsc rsc commented Dec 10, 2015

It's not bogus, it's just non-minimal. I have an email thread going with the author.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 10, 2015

Okay, please summarize what you find out.

Because I thought ASN.1 was DER and DER was the minimal-form-required version of BER, which I guess isn't the full story.

@danmux
Copy link

@danmux danmux commented Dec 11, 2015

Could this be related to a bug we have after upgrading from 1.4 to 1.5:

pk error x509: trailing data after ASN.1 of public-key

this ...

http://play.golang.org/p/U6pTR1oC1m

works in 1.4

@danmux
Copy link

@danmux danmux commented Dec 11, 2015

FYI. Whilst this is changed behaviour from 1.4 - 1.5 it has shown up the fact that one of our systems is generating 'bad' PK's. We can condition the []byte, and fix the culprit, so can move forward. In our case there would be no compelling argument to revert the stricter 1.5

If this is not the same issue then apologies.

@rsc
Copy link
Contributor Author

@rsc rsc commented Dec 11, 2015

@danmux, this issue is about something that changed since 1.5. You may be seeing something in 1.5 different from 1.4 but it's not this. I'd still be interested to hear more about your problem if you'd like to file a new issue. If you can describe the condition you use to detect the issue that might be enough, but of course code is nicer. :-) Thanks.

@danmux
Copy link

@danmux danmux commented Dec 11, 2015

ok thanks, ftr.. #13580 and happy for you to delete my polluting comments in here.

@rsc
Copy link
Contributor Author

@rsc rsc commented Dec 17, 2015

After discussion, the plan is to eliminate that keyczar test. The test cared more that the code didn't crash than that it accepted the signature in question. So we do not need to roll back CL 16517.

@rsc rsc closed this Dec 17, 2015
@golang golang locked and limited conversation to collaborators Dec 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.