Re: Emacs core TLS support

[Andreas Schwab]

Ted Zlatanov <address@hidden> writes:

> +int
> +emacs_gnutls_write (int fildes, gnutls_session_t state, char *buf,
> +                    unsigned int nbyte)
> +{
> +  register int rtnval, bytes_written;
> +
> +  puts("emacs_gnutls_write");

You should remove the debugging output.

> +DEFUN ("gnutls-init", Fgnutls_init, Sgnutls_init, 2, 2, 0,
> +       doc: /* Initializes GNU TLS for process PROC for use as 
> CONNECTION-END.
> +CONNECTION-END is used to indicate if this process is as a server or
> +client. Can be one of `gnutls-client' and `gnutls-server'.  Currently
> +only `gnutls-client' is supported.
> +
> +Processes must be initialized with this function before other GNU TLS
> +functions are used.  This function allocates resources which can only
> +be deallocated by calling `gnutls-deinit'. Returns zero on success. */)
> +    (Lisp_Object proc, Lisp_Object connection_end)
> +{
> +  int ret;
> +  
> +  CHECK_PROCESS (proc);
> +
> +  ret = gnutls_init((gnutls_session_t*)&(XPROCESS(proc)->gnutls_state), 

Aliasing violation.

> +                 connection_end);
> +
> +  return XINT(ret);

IMHO all your functions should return t on success and either some error
symbol on failure or even raise an error.

> +DEFUN ("gnutls-cred-set", Fgnutls_cred_set, 
> +       Sgnutls_cred_set, 2, 2, 0,
> +       doc: /* Enables GNU TLS authentication for PROCESS.
> +TYPE is an integer indicating the type of the credentials, either
> +`gnutls-anon', `gnutls-srp' or `gnutls-x509pki'.
> +
> +Each authentication type may need additional information in order to
> +work.  For anonymous (`gnutls-anon'), see also
> +`gnutls-anon-set-client-cred'.       For SRP (`gnutls-srp'), see also
> +`gnutls-srp-set-client-cred'.  For X.509 PKI (`gnutls-x509pki'), see
> +also `gnutls-x509pki-set-client-trust-file',
> +`gnutls-x509pki-set-client-key-file', and
> +`gnutls-x509pki-set-cert-callback'. */)
> +    (Lisp_Object proc, Lisp_Object type)
> +{
> +  gnutls_session_t state;
> +  gnutls_certificate_credentials_t x509_cred;
> +  gnutls_anon_client_credentials_t anon_cred;
> +  gnutls_srp_client_credentials_t srp_cred;
> +  int ret;
> +
> +  CHECK_PROCESS (proc);
> +  state = (gnutls_session_t) XPROCESS(proc)->gnutls_state;
> +
> +  x509_cred = (gnutls_certificate_client_credentials) 
> XPROCESS(proc)->x509_cred;
> +  anon_cred = (gnutls_anon_client_credentials_t) XPROCESS(proc)->anon_cred;
> +  srp_cred = (gnutls_srp_client_credentials_t) XPROCESS(proc)->srp_cred;
> +
> +  switch (XINT (type))

Need to check type.

> +  return XINT(ret);

     return make_number (ret);

> +  // defsubr (&Sgnutls_x509pki_set_client_key_file);
> +  // defsubr (&Sgnutls_x509pki_set_client_trust_file);
> +  // defsubr (&Sgnutls_srp_set_client_cred);
> +  // defsubr (&Sgnutls_anon_set_client_cred);

No C99.

> === added file 'src/gnutls.h'
> --- src/gnutls.h      1970-01-01 00:00:00 +0000
> +++ src/gnutls.h      2010-09-05 04:42:32 +0000
> @@ -0,0 +1,4 @@
> +#ifdef HAVE_GNUTLS
> +#include <gnutls/gnutls.h>
> +
> +#endif

I don't see the point of this header.

> === modified file 'src/process.h'
> --- src/process.h     2010-08-11 12:34:46 +0000
> +++ src/process.h     2010-09-05 04:42:32 +0000
> @@ -121,6 +121,14 @@
>         needs to be synced to `status'.  */
>      unsigned int raw_status_new : 1;
>      int raw_status;
> +
> +#ifdef HAVE_GNUTLS
> +    /* XXX Store GNU TLS state and auth mechanisms in Lisp_Objects. */
> +    Lisp_Object gnutls_state;
> +    Lisp_Object x509_cred, x509_callback;
> +    Lisp_Object anon_cred;
> +    Lisp_Object srp_cred;
> +#endif

None of them should be Lisp_Objects.  Also make sure the resources are
properly released when the process object is deleted.

Andreas.

-- 
Andreas Schwab, address@hidden
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."