fix: pem parsing should allow single dashes in comments

[lrstewart]

Description of changes:

Currently, s2n-tls can't parse certificates with comments containing '-' characters. Words and names containing '-' aren't uncommon-- "s2n-tls" has a '-'.

As an easy fix, this PR updates the parser to look for "--" instead of "-". "--" should be less common, and this fixes the problem without requiring a major rewrite of the parser.

This results in two changes to certificate format requirements:

1. Files that contain comments with '-' were not valid before but now are valid
2. Files that use single dashes in pem delimiters (like a copy/paste error resulting in "-BEGIN CERTIFICATE-----") were valid before but are now skipped.

Alternatively, if we want to continue accepting "-BEGIN CERTIFICATE-", I could skip to "-BEGIN" instead of '-' or "--". That would skip any extra preceding dashes and read the BEGIN token. The downsides are:

1. I can see a valid name in a comment being something like "WE-BEGIN". Dashes in names are just standard.
2. Malformed tokens like "-----BEGAN CERTIFICORT----" were not valid before but would be ignored.
3. More than 64 dashes before the "BEGIN" was not valid before but would be valid.
4. More generally, it significantly changes our parsing approach, possibly creating other errors I've missed.

Testing:

I added some easy to update unit tests, rather than relying on test pem files. They're not exhaustive, but they cover the cases I changed and a couple other important cases.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[alexw91]

This change looks good. The only other test that I would like to see is a more "real-world" test of parsing some real PEM comments found in the wild.

I think it'd be worthwhile to check in that PEM file under /tests/pems/centos-default-ca-bundle-2019.crt and verify that s2n's PEM parser correctly skips all 133 of those comments.

Once that is done we can also close out: #1136

[lrstewart]

I think it'd be worthwhile to check in that PEM file under /tests/pems/centos-default-ca-bundle-2019.crt and verify that s2n's PEM parser correctly skips all 133 of those comments.

We actually already have a trust store checked in for testing: https://github.com/aws/s2n-tls/tree/main/tests/pems/trust-store It's from an Amazon Linux 2012 instance. Is there anything particularly interesting about centos, or did you just want us to test with any copied trust store?