Re: [openssl-dev] [openssl.org #4698] PEM parsing incorrect; whitespace in PEM crashes parser
On 10/05/2016 09:15 AM, Kaduk, Ben via RT wrote: > I refactored this stuff a while ago to add a flags field that would > force the temporary read buffer to be allocated from the secure heap; I > should really dig it up and clean it up for master. That's https://github.com/openssl/openssl/pull/1700 , FWIW. -Ben -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4698 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4698] PEM parsing incorrect; whitespace in PEM crashes parser
One more reference: https://tools.ietf.org/html/rfc4648#section-3.3 describes the considerations for 'non-base64 characters'. Short form: MIME requires that they be ignored. 7468 says SHOULD. 4648 says 'reject, unless the referencing spec says otherwise' (which 7468 does.) I wrote previously that MIME's limit on line length is 72; according to 4648 3.1 it's actually 76. Sorry. The point is, it's NOT 64 (which is what PEM specifies.). (65 in OpenSSL must include the end-of-line.) Note that all 3 constants are (deliberately) a multiple of 4, meaning that the decoding of a byte can't span lines. However, this is not true in the wild; end-of-line can appear anywhere. (Again, wrapping by MUAs, web browsers and embedded devices are the most frequent offenders.) Here's the full text of 3.3: >Base encodings use a specific, reduced alphabet to encode binary >data. Non-alphabet characters could exist within base-encoded data, >caused by data corruption or by design. Non-alphabet characters may >be exploited as a "covert channel", where non-protocol data can be >sent for nefarious purposes. Non-alphabet characters might also be >sent in order to exploit implementation errors leading to, e.g., >buffer overflow attacks. > >Implementations MUST reject the encoded data if it contains >characters outside the base alphabet when interpreting base-encoded >data, unless the specification referring to this document explicitly >states otherwise. Such specifications may instead state, as MIME >does, that characters outside the base encoding alphabet should >simply be ignored when interpreting data ("be liberal in what you >accept"). Note that this means that any adjacent carriage return/ >line feed (CRLF) characters constitute "non-alphabet characters" and >are ignored. Furthermore, such specifications MAY ignore the pad >character, "=", treating it as non-alphabet data, if it is present >before the end of the encoded data. If more than the allowed number >of pad characters is found at the end of the string (e.g., a base 64 >string terminated with "==="), the excess pad characters MAY also be >ignored. > Timothe Litt ACM Distinguished Engineer -- This communication may not represent the ACM or my employer's views, if any, on the matters discussed. -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4698 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4698] PEM parsing incorrect; whitespace in PEM crashes parser
On 10/05/2016 07:56 AM, Richard Levitte via RT wrote: > To be noted, there's more in section 2: > >Most extant parsers ignore blanks at the ends of lines; blanks at the >beginnings of lines or in the middle of the base64-encoded data are >far less compatible. These observations are codified in Figure 1. >The most lax parser implementations are not line-oriented at all and >will accept any mixture of whitespace outside of the encapsulation >boundaries (see Figure 2). Such lax parsing may run the risk of >accepting text that was not intended to be accepted in the first >place (e.g., because the text was a snippet or sample). > > I haven't looked enough in our code recently to remember if we're doing > "standard" (figure 1) or "strict" (figure 3) parsing... what I hear is a > request for us to move to "lax" (figure 2) parsing. > If I remember correctly, it's somewhere in between. The core PEM-parsing code is vintage EAY, and contains some "interesting" behavior, like going to the end of the line/buffer that was read, backtracking past any characters with ASCII value less than or equal to that of , and writing \n\0. So, it seems like trailing whitespace would be ignored, but leading whitespace would trip up the "len == 65" check later on. I refactored this stuff a while ago to add a flags field that would force the temporary read buffer to be allocated from the secure heap; I should really dig it up and clean it up for master. -Ben -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4698 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4698] PEM parsing incorrect; whitespace in PEM crashes parser
On 05-Oct-16 08:56, Richard Levitte via RT wrote: > To be noted, there's more in section 2: > >Most extant parsers ignore blanks at the ends of lines; blanks at the >beginnings of lines or in the middle of the base64-encoded data are >far less compatible. These observations are codified in Figure 1. >The most lax parser implementations are not line-oriented at all and >will accept any mixture of whitespace outside of the encapsulation >boundaries (see Figure 2). Such lax parsing may run the risk of >accepting text that was not intended to be accepted in the first >place (e.g., because the text was a snippet or sample). > > I haven't looked enough in our code recently to remember if we're doing > "standard" (figure 1) or "strict" (figure 3) parsing... what I hear is a > request for us to move to "lax" (figure 2) parsing. Yes. Actually, the text is even more lax than the BNF; it says in paragraph 1 that parsers SHOULD ignore whitespace and other non- base64 characters That is, anything but A-Za-z0-9+/ and = at the end (as pad) should be ignored between the header and the footer. Many decoders do that silently, some warn if the junk isn't whitespace. Let's step back a bit from the letter of the RFCs and consider what brought this up: The real-word issues that drive this are cases like cut and paste of a CSR, certificate, or key from a webpage, terminal window or e-mail. All may re-wrap such that whitespace is introduced or lost. Further, especially with long keys, the text may not all be visible at once, so one ends up scrolling and/or copy/pasting in sections. Again introducing and/or losing white space. And exactly how textboxes on web pages represent EOL and interact with copy/paste varies. Lost newlines can produce long lines, and many base64 encoders (e.g. Perl's MIME::Base64) produce PEM that's longer than 64 characters (e.g. the 72 characters recommended for MIME.) CSRs/Certificates/Keys appear on webpages generated by embedded devices (think NAS, routers), as well as CAs and terminal windows. So while one would like to think that they're never touched by human hands, the reality is that they are. I'm not as concerned about "accepting text that was not intended to be accepted in the first place" because validation of the data will occur. CSRs and certificates are signed, and will fail validation if corrupt. Keys won't work if corrupt. All have to pass ASN.1 parsing, which also will catch many forms of corruption. OpenSSL should accept the CSR that I posted as a test case. Whether to also ignore non-base64 characters is debatable. I vote for warning (e.g. a distinct SUCCESS code that the caller can elect to report or ignore). What's fixed is that there must be a "-BEGIN" line, and there's little excuse for not having a "-END" line, though the newline after the "-END" may be optional. Embedded whitespace must be ignored - which includes that line length is unrestricted. This is something that both humans with a mouse and software can comprehend... The approach I use is to discard all whitespace, check for only base64 + optional pad & ensure that the length, including 0-2 pad (=) at the end is an even multiple of 4 characters long. Otherwise, (non-base64 or not a sane length) I warn but process the input. (A Perl implementation is in the OpenXPKI issue that I cited.) Naturally, I am NOT arguing that PEM can be produced in lax form; this is only about making the input parsing compatible with (RFC-compliant) cases common in the real world. I hope this provides context for your decisions... Timothe Litt ACM Distinguished Engineer -- This communication may not represent the ACM or my employer's views, if any, on the matters discussed. -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4698 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4698] PEM parsing incorrect; whitespace in PEM crashes parser
On 05-Oct-16 07:52, Salz, Rich via RT wrote: > Well, it is a SHOULD not a MUST. But point taken it could be (much) better :) > > It's an important SHOULD. Whitespace introduction happens in the wild. This is the quote from the OpenXPKI folks: > I just saw this today at a customer install that a user uploaded a > PCSK10 request with extra newlines, anything based on Crypt::PKCS10 is > happy with it but openssl crashes when it tries to sign. See https://github.com/openxpki/openxpki/issues/437 -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4698 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4698] PEM parsing incorrect; whitespace in PEM crashes parser
Well, it is a SHOULD not a MUST. But point taken it could be (much) better :) -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4698 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev