regression in PemReader in 1.78 throws IOException: -----END CERTIFICATE----- not found

[ctcpip]

previously, PemReader was forgiving when parsing a string with leading whitespace in the -----END line. however, this behavior recently changed and now throws IOException

caused by this change

as an aside, ironically the changelog says Make PEM parsing more forgiving of whitespace 😅

while the leading whitespace is technically invalid, this change breaks previously working code. possible options are to revert part of the change, or at least call out the breaking behavior in release notes

[dghgit]

I'll update the release notes, experience has shown it's much better to try and stick to what's valid, agree with the sense of irony though, this is one of those things where you never win...

[hellerstanislav]

While I agree that it is better to stick to valid formats and deny any violations of the protocol, the world is not ideal and implementations using BC aren't either. For us, this is a big deal - since we manage an application which deals with secrets provided by thousands of customers, it is neither easy nor feasible to ask them to reupload their keys in case of a failure caused by leading whitespaces in PEM format.

Would it be possible to put this change behind some sort of flag so that we would be able to select lax parsing instead of the strict one?

[hytgbn]

+1 to go back to previous behavior. There are many existing real world cases that comes with leading spaces , not arguing that they are valid but this breaks our customers. Making parser stricter by default seems to be a dangerous step that can break lot of customers. If not, at least there needs to be a flag for more permissive parsing (previous behavior)

[hytgbn]

I think it's actually interesting that from this CR

blob2 is supported (trailing WSs after BEGIN line)

    private static final String blob2 =
        "-----BEGIN BLOB-----   \r\n"
        + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==\r\n"
        + "-----END BLOB-----    \r\n";

but blob4 is not supported.

    private static final String blob4 =
        "-----BEGIN BLOB-----\r\n"
        + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==\r\n"
        + "    -----END BLOB-----\r\n";

They don't seem to be consistent in interpreting RFC.

How do you think about adding trim() in https://github.com/pgpainless/bc-java/blob/main/core/src/main/java/org/bouncycastle/util/io/pem/PemReader.java#L78?

[dghgit]

I'd have to go back through the original emails, but I seem to remember one of the issues with the previous behavior was it resulted in some ambiguity about how things would be read.

We'd be happy to accept a pull request on this, providing it was protected by a system property.

[hellerstanislav]

@dghgit Hi David, can you please take a look at the pull request submitted by TaZbon? I am hoping this solution would be acceptable.

Cheers

[dghgit]

Pull request merged. Thanks.

[dudaerich]

@dghgit Hi David, do you know when the fix could be released? Is there any plan? Thank you.