At 04:13 PM 2/15/2013, Chris Hegarty wrote: >Mike, > >The issue you are describing exists both before and after Marks proposed >patch. I am somewhat sympathetic to it, but it is outside the scope of what >this issue is trying to address. > >Since the code is open ;-) you can of course propose a separate patch, but I >don't think it should prevent this proceeding.
Agreed. >-Chris > >On 15 Feb 2013, at 20:26, Michael StJohns <mstjo...@comcast.net> wrote: > >> At 11:57 AM 2/15/2013, Chris Hegarty wrote: >>> Mike, >>> >>> I believe that Marks changes are a like for like replacement with what was >>> there before, only using a supported public API. Are you saying otherwise? >>> Or have you identified another potential issue? >> >> I ran into this problem when trying to do my own de-armoring code and >> dealing with stuff that had been OCR'd - (don't ask -it was painful all >> around, but it was what I had). I ended up having to wrap the de-armoring >> code in regex processing to ensure well-formed base64. >> >> I guess what I'm saying is that you should be able to detect a base64 >> problem and report it before trying to decode the byte stream. The fact >> that the old code didn't do this is occasionally problematic as I have at >> times spent hours trying to figure out what was wrong with the certificate >> encoding when it was the base64 that was confused. >> >> While I'm at it ( :-) ) it would be nice the code that detects the sentinels >> (e.g. the "-----BEGIN <etc>-----") would accept a number of the more common >> variations - "BEGIN X509 CERTIFICATE" "BEGIN CERTIFICATE REQUEST" (instead >> of "BEGIN CERTIFICATE and BEGIN NEW CERTIFICATE..."), and not care about >> white space between the "-----" and the beginning or end of the sentinel >> string. >> >> To answer the question you didn't quite ask - no, this isn't critical, but >> since the code is open... just a thought. >> >> Mike >> >> >> >>> -Chris. >>> >>> On 15/02/2013 16:52, Michael StJohns wrote: >>>> Is the "mime" variant of Base64 the correct one for this? I ask because >>>> that variant ignores extraneous characters rather than throwing an error >>>> on decode. Also, reading the code for the Base64 implementation, it >>>> silently "fixes" the case where there are missing padding "=" characters. >>>> Neither of these seem ideal for security related processing. >>>> >>>> It may be reasonable to add a PEM variant to the Base64 code that deals >>>> with the above. >>>> >>>> Mike >>>> >>>> >>>> >>>> At 08:24 AM 2/14/2013, Mark Sheppard wrote: >>>>> Hi, >>>>> as part of a refactoring of the jdk codebase to use the base64 >>>>> capabilities of java.util.Base64, the following modifications, >>>>> as per the webrev, >>>>> >>>>> http://cr.openjdk.java.net/~chegar/8006182/webrev.00/ >>>>> >>>>> have been made to complete task JDK-8006182. >>>>> >>>>> Could you oblige and review these changes, please? >>>>> >>>>> Description: >>>>> jdk8 has java.util.Base64 to define a standard API for base64 >>>>> encoding/decoding. It would be good to investigate whether this API could >>>>> be used in the security components, providers and regression tests. >>>>> >>>>> In the main this work involved replacing the sun.misc.BASE64Encoder and >>>>> sun.misc.BASE64Decoder with the >>>>> corresponding Mime Base64 Encoder/Decoder (as per rfc2045) from the >>>>> java.util.Base64 class. >>>>> This is a like for like replacement. >>>>> As such, sun.misc.BASE64Encoder maps to the encoder returned by >>>>> java.util.Base64.getMimeEncoder() >>>>> sun.misc.BASE64Decoder maps to the decoder returned by >>>>> java.util.Base64.getMimeDecoder() >>>>> >>>>> However a couple of items worth noting: >>>>> >>>>> In the jarsigner (Main.java) the standard Base64 encoder (rfc 4648), >>>>> java.util.Base64.getEncoder(), has been used to replace the >>>>> JarBASE64Encoder, which was a package private extension of BASE64Encoder, >>>>> which avoids writing newline to the encoded data. >>>>> >>>>> In the keytool (Main.java), methods such as dumpCert, printCert. >>>>> printCRL, and so on, write a Base64 encoding to an OutputStream, >>>>> typically std out. >>>>> This is achieved in the BASE64Encoder, by passing the OutputStream to >>>>> methods such as encodeBuffer(). >>>>> >>>>> A couple of options exist to do this under the new Base64 utilities, >>>>> which include: >>>>> >>>>> * using a Mime Encoder encodeToString() and output to the stream via >>>>> println() >>>>> >>>>> * use the wrap capabilities of the Base64.Encoder: >>>>> - define a package private class, which extends FilterOutputStream >>>>> (e.g. NoCloseWrapperOutputStream) and, overrides close() to do nothing >>>>> - inject the OutputStream, passed to the keytool method, into the >>>>> NoCloseWrapperOutputStreamwapper, >>>>> - wrap() the NoCloseWrapperOutputStreamwrapper in the Mime Encoder, >>>>> which will in turn return an encapsulating OutputStream; >>>>> - write the data buffer to be encoded to the encoder's OutputStream; >>>>> - close the encoder's OutputStream, which completes the base64 encoding; >>>>> - append a newline to the initial OutputStream. >>>>> >>>>> pragmatics and the simplest thing that works, went for the first option. >>>>> >>>>> regards >>>>> Mark >> >>
