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

allow whitespaces in data segments #53

Merged

Conversation

watson28
Copy link
Contributor

@watson28 watson28 commented Dec 12, 2023

I had an issue with a PEM certificate where the data segments were not separated by breaking lines (\n) but with empty spaces ( ). The library failed to parse the pem file, but OpenSSL didn't complain. Here is an example with the RSA keys used in the unit tests:

~ cat test-cert-rust-pem-rs.pem
-----BEGIN RSA PUBLIC KEY-----
MIIBOgIBAAJBAMIeCnn9G/7g2Z6J+qHOE2XCLLuPoh5NHTO2Fm+PbzBvafBo0oYo QVVy7frzxmOqx6iIZBxTyfAQqBPO3Br59BMCAwEAAQJAX+PjHPuxdqiwF6blTkS0 RFI1MrnzRbCmOkM6tgVO0cd6r5Z4bDGLusH9yjI9iI84gPRjK0AzymXFmBGuREHI sQIhAPKf4pp+Prvutgq2ayygleZChBr1DC4XnnufBNtaswyvAiEAzNGVKgNvzuhk ijoUXIDruJQEGFGvZTsi1D2RehXiT90CIQC4HOQUYKCydB7oWi1SHDokFW2yFyo6 /+lf3fgNjPI6OQIgUPmTFXciXxT1msh3gFLf3qt2Kv8wbr9Ad9SXjULVpGkCIB+g RzHX0lkJl9Stshd/7Gbt65/QYq+v+xvAeT0CoyIg
-----END RSA PUBLIC KEY-----

~ openssl rsa -in test-cert-rust-pem-rs.pem -text -noout
Private-Key: (512 bit, 2 primes)
modulus:
    00:c2:1e:0a:79:fd:1b:fe:e0:d9:9e:89:fa:a1:ce:
    13:65:c2:2c:bb:8f:a2:1e:4d:1d:33:b6:16:6f:8f:
    6f:30:6f:69:f0:68:d2:86:28:41:55:72:ed:fa:f3:
    c6:63:aa:c7:a8:88:64:1c:53:c9:f0:10:a8:13:ce:
    dc:1a:f9:f4:13
publicExponent: 65537 (0x10001) ...

This PR allows the acceptance of white space as separators in the PEM-encoded data.

Copy link
Owner

@jcreekmore jcreekmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the code, it looks fine. However, regex apparently bumped since the last time CI ran and requires Rust 1.65 and this crate is still back on 1.60.

If you want to change the Cargo.toml and the workflow to update it to 1.65 (in a follow on commit), I will merge once it passes CI. Otherwise, I will do that tomorrow and will have to get you to rebase your PR.

Scratch that, we don't have an actual dependency on regex. It is apparently pulled in through a test dependency. I want to go back and figure out how to make it so that we don't have to update MSRV.

@jcreekmore jcreekmore force-pushed the allow-whitespace-data-segment branch from 34d5644 to f18bf04 Compare December 12, 2023 02:02
Copy link
Owner

@jcreekmore jcreekmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great, thanks.

@jcreekmore jcreekmore merged commit e64b7f7 into jcreekmore:master Dec 12, 2023
2 checks passed
@jcreekmore
Copy link
Owner

Released 3.0.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants