[External] : Re: PEM API github repo

Sat Mar 9 16:09:28 UTC 2024

... try again from from my subscribed mail account...

Hi Tony,
>
> in my jdk fork, I created a branch named pem-feedback-karl.
>
> https://github.com/KarlScheibelhofer/jdk/tree/pem-feedback-karl
>
> It is based on the pem branch of your jdk fork.
> In this pem-feedback-karl branch, I did some cleanup without changing
> the API. Your tests pass as before.
>
> My original pem-keystore implementation for the SUN provider is in this
> branch
>
> https://github.com/KarlScheibelhofer/jdk/tree/pem-keystore
>
> It did not use the PEM API.
>
> In the branch
>
> https://github.com/KarlScheibelhofer/jdk/tree/pem-keystore-pem-api
>
> I modified the PEM keystore implementation to use the PEMDecoder and
> PEMEncoder.
> To make it pass all tests, I had to fix some issues with the PEM api:
>
> * fix missing line-breaks when encoding certificates (and CRLs)
> * use uniform line length for PEM encoding keys and certificates
>
> During this work, I took some notes regarding the PEM api:
>
> * Consider decoupling the raw PEM encoding and decoding from
> SecurityObject.
>   This would make the API usable for general purpose PEM encoding and
> decoding, not just for keys and certificates, as it is now.
>
> * For this PEM keystore implementation it is essential to parse the
> preceding explanatory text lines.
>   The PEM decoder should support this.
>   As it is now, the keystore implementation must parse the PEM objects
> itself, including reading PEM header and footer lines.
>   Having to handle half the work in the application diminishes the
> value of the PEMDecoder.
>   It only delegates the decoding of certificates and keys to the
> PEMDecoder.
>
> * PEMDecoder should be able to handle or pass through unknown PEM
> objects gracefully.
>   Otherwise the application has to check the PEM labels in advance
> itself, which does not make sense.
>
> * Though supporting InputStream/OutputStream for reading and writing
> makes sense,
>   supporting Reader/Writer feels even more essential for robust
> support for all character encodings.
>   Multi-Byte character encodings won't work with InputStream/OutputStream.
>
> * The standard line separator for PEM is "\r\n".
>   For PEM files stored in a typical linux file system, "\n" is
> typically used, however.
>   See the output of openssl, for example.
>   Because there are still several command line utilities that do not
> work well with "\r\n" line breaks.
>   Supporting a different line-separator than "\r\n" is a good idea in
> my opinion.
>   Base64.getMimeEncoder also supports selecting a custom line separator.
>
> * The PEMEncoder encodes the predefined SecurityObjects only.
>   There is no way to use it to PEM encoded any other type of object.
>   Consider opening a path to generic use.
>
> * If an application has a DER encoded certificate, it has to decode
> and parse the certificate before
>   it can encode it using PEMEncoder.
>   This is inconvenient.
>
> * PEMEncoder uses 64 characters as line length for private and public keys,
>   and 76 characters for certificates.
>   Use the same line length for all types by default.
>   Consider adding support for selecting a custom line length.
>   Base64.getMimeEncoder also supports selecting a line custom length.
>
> I hope this helps advancing the PEM efforts.
>
> Best regards, Karl
>
> On Thu, Jan 25, 2024 at 10:02 PM Anthony Scarpino
> <anthony.scarpino at oracle.com> wrote:
> >
> >
> > On 1/25/24 9:20 AM, Daniel Jeliński wrote:
> > > Hi Tony,
> > > Thanks for the links! The API looks very promising.
> > >
> > > Out of curiosity, why aren't you using the Base64 MIME
> > > encoder/decoder? They are supposed to produce/remove the newline
> > > characters.
> >
> > I can look it over again.  I had inconsistencies during testing with
> > expected data and returned data.
> >
> > > The relationship between the byte[] and String data should be
> > > specified. Base64 explicitly specifies that the String APIs are
> > > translated to the byte[] APIs with ISO 8859-1 encoding. The PEMDecoder
> > > is currently using the default charset, which might produce
> > > interesting results if the charset is set to EBCDIC. The encoder is
> > > using UTF-8, which is reasonable, but given that the produced output
> > > will always be ASCII, ISO 8859-1 will perform better.
> >
> > That's fine
> >
> > > There's a disparity between the decoder and the encoder APIs; both
> > > work with strings, but Decoder accepts InputStream and not arrays, and
> > > Encoder produces byte arrays, but does not work with streams. This
> > > should be more uniform. I like Decoder's InputStream support (that's
> > > currently the only way to read multiple CA certificates from a single
> > > file - the String overload currently only returns the first one), so
> > > I'd add OutputStream support to Encoder for parity.
> >
> > I don't see parity as a requirement.  I see encoding and decoding as
> > having unique requirements.  Decoding from a file or InputStream makes
> > sense.  A decode(byte[]) method didn't seem necessary as I felt it
> > unlikely a user would have a single byte[] with PEM data.  They were
> > more likely to have a String or an InputStream.  The developer can wrap
> > it with ByteArrayInputStream or String(byte[], Charset), which is what a
> > public API method would do internally.
> >
> > Encoding is a single operation on an object.  Pass in a SecurityObject
> > and PEM data is returned.  Returning a byte[] is the most flexible
> > without creating more methods.  In the case where meta data is
> > in-between the PEM or some other data formatting.
> >
> > Why should the API have an OutputStream method, something like:
> >    pem.encodeToOutputStream(so, outputstream);
> > when the API as written today, the developer can use:
> >    outputstream.writeBytes(pem.encode(obj));
> >
> > I don't like to add API methods just for symmetry, they need to have a
> > compelling reason.  I have not seen that in the OutputStream case.
> >
> >
> > > Karl's earlier
> > > suggestion to support Stream<SecurityObject> also makes a lot of
> > > sense.
> > I haven't counted out Stream, I just haven't seen a compelling reason.
> > My tests use stream() from an array list of PEM strings.  But I haven't
> > seen the situation where the API needs stream support that couldn't be
> > done in a different way.  This is a preview JEP, and we still have time.
> >   If there is a compelling example, point it out to me.
> >
> > > I'm not a big fan of the non-static factory methods
> > > withEncryption/withDecryption/withFactory. The problem with non-static
> > > methods that return an instance of the same type is that you need to
> > > check the documentation to know if the method returns a new instance
> > > or if it mutates the current one. Can we use static factory methods
> > > instead? Either that, or create a builder class.
> >
> > The API states the classes are immutable which was a big requirement and
> > it why it's stated all over the docs.  A builder class was considered
> > early in the API development, but it was too much complication for a few
> > optional cases where the developer may need to specify details like
> > encryption or a factory.  The API has the builder idea, without the
> > extra builder construction methods.  I don't see how a static factory
> > method fit here.
> >
> > > I don't like the PEMEncoder.withEncryption API. It's not predictable
> > > enough; when encoding data, it's not consistent between writing
> > > unencrypted data (certificate, crl), throwing (PublicKey,
> > > EncryptedPrivateKeyInfo) and writing encrypted data (unencrypted
> > > private keys). The alternative of forcing the users to encrypt using
> > > EncryptedPrivateKeyInfo looks better to me.
> >
> > That was a design decision to make the API easier to use.  The
> > non-security export does not need to be burden with understanding
> > EncryptedPrivateKeyInfo.
> >
> > The API can be consistent if you choose to only pass in
> > EncryptedPrivateKeyInfo and not set withEncryption().  If an
> > ArrayList<SecurityObject> encodes with a stream(), it is nice to
> > configure encryption for private keys and still encode public keys and
> > certificates with the same encoder.
> >
> >
> > > I'd love to see support for the OpenSSL private key formats; it seems
> > > that RSAPrivateCrtKeyImpl already supports PKCS#1 format, so it may be
> > > just a matter of exposing that functionality. Other key types like EC
> > > might need more work. That might be added later after the API is
> > > finalized.
> >
> > OpenSSL 3.0 is transitioning away from their format to PKCS#8.  It is an
> > obsoleted format.  While I have thought about decoding support of RSA
> > PKCS#1 for compatibility, I have no intention to support OpenSSL or
> > PKCS#1 encoding with this PEM API.
> >
> > If someone is interested in old OpenSSL or other encoding formats, that
> > is why the Encoder and Decoder interfaces are included.  To give a
> > structure for developing other encoding.
> >
> > >
> > > Thanks,
> > > Daniel
> > >
> > > śr., 24 sty 2024 o 22:24 Anthony Scarpino
> > > <anthony.scarpino at oracle.com> napisał(a):
> > >>
> > >> Hi,
> > >>
> > >> The following github link is to the PEM API as it is written in the
> > >> draft JEP (https://openjdk.org/jeps/8300911).  There has been a few
> > >> changes since the original posting.
> > >>
> > >>
> https://urldefense.com/v3/__https://github.com/ascarpino/jdk/tree/pem__;!!ACWV5N9M2RV99hQ!NmZu22NrC2hxWJuqLHZ6l1C0KYVK0Qf_rV7tV-1uLqUb_5sFMJXyCCKVPjmEmGCeQ6US2RJquDm9AJqZXO46ju8Q$
> > >>
> > >> The Encoder and PEMEncoder to now return byte[] for the encode()
> method.
> > >>    A new PEMEncoder method, encodeToString(), was added.  I believe
> these
> > >> make it easier for outputting data to a file and InputStreams, while
> > >> still supporting a method that returns a String.
> > >>
> > >> For decode, InputStream has replaced Reader.  There were comments
> > >> preferring InputStream and I found that Reader's buffering quirks were
> > >> problematic. Decoding from a byte[] is easy through an
> ByteArrayInputStream.
> > >>
> > >> If you are interested in testing out the API, please download and
> > >> compile the repo.  Then let me know how your experience went.
> > >>
> > >> thanks
> > >>
> > >> Tony
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20240309/e69fbe15/attachment.htm>