-
Notifications
You must be signed in to change notification settings - Fork 2.5k
ssl: ability to add raw X509 certs to the cert store #6877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ssl: ability to add raw X509 certs to the cert store #6877
Conversation
Just to be clear - this adds it to the in-memory certificate store, this doesn't mutate any files on-disk. In other words, when I start some session, it gets the default on-disk information; and you can add to and enrich this information for only the lifetime of the application. This makes sense to me as a useful feature. Questions:
|
Thank you for review!
|
Thanks; I think that's a helpful improvement!
In your initial comment, you mentioned that "there are custom cert stores on macOS and Windows"; but I don't understand what that comment has to do with this PR. In your application, are you using those custom cert stores on macOS and Windows directly, and then libgit2 makes use of them just because that's the architecture of those libraries? But there's no real way to do this on OpenSSL, which is why you're creating this PR? Or are there custom stores on those systems, but this PR doesn't address those? This is just my ignorance about how these custom certificate systems work on all those platforms, I'm hoping to get some more of this knowledge about how you're working. Thanks! |
Sorry for the misunderstanding. I mentioned certificate stores to provide context for the feature. Let me explain further: On macOS, there's the Apple Keychain, and on Windows, the Windows Credential Manager. Both offer APIs to list certificates in the store and retrieve them in X509 format, which our application does. Once we retrieve the certificates, we need to add them to the SSL backend in libgit2. My changes make this possible. |
Oh! You're building with OpenSSL on all platforms and you're pulling carts out of the various platform specific subsystems and then feeding them into OpenSSL. Is that roughly accurate? I had assumed that you were building with OpenSSL on Linux (and using the win32 and Mac TLS stacks on windows and Mac, respectively). That all makes sense; like I said, just curious about your use case and how / or if we should think about this setting for Mac and windows. |
(No need to apologize btw, I was making assumptions.) |
Yes, you’re right. We’re building with openssl on all platforms (win, mac, linux) because of our app specific. We don’t like the OS specific ssl stacks for less transparency. |
Question: does this compile with |
Fixed in commit below. |
I have also added a test that is similar to others in the |
We provide functionality for callers to mutate the SSL context. We want to test these various mutations, but OpenSSL does not provide a mechanism to _reset_ these mutations (short of burning down the SSL context). Provide an internal mechanism to reset the state for our own tests.
Update the custom cert tests to test various custom certificates.
The OpenSSL certificate setting functions _may_ interact; try to document that a bit better.
I needed the cert and the key, so I created a new one for test.libgit2.org and updated the PR. I also noticed that the custom cert tests would fail unless you cleared the context between runs, so I added a (currently) internal function to do that. This passes the CI runs - please take a look and make sure that I didn't break anything? |
Your changes look fine. Thanks for help! |
Thanks @yerseg ! |
There are custom cert stores on macOS and Windows (Apple Keychain, Windows Credentials Manager). In our product we enumerate certs in the stores and add them to SSL cert store. It seems like useful feature at all.