karthink / gptel

A simple LLM client for Emacs
GNU General Public License v3.0
1.04k stars 113 forks source link

Add support for mutual TLS #196

Closed r0man closed 4 months ago

r0man commented 5 months ago

Hi @karthink,

first, thanks for GPTel and the great video you made about it. I enjoyed it!

I would like to use GPTel with an OpenAI gateway at work that requires mutual TLS.

This change adds 2 new slots to the gptel-backend struct to support mutual TLS:

The client certificate and private key are then used in the curl backend via the --cert and --key options when making HTTP requests.

There's one caveat. Ideally this would also be added for the url-retrieve backend. I would have liked to test this with the gateway I'm using, but for some reason GNUTLS has issues with the client certificate and key I have at hand. It might be that it works out of the box by adding something like this to your ~/.authinfo.gpg file and set network-stream-use-client-certificates:

machine gateway.exmple.com port 443 key /path/to/my-key.pem cert /path/to/my-cert.pem

But I couldn't get this working. The changes to the curl backend would allow me to use a LLM at work.

Would you like to add this to GPTel?

Thanks, Roman.

karthink commented 5 months ago

@r0man Thank you for the PR. A little busy at the moment but will take a look at it soon. One thought i had was whether there should be a catchall field (:curl-args or something) instead of :client-certificate and :private-key in the backend struct. This way any Curl args that the user wants to add to the connection in the future can be incorporated without requiring further changes to the source -- Curl has many, many arguments!

I'm imagining it might look like:

(gptel-make-openai "OpenAI-TSL"
  :stream t
  :key gptel-api-key
  :models '("gpt-4")
  :curl-args '("--key" "ABCD1234" "--cert" "ABCD1234"))

or

:curl-args `("--key" ,(my-openai-conn-private-key)
             "--cert" ,(my-openai-conn-certificate))

since you probably don't want to store the key and certificate in plain-text in your Emacs configuration.

This needs a little more thought -- especially for url.el support. There's also something to be said for the direct approach: :certificate and :private-key don't require knowledge of Curl command line arguments.

r0man commented 5 months ago

Hi @karthink,

thanks for taking a look. If we eventually want to support mutal TLS with url-retrieve as well, I think I would prefer the :client-certificate and :client-private-key slot approach. Mostly, because the user configuring the backend does not have to care if curl or url-retrieve is used.

Note, that the values for those slots are filenames, pointing to the certificate and private key on disk. Curl and the lower level Emacs networking functions expect the certificate and key to be filenames.

If we go with the :client-certificate and :client-private-key slot approach, I don't have a concrete use case for setting the curl options in the backend myself right now. But I think it could be useful for people implementing backends for more exotic APIs (as I'm doing now).

Please let me know what you think. I'm also happy to work on another pull request, implementing the curl-args approach.

Thanks, Roman.

r0man commented 4 months ago

Hi @karthink, anything I can do to move this forward?

karthink commented 4 months ago

To use the :client-certificate and :client-private-key slots, I'd like it to include url-retrieve support. I haven't had time to figure out how to pass these parameters to the url.el library, it's not documented very well.

The other approach (a :curl-args slot) is preferable if this is Curl-only. As a bonus it covers all custom arguments the user might want to pass to Curl. I'm leaning towards this approach.

r0man commented 4 months ago

Hi @karthink, yeah, I agree it is not documented very well. I'm still fighting with GnuTLS and the certificates I have to use. Anyway, I opened another PR that adds a curl-args slot to the backend. https://github.com/karthink/gptel/pull/221 Can you take a look at this one please? Thanks, Roman.