From: Ted Zlatanov <tzz@lifelogs.com>
To: emacs-devel@gnu.org
Subject: Re: GnuTLS and zeroing keys in Emacs
Date: Sat, 15 Jul 2017 15:00:23 -0400 [thread overview]
Message-ID: <877ez9frug.fsf@lifelogs.com> (raw)
In-Reply-To: 9ee79781-8de0-06ef-6477-28af9852a92b@cs.ucla.edu
On Fri, 14 Jul 2017 16:42:22 -0700 Paul Eggert <eggert@cs.ucla.edu> wrote:
PE> Thanks for your recent GnuTLS-related contributions for Emacs. I found some
PE> minor glitches with respect to integer overflow detection and style, and
PE> installed the attached patch to try to help clear up the problems I
PE> found.
Beautiful, thank you for this. I appreciate your corrections very much
and will try to follow that style in the future as well. I agree with
them except in minor issues as noted below, and you're welcome to push
them any time.
PE> I did notice one other thing: sometimes the new code zeros out keys that will be
PE> garbage, I guess under the theory that this will help protect the user from
PE> hostile code within Emacs that attempts to steal keys by reading "unused"
PE> memory. However, zeroing out these keys does not actually destroy the keys
PE> reliably, as some compilers elide code that clears about-to-become-garbage
PE> objects, and some of the strings may be moved in garbage-collected memory (so
PE> their old contents are not zeroed).
PE> I left in the code that clears keys, but I'm wondering: is there some general
PE> Emacs-internal guideline about this sort of thing? If we're serious about
PE> clearing keys I suppose we need to tell the GC and compilers not to move them
PE> around or optimize away useless stores. If not, then perhaps we should mark the
PE> key-clearing code in some way that says "this doesn't work in general but is the
PE> best we can easily do".
Hmm, the best way is to either use gnutls_memset() (available since only
3.4.0 in lib/safe-memfuncs.c) or to copy it. If you think this will only
have utility in GnuTLS-related code, I'd make a wrapper that calls
gnutls_memset() if it's available, taking advantage of future
improvements, or our own version of it. If you think it's generally
useful, I'd copy it right away so it's available even if GnuTLS is not.
My vote is to copy it. Let me know if you want to do it, or if I should.
/**
* gnutls_memset:
* @data: the memory to set
* @c: the constant byte to fill the memory with
* @size: the size of memory
*
* This function will operate similarly to memset(), but will
* not be optimized out by the compiler.
*
* Returns: void.
*
* Since: 3.4.0
**/
void gnutls_memset(void *data, int c, size_t size)
{
volatile unsigned volatile_zero = 0;
volatile char *vdata = (volatile char*)data;
/* This is based on a nice trick for safe memset,
* sent by David Jacobson in the openssl-dev mailing list.
*/
if (size > 0) {
do {
memset(data, c, size);
} while(vdata[volatile_zero] != c);
}
}
PE> - Lisp_Object cp = listn (CONSTYPE_HEAP, 15,
PE> - /* A symbol representing the cipher */
PE> - intern (gnutls_cipher_get_name (gca)),
You removed the comments from these lists in the C code in several
places, but I do think they are useful to a reader. Can we keep them?
PE> + ret = ((encrypting ? gnutls_aead_cipher_encrypt : gnutls_aead_cipher_decrypt)
PE> + (acipher, vdata, vsize, aead_auth_data, aead_auth_size,
PE> + cipher_tag_size, idata, isize, storage, &storage_length));
...
PE> + ret = ((encrypting ? gnutls_cipher_encrypt2 : gnutls_cipher_decrypt2)
PE> + (hcipher, idata, iend_byte - istart_byte,
PE> + SSDATA (storage), storage_length));
As a matter of readability, I'd make a function pointer variable and
assign it separately. But I realize the advantage of brevity and
type-agnostic code here, so I leave it up to you.
PE> -Returns nil on error.
PE> +Return nil on error.
Is this right? It seems it's talking from the POV of the function.
Thanks
Ted
next prev parent reply other threads:[~2017-07-15 19:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-14 23:42 GnuTLS and zeroing keys in Emacs Paul Eggert
2017-07-15 19:00 ` Ted Zlatanov [this message]
2017-07-16 23:53 ` Paul Eggert
2017-07-17 13:52 ` Ted Zlatanov
2017-07-17 0:29 ` Paul Eggert
2017-07-17 14:07 ` Ted Zlatanov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877ez9frug.fsf@lifelogs.com \
--to=tzz@lifelogs.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.