WiP: support PKCS12 format for keystore

[nitram509]

This is a DRAFT PR, and any feedback is welcome.
I will change the title, and drop the 'WiP' once it's ready for review ... just wanted to be transparent during development.
Relates to issue #47

---

Technical notes:

• since classic JKS and PKCS12 format are very different, I implement a "switch", which based on magic bytes detects the file format when reading. When writing, a parameter must be provided.
• add new dependency 'software.sslmate.com/src/go-pkcs12' for PKCS12 support
• add new dependency 'github.com/corbym/gocrest' for testing - I find Hamcrest matchers (test style) very readable, and the test failing messages are easier to understand.
• rework the load & store functions into a separate source file (incl. tests) to improve readability somewhat

[pavlo-v-chernykh]

Hi @nitram509

Thank you for your work on this feature, and for keeping me in the loop before it is finalized. I appreciate the time and effort you've been investing.

Regarding tests, while I generally prefer using github.com/stretchr/testify, for this project I've decided to stick with the standard library approach to reduce the number of third-party dependencies to the minimum. Let's maintain consistency across the project's tests by continuing to use the stdlib approach for both existing and new tests.

Great work! Looking forward to completing the implementation 🚀

[pavlo-v-chernykh]

P.S. I think we will release this feature as a new major version. So, if you need new Go features like generics or anything else, feel free to upgrade the Go version in go.mod to the previous major release (1.22).

https://endoflife.date/go
https://go.dev/doc/devel/release#policy

[nitram509]

@pavlo-v-chernykh
thank you for the feedback.

may I reflect one thought:
about the testing lib ... I know testify as well and just have a personal preference for hamcrest. While you mentioned minimal dependencies, that would still work, as runtime code does not need nor use test-dependencies.
And any test-framework is way easier to read (and write), compared to vanilla Go.
Is there a chance to revise/reconcile your decision and use testify?
I'm also willing to migrate the tests accordingly, to make it consistent.

---

One more thing I found out yesterday ... and a related question:
The lib does not yet support private keys, in the truststore - just public keys (it throws an error, as the underlying ASN.1 structure can't be read yet).
What do you think in terms of keystore-go implementing p12 support partially?
Meaning, just public certs (as in e.g. the JDKs) are supported to add/change/remove, but not (yet) private keys nor key-pairs.
What's your view?

[pavlo-v-chernykh]

@pavlo-v-chernykh thank you for the feedback.

may I reflect one thought: about the testing lib ... I know testify as well and just have a personal preference for hamcrest. While you mentioned minimal dependencies, that would still work, as runtime code does not need nor use test-dependencies. And any test-framework is way easier to read (and write), compared to vanilla Go. Is there a chance to revise/reconcile your decision and use testify? I'm also willing to migrate the tests accordingly, to make it consistent.

If you are eager to port all existing tests to Testify, let’s do it.

P.S. In my previous comment, I explained my reasoning regarding dependencies when starting the project back in 2016, before Go modules existed. At that time, Testify was fresh and new and the only dependency I wanted to use. So, I decided to go without it to simplify usage for users. While that is no longer the case, I still prefer to minimize the number of dependencies.

P.S. I understand that test dependencies do not affect the production build, only the time required to download dependencies on CI/CD if users do not use vendoring.

One more thing I found out yesterday ... and a related question: The lib does not yet support private keys, in the truststore - just public keys (it throws an error, as the underlying ASN.1 structure can't be read yet). What do you think in terms of keystore-go implementing p12 support partially? Meaning, just public certs (as in e.g. the JDKs) are supported to add/change/remove, but not (yet) private keys nor key-pairs. What's your view?

Could you elaborate on this? I think I did not fully understand your point. My guess is that the go-pkcs12 library still do not support all the functionality required to be on par with Java implementation and support all the use cases to be compatible with jdk keytool. Am I right?

[nitram509]

Could you elaborate on this? I think I did not fully understand your point. My guess is that the go-pkcs12 library still do not support all the functionality required to be on par with Java implementation and support all the use cases to be compatible with jdk keytool. Am I right?

Indeed. What I found, that reading p12 files with and without a password works with go-pkcs12 lib.
So the typical use case of public certs stored in a truststore of a JDK is covered.
But, when people want to add a keypair, or read a truststore containing a keypair, this is not working, because not supported by the lib.

[pavlo-v-chernykh]

I got it. Could you check, would it be possible to use the code from pkcs12.go#L152 or an Encoder implementation to implement behavior compatible with JDK?
In comments, I see the following:

// Files produced with this encoder can be read by OpenSSL 1.1.1 and higher,
// Java 12 and higher, and Windows Server 2019 and higher.

[nitram509]

@pavlo-v-chernykh the Hacktoberfest is starting soon, and it would be cool to contribute.
May I ask, you could set the topic "hacktoberfest" to this repo, as outlined in the participation rules?
https://hacktoberfest.com/participation/#pr-mr-details

[pavlo-v-chernykh]

@nitram509 hi,

TLDR:

• The current PR will not count for Hacktoberfest because it was created before October 1.
• I will create two separate tasks for the decoder and encoder of PKCS12 to manage expectations.
• I have added the hacktoberfest-accepted label, which you should apply to your next PR; this should be enough for you to participate in Hacktoberfest.

Just to let you know, for your pull requests to count towards Hacktoberfest, they need to be created and merged between October 1 and October 31. So your current PR doesn’t count toward the event. See [out-of-bounds] section https://hacktoberfest.com/participation/#pr-mr-details.

AFAIU the PR labeled with hacktoberfest-accepted label is enough. So I added the hacktoberfest-accepted label to the project, and you can label your next PR with it. I would rather not mark repository with hacktoberfest tag to reduce potential spamming PRs. See [participating] section https://hacktoberfest.com/participation/#pr-mr-details.

To manage expectations, I will create tasks that describe the requirements and acceptance criteria for decoder and encoder separately.

[pavlo-v-chernykh]

@nitram509

The ticket for decoder #52

Please read the description, and let’s discuss any unclear points if required.

[nitram509]

I will close this one, as it seems simpler to me, with latest changes merged.
Will cherry pick, where needed.
Will create a new PR afterwards.