Re: Emacs core TLS support

[Simon Josefsson <simon@josefsson.org>]

Ted Zlatanov <tzz@lifelogs.com> writes:

> +(defconst gnutls-version "0.3.1")

This should be removed.  If the GnuTLS version is at all interesting for
elisp callers, there could be an elisp function gnutls-check-version
that in C calls gnutls_check_version.

> +(defun open-ssl-stream (name buffer host service)
> +  "Open a SSL connection for a service to a host.

I suggest using 'TLS' or possibly 'SSL/TLS' consistently in
documentation.  Is 'open-ssl-stream' for backwards compatibility?
Otherwise I suggest 'open-tls-stream'.

> +
> +;; (open-ssl-stream "tls" "tls-buffer" "yourserver.com" "https")

Looks like debug code that should be removed?

> +(defun starttls-negotiate (proc &optional priority-string
> +                                credentials credentials-file)
> +  "Negotiate a SSL or TLS connection.

Here I suggest 'TLS' or 'SSL/TLS' instead.

> +PRIORITY-STRING is as per the GnuTLS docs.

Maybe there could be an info hyperlink here?

> +                               "/tmp/ca.pem"

This should be fixed, naturally.
> +
> +         (priority-string (or priority-string
> +                              (cond
> +                               ((eq credentials 'gnutls-anon)
> +                                "PERFORMANCE:+ANON-DH:!ARCFOUR-128")
> +                               ((eq credentials 'gnutls-x509pki)
> +                                "PERFORMANCE"))))

I think NORMAL should be used instead of PERFORMANCE here.

> +    (gnutls-message-maybe
> +     (setq ret (gnutls-boot proc priority-string credentials credentials-file))
> +     "boot: %s")

How much debug code do we want to retain?  I'm not sure.

> +(defun starttls-open-stream (name buffer host service)
> +  "Open a TLS connection for a service to a host.

'TLS' or 'SSL/TLS' again.

> +DEFUN ("gnutls-global-init", Fgnutls_global_init,
> +       Sgnutls_global_init, 0, 0, 0,
> +       doc: /* Initializes global GNU TLS state to defaults.
> +Call `gnutls-global-deinit' when GNU TLS usage is no longer needed.
> +Returns zero on success.  */)
...
> +DEFUN ("gnutls-global-deinit", Fgnutls_global_deinit,
> +       Sgnutls_global_deinit, 0, 0, 0,
> +       doc: /* Deinitializes global GNU TLS state.
> +See also `gnutls-global-init'.  */)

I think this shouldn't be exposed to Elisp, Emacs startup code could
initialize GnuTLS directly.

> +DEFUN ("gnutls-boot", Fgnutls_boot, Sgnutls_boot, 3, 6, 0,
> +       doc: /* Initializes client-mode GnuTLS for process PROC.
> +Currently only client mode is supported.  Returns a success/failure
> +value you can check with `gnutls-errorp'.
> +
> +PRIORITY_STRING is a string describing the priority.
> +TYPE is either `gnutls-anon' or `gnutls-x509pki'.
> +TRUSTFILE is a PEM encoded trust file for `gnutls-x509pki'.
> +KEYFILE is ... for `gnutls-x509pki' (TODO).
> +CALLBACK is ... for `gnutls-x509pki' (TODO).

Two comments here: 1) The name is a bit generic..?  2) The design makes
it a bit difficult to support multiple credentials.  The GnuTLS API
allows clients to have several credentials (X.509, OpenPGP, etc).
Perhaps copying the GnuTLS API further is more flexible.

Good work.  I think you are getting there!

/Simon