Subject: bug#50507: New function in Emacs GnuTLS implementation

From: Robert Pluim <[email-masked]>
Date: Mon, 26 Sep 2022 19:19:50 +0200

>>>>> On Mon, 26 Sep 2022 11:43:41 -0400, Nikolaos Chatzikonstantinou <[email-masked]> said:
    Nikolaos> Date: Mon, 26 Sep 2022 11:08:18 -0400
    Nikolaos> Subject: [PATCH] fix(gnutls): add possibility of password for key-file

    Nikolaos> The GnuTLS function

    Nikolaos>     gnutls_certificate_set_x509_key_file

    Nikolaos> is replaced by its second version

    Nikolaos>     gnutls_certificate_set_x509_key_file2

    Nikolaos> and the definitions of gnutls-boot and gnutls-boot-parameters are
    Nikolaos> modified to include the :pass and :flags keys, which are additional
    Nikolaos> parameters in the second version.

    Nikolaos> Signed-off-by: Nikolaos Chatzikonstantinou
    Nikolaos> <[email-masked]>

We donʼt use Signed-off-by, and the commit message has some rules
which are described in CONTRIBUTE (start at "** Commit messages" and
read up to and including "** Committing your changes")

    Nikolaos> +PASS is a string, the password of the key.
    Nikolaos> +
    Nikolaos> +FLAGS is an ORed sequence of gnutls_pkcs_encrypt_flags_t values.
    Nikolaos> +

Youʼre at the lisp level here. Perhaps you could define a mapping from
the C-level enum to lisp defconsts or similar? Or you could define it
as taking a list of flags, and then the C-code can take care of ORing
them.

    Nikolaos> +  pass                  = plist_get (proplist, QCpass);
    Nikolaos> +  flags                 = plist_get (proplist, QCflags);

pass and flags will both be 'nil' here if theyʼre not specified, so
that....

    Nikolaos>    if (!STRINGP (hostname))
    Nikolaos>      {
    Nikolaos> @@ -2038,8 +2051,8 @@ DEFUN ("gnutls-boot", Fgnutls_boot, Sgnutls_boot, 3, 3, 0,
    Nikolaos>  	      keyfile = ansi_encode_filename (keyfile);
    Nikolaos>  	      certfile = ansi_encode_filename (certfile);
    Nikolaos>  # endif
    Nikolaos> -	      ret = gnutls_certificate_set_x509_key_file
    Nikolaos> -		(x509_cred, SSDATA (certfile), SSDATA (keyfile), file_format);
    Nikolaos> +	      ret = gnutls_certificate_set_x509_key_file2
    Nikolaos> +		(x509_cred, SSDATA (certfile), SSDATA (keyfile), file_format, SSDATA (pass), XUFIXNUM (flags));

...this is likely to fail in that case. Or maybe not, I havenʼt tested
it, but XUFIXNUM(nil) in a build with asserts enabled will trigger an
assert and exit, I think.

In any case, if youʼre going to replace _file with _file2, you should
describe the new constraints on the arguments. e.g. Maybe having pass
as nil is OK, but then you need to say that, or maybe you need to fall
back to _file if :pass is not specified.

Robert
--