... 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.scarp...@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.scarp...@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 >
