Closed
Conversation
While Emilia's reworking in 3cdd1e9 certainly helped, the code for decoding, at least, was still a little complex for something that really shouldn't be. This change achieves some of its reduction in complexity by dropping support for '-' in base64 data. Previously, this acted like a comment marker (see below). We have deployed this change at Google and have not found any ill effects: this appears to be completely unused in reality. This is a “port” of BoringSSL's base64 code to OpenSSL. The stdint.h types have been changed to C89 types, the code has been reformatted and variable declarations have been changed to C89 style. Here's an example of a PEM blob that was previously accepted but would be rejected with this change: -----BEGIN RSA PRIVATE KEY----- MIICXQIBAAKBgQC+qeXl4ZUfQZFmcGAPwdt7Mza4NQ6mJHehc4V/hVYc6eepvL/5 uyyflzuhVy5ufctdi92FlXcIct5nNPdqK0PPdWH5Uzw0t/OjI5y/SJh8ur20krqw j/N1IOs63AcGLIVSkwx89iQbxj+2tV+YxFpGunUYyR/bJJWczuDMA/CujQIDAQAB AoGBAKA6IRRdzbbVoD5JI8E6NZtEP7DwDZ57uPk6Hq86u1JTEzcmguJ4dJitPBRr Mn7yQgwcNQ5EvCKifdqXvXBAaZuiiPFuCS/gfUw04jVHXWvG8ZvBQC3dutUYnFW7 hdun8QU/Z6a1BethvESi1J1vgY2+XC4cBIvbutTc9HhMhbQ1AkEA8YTKGsVEYoKE d7sSx4qjeN4bgzeVgIwRt01wJ1EJN62LhwO+pYSXvTt14aHxiascejJqUhtuWvzR nuwydqiDpwJBAMoYgUoWdgW4O/C5ZXjiSia54jzrt7upxSq88njTRo/MCQfuJVbc 3GUD+15V0zNhx9D7lcI+1uxhfcD7jWbJEqsCQBrE/SG6e7nvfX9H3O0BEN10wNfq cUeuPshybNvuv3bMZYqxf5AZAjiXPpmjuYHo1V8191Lid3jeTN2wkGdWhkECQQCI Rj3oV3z+Hl1M1bc27GBT/MQxkEE0qiXpy780+kJ6dHsifdNv3z4+X5EA656e5zB2 Gy/A697BRnwlxXpz9OJBAkAUe7Ap0yU8aO6g5g+gsH+18bF0MftWh81VLOo09rRp SOHxNGGJLE5As5XkCGUZVIass1r8Q4N22Wip0QzeSWDi - - . * .. . * * - * * @()Ooc()* o . - (Q@*0CG*O() ___ - |\_________/|/ _ \ - | | | | | / | | - | | | | | | | | - | | | | | | | | - | | | | | | | | - | | | | | | | | - | | | | | \_| | - | | | | |\___/ - |\_|__|__|_/| - \_________/ - -----END RSA PRIVATE KEY-----
Contributor
|
It’s worth keeping the current code just so the beer stein works. :) |
Contributor
Author
|
Dr Henson points that that asn1parse depends on the base64 BIO handling PEM blobs and thus this change either needs to continue supporting '-' in base64 data (hopefully not) or needs to teach the base64 BIO about doing so. |
Contributor
|
Related issue with asn1parse and handling base64 PEM blobs: #1381 (comment) |
Closed
Member
|
This has bitrot too much so it is not going to be merged in this form. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While Emilia's reworking in 3cdd1e9 certainly helped, the code for
decoding, at least, was still a little complex for something that really
shouldn't be.
This change achieves some of its reduction in complexity by dropping
support for '-' in base64 data. Previously, this acted like a comment
marker (see below). We have deployed this change at Google and have not
found any ill effects: this appears to be completely unused in reality.
This is a “port” of BoringSSL's base64 code to OpenSSL. The stdint.h
types have been changed to C89 types, the code has been reformatted and
variable declarations have been changed to C89 style.
Here's an example of a PEM blob that was previously accepted but would
be rejected with this change: