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

asn1parse inform pem is not RFC7468 compliant #7317

Open
tomato42 opened this issue Sep 25, 2018 · 11 comments

Comments

Projects
None yet
4 participants
@tomato42
Copy link
Contributor

commented Sep 25, 2018

running openssl-1.1.0h-3.fc27

openssl req -x509 -newkey rsa -pkeyopt rsa_keygen_bits:2048 -keyout /tmp/localhost.key -out /tmp/localhost.crt -subj /CN=localhost -nodes -batch
openssl x509 -in /tmp/localhost.crt -out /tmp/localhost.pem -text
openssl asn1parse -in /tmp/localhost.pem -inform pem

outputs:

Error: offset too large

This is despite RFC 7468 requirement:

   Data before the encapsulation boundaries are
   permitted, and parsers MUST NOT malfunction when processing such
   data.
@mspncp

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

This was fixed 6b5c1d9 but requires the option -strictpem, see ASN1PARSE(1).

@mspncp mspncp closed this Sep 25, 2018

@mspncp

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

@mattcaswell what was the reason why -strictpem was not made the default behaviour? Was it for compatibility reasons? Should we consider making it the default for the next major version?

@mspncp mspncp reopened this Sep 25, 2018

@tomato42

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2018

+1 on that question, while I understand the usability of decoding pem-header-less base64 encoded data, shouldn't the default be the standard PEM?

@mspncp

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2018

Give me a day or two and I can prepare a pull request which makes -strictpem the default (but keeping the option for compatibility reasons) and adds a new -nopem option to return to the old behaviour.

mspncp added a commit to mspncp/openssl that referenced this issue Sep 26, 2018

apps/asn1parse: improve RFC7462 compliance
Fixes openssl#7317

The asn1parse command now supports three different input formats:

     openssl asn1parse -inform PEM|DER|B64

       PEM: base64 encoded data enclosed by PEM markers (RFC7462)
       DER: der encoded binary data
       B64: raw base64 encoded data

The PEM input format is the default format. It is equivalent
to the former `-strictpem` option which is now marked obsolete and
kept for compatibility reasons only.

The B64 is equivalent to the former default input format of the
asn1parse command (without `-strictpem`)

@mspncp mspncp referenced a pull request that will close this issue Sep 26, 2018

Open

WIP: apps/asn1parse: improve RFC7468 compliance #7320

0 of 2 tasks complete
@mspncp

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

Please have a look at #7320.

@mspncp mspncp changed the title asn1parse inform pem is not RFC7462 compliant asn1parse inform pem is not RFC7468 compliant Sep 26, 2018

@mattcaswell

This comment has been minimized.

Copy link
Member

commented Sep 26, 2018

@mattcaswell what was the reason why -strictpem was not made the default behaviour? Was it for compatibility reasons?

I have absolutely no recollection of making that commit :-)
I have no problem with making this the default.

@mattcaswell

This comment has been minimized.

Copy link
Member

commented Sep 26, 2018

Although....is making it the default a breaking change in any way?

@mspncp

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

Well, I wouldn't backport it to 1.1.1., at least not with the new default. Only keep it on master which (almost surely) will be scheduled as a major release.

@mspncp mspncp added the 3.0.0 label Sep 26, 2018

@mattcaswell

This comment has been minimized.

Copy link
Member

commented Sep 26, 2018

Only keep it on master which (almost surely) will be scheduled as a major release.

Right - but we are likely to specify a policy around breaking changes. It won't be "open season".

@levitte

This comment has been minimized.

Copy link
Member

commented Sep 26, 2018

This suggests that we need a roadmap where we specify things that go into a few major releases going forward. (this could potentially mean that we'll do major releases more frequently)

@mspncp

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

we'll do major releases more frequently

Don't forget, we only have thirteen of them left ;-)

mspncp added a commit to mspncp/openssl that referenced this issue Jul 17, 2019

apps/asn1parse: improve RFC7462 compliance
Fixes openssl#7317

The asn1parse command now supports three different input formats:

     openssl asn1parse -inform PEM|DER|B64

       PEM: base64 encoded data enclosed by PEM markers (RFC7462)
       DER: der encoded binary data
       B64: raw base64 encoded data

The PEM input format is the default format. It is equivalent
to the former `-strictpem` option which is now marked obsolete and
kept for compatibility reasons only.

The B64 is equivalent to the former default input format of the
asn1parse command (without `-strictpem`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.