Re: libcurl leaks information in freed memory

2018-10-22 Thread Gabriel Zachmann via curl-library



On 10/22/18 3:24 PM, Daniel Stenberg wrote:
curl_global_init_mem - 
https://curl.haxx.se/libcurl/c/curl_global_init_mem.html
Oh, didn't know about that. Actually I did look for something like that. 
Thank's for pointing it out.


--
Gabriel



smime.p7s
Description: S/MIME Cryptographic Signature
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-22 Thread Rich Gray via curl-library

Daniel Stenberg via curl-library wrote:

On Mon, 22 Oct 2018, Gabriel Zachmann via curl-library wrote:


Actually would be possible to allow an application to supply an
allocator and deallocator callbacks to libcurl via an option?


Sure. I don't know why I forgot about that option. I like that way and 
think that's the way to go.


curl_global_init_mem - https://curl.haxx.se/libcurl/c/curl_global_init_mem.html

I suppose what could still be looked into further is if/where we keep data 
around longer than we need to before we free it.




Shooting from the hip as a non-developer, perhaps the starting point might be:

- Establish a standard libcurl secure erase routine convention which could 
be a callback.  It could default to a secure erase routine if available 
(memset_s), otherwise be plain memset(a,0,n).  It would not necessarily zero 
memory, it could write random(ish) data instead if desired.  Perhaps its 
definition would be that it is only used to do otherwise unnecessary 
erasures, it would be a nop in environments where this level of security is 
not required.  (Or have a second secure_memory_cleanup() function with this 
meaning...? )


- establish standard allocator & deallocator calls, again settable as 
callbacks, defaulting to the normal allocators (malloc/free) with something 
like a prefix of bytes to erase being set by the secure allocator.  But make 
a second, allocator for stuff that doesn't have to be secure (sets bytes to 
erase count to zero?)  This second one would be the normally used workhorse. 
 The single deallocator could tell whether erasure is needed, so one 
doesn't have to worry about failing to call the matching one.  Maybe these 
[de]allocators would only be used in secure routines, the remaining code 
could continue to use the normal malloc/free or whatever, unaffected by 
these changes.


With these four calls, even as simple wrappers/macros for the current, 
non-wiping usage, developers could start placing these calls in code where 
they think they are needed.  If nothing else, this would flag security 
sensitive areas in the code.  The underlying functions could be worked on 
over time as experience is gained, especially with different 
systems/compilers.  Make an example source file (based on whatever libcurl's 
versions do) so users can modify it and set their own callbacks.


Maybe a simple global setopt would tell libcurl whether to use insecure 
routines or not.  (0=insecure 1=libcurl secure versions 2=user?)  There 
would be setopt calls to set user callbacks for these functions.  Does this 
functionality need to be per easy handle, so that insecure protocols don't 
pay any penalty??



Rats, didn't look at curl_global_init_mem() until now.  Maybe add secure_ 
versions?  (Except free().  For sanity, it should be able to do the right 
thing with a secure or insecure allocations?)  I just threw in a secure 
erase option (or two).  Do you need variable arguments here for expansion or 
maybe define a struct table of these calls to allow expansion?


Insecure (current) behavior:
curl_global_init_mem(CURL_GLOBAL_DEFAULT, CURL_INSECURE_MEMORY_FNS);

Use built-in secure routines:
curl_global_init_mem(CURL_GLOBAL_DEFAULT, CURL_SECURE_MEMORY_FNS);

User specified:
curl_global_init_mem(CURL_GLOBAL_DEFAULT, _secure_fn_table);


(If one went the crazy of per easy handle control, having these functions as 
a handy table would allow easy switching.)


Maybe you could do something with variable args and flags bits specifying 
what follows (including non-memory related things) or variable args with a 
stopper value so the list can grow.


Cheers!
Rich
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-22 Thread Daniel Stenberg via curl-library

On Mon, 22 Oct 2018, Gabriel Zachmann via curl-library wrote:


Actually would be possible to allow an application to supply an
allocator and deallocator callbacks to libcurl via an option?


Sure. I don't know why I forgot about that option. I like that way and think 
that's the way to go.


curl_global_init_mem - 
https://curl.haxx.se/libcurl/c/curl_global_init_mem.html


I suppose what could still be looked into further is if/where we keep data 
around longer than we need to before we free it.


--

 / daniel.haxx.se
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-22 Thread Gabriel Zachmann via curl-library



On 10/22/18 12:05 PM, Petr Pisar via curl-library wrote:


Actually would be possible to allow an application to supply an
allocator and deallocator callbacks to libcurl via an option? This way
the application could control the sensitive data storage. E.g. by
allocating a memory from core-locked (non-swappable) region. It could
also scrub the data from the memory instead of libcurl. The callback
could also be used by underlying crypto library for storing session keys
etc. In other words the application would become responsible for the
safety measures. libcurl would only use the callbacks instead of a
native allocator (if provided).


Sure. I don't know why I forgot about that option. I like that way and 
think that's the way to go.


--
Gabriel



smime.p7s
Description: S/MIME Cryptographic Signature
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-22 Thread Petr Pisar via curl-library
On Mon, Oct 22, 2018 at 08:13:45AM +0200, Gabriel Zachmann via curl-library 
wrote:
> On 10/19/18 11:49 AM, Erik Janssen wrote:
> 
> > That said, explicit wipe of the most sensitive parts, probably controlled 
> > by the application through options, would be low-cost, and reduces the 
> > chance of exporting them in core dumps, etc.
> 
[...]
> still keep sensitive information in their own memory. However, for 
> applications that clear their own copy, an option would be nice for 
> libcurl clearing the memory, maybe by an explicit call in the suggested way:

Actually would be possible to allow an application to supply an
allocator and deallocator callbacks to libcurl via an option? This way
the application could control the sensitive data storage. E.g. by
allocating a memory from core-locked (non-swappable) region. It could
also scrub the data from the memory instead of libcurl. The callback
could also be used by underlying crypto library for storing session keys
etc. In other words the application would become responsible for the
safety measures. libcurl would only use the callbacks instead of a
native allocator (if provided).

--Petr



signature.asc
Description: PGP signature
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-22 Thread Gabriel Zachmann via curl-library



On 10/19/18 11:49 AM, Erik Janssen wrote:


That said, explicit wipe of the most sensitive parts, probably controlled by 
the application through options, would be low-cost, and reduces the chance of 
exporting them in core dumps, etc.


I think that this would be a good way to go.
I agree that wiping all allocated memory might have performance impacts 
and that is generally an overkill. Also because most applications will 
still keep sensitive information in their own memory. However, for 
applications that clear their own copy, an option would be nice for 
libcurl clearing the memory, maybe by an explicit call in the suggested way:
> I so see a point in explicitly wiping previous url or credentials 
when the next one, or empty string, is specified.
That way there are no performance impacts for average programs, but 
programs that care about sensitive data in a special way can explicitly 
clear it from libcurl.


Sure every effort we take cannot eliminate the possibility to obtain 
information from the process, but we can shrink the time window and make 
it harder; and I think we should give the user the possibility to do so.


--
Gabriel



smime.p7s
Description: S/MIME Cryptographic Signature
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-19 Thread Dennis Clarke via curl-library

On 10/19/2018 10:15 AM, Daniel Stenberg via curl-library wrote:

On Fri, 19 Oct 2018, Gabriel Zachmann wrote:

memory before the pointer it returns. 

I attached some code that should be capable of doing so.


If clearing the memory just before free is all that's necessary, I 
suppose an alternative option is to link in a malloc replacement that 
does exactly this.


Please no.

There must be a way to fire off a thread to clear the memory and then
free it and maybe call it clear_free() or some trivial but blunt trauma
approach which works without a malloc/calloc replacement.

Dennis
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-19 Thread Daniel Stenberg via curl-library

On Fri, 19 Oct 2018, Gabriel Zachmann wrote:

memory before the pointer it returns. 

I attached some code that should be capable of doing so.


If clearing the memory just before free is all that's necessary, I suppose an 
alternative option is to link in a malloc replacement that does exactly this.


However, some of that data will then be around during entire transfers for 
potentially a very long time already, so I'd question how much such a simple 
fix will do (besides adding a lot of overhead).


Also, as has been pointed out, in most cases the sensible data like user names 
or passwords are alredy held in memory by the application that passes that 
information to libcurl and that application is likely to (in most situations) 
hold on to that data for the duration of the transfer. In memory.


--

 / daniel.haxx.se
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-19 Thread Daniel Gustafsson via curl-library
> On 19 Oct 2018, at 10:55, Gabriel Zachmann via curl-library 
>  wrote:
> 
> On 19.10.18 08:58, Gabriel Zachmann via curl-library wrote:
> 
>> get the length. But yes, if all allocated memory should be freed, we have to 
>> keep track of the size. A simple way to do so is using an custom allocator 
>> that allocates more memory as requested and saves the size in the memory 
>> before the pointer it returns. 
> I attached some code that should be capable of doing so.

Even with this approach, this is shrinking the window rather than eliminating
it, as pointed out by Colin Percival in the linked article upthread.

Since this is an attack which require local privilege escalation to work,
aren’t we fighting an uphill battle as a rogue root just as well can set a
breakpoint in your process and steal credentials before memory is cleared?

Now, I’m not saying that we shouldn’t do what we can to scrub memory in some
cases, we probably should.  But, we need to start by identifying which cases
that are important, why they are important and to which end we are doing it.

cheers ./daniel
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

RE: libcurl leaks information in freed memory

2018-10-19 Thread Erik Janssen via curl-library
Hi,

I doubt whether curl should go that way. 

I so see a point in explicitly wiping previous url or credentials when the next 
one, or empty string, is specified. It would reduce, but not eliminate, a 
potential attack surface. However, generic wipe of memory is expensive but 
doesn't solve the potential vulnerability. 

One can't malloc() all memory and get data from another application on 
Linux/Windows as these OS-es won't let that happen? One can peek in process 
memory only when the privileges allow. 

Overwriting credentials reduces the window in which this data is visible but in 
the end curl needs to have it at least for the duration of the transfer. So, 
the risk is reduced but not eliminated. And even when an application removes 
credentials from curl, it is likely to have them somewhere else in memory, 
still in reach of debug tools. Same for data. I think secure deployment of an 
application should be arranged by proper isolation on OS level, so others can't 
peek into it.

Smaller systems may have realtime OS-es that don't clear memory between 
processes for performance reasons. I don't think curl should do an expensive 
wipe by default on such systems.

That said, explicit wipe of the most sensitive parts, probably controlled by 
the application through options, would be low-cost, and reduces the chance of 
exporting them in core dumps, etc.

My 2ct,

Erik

-Original Message-
From: curl-library  On Behalf Of Gabriel 
Zachmann via curl-library
Sent: vrijdag 19 oktober 2018 10:56
To: libcurl development ; Daniel Stenberg 

Cc: Gabriel Zachmann ; Hardt, Marcus 

Subject: Re: libcurl leaks information in freed memory



On 19.10.18 08:58, Gabriel Zachmann via curl-library wrote:

> get the length. But yes, if all allocated memory should be freed, we 
> have to keep track of the size. A simple way to do so is using an 
> custom allocator that allocates more memory as requested and saves the 
> size in the memory before the pointer it returns.
I attached some code that should be capable of doing so.

> And measuring if the cleared memory was really cleared is also not that 
> trivial.

We can use the following commands to do so:
gcore -o /tmp/ $PID
cat /tmp/.$PID | strings | grep yyy

and use a characteristic string for yyy, so we can see if that string 
was really removed from memory.

But that would not be a simple unit test.


--
Gabriel

---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-19 Thread Kamil Dudka via curl-library
On Thursday, October 18, 2018 4:00:10 PM CEST Daniel Stenberg via curl-library 
wrote:
> On Wed, 17 Oct 2018, Gabriel Zachmann wrote:
> >> This, plus zeroing buffers is really hard.
> > 
> > I understand that this is not quite easy. However, while we won't find an
> > optimal solution, I think we can do better. Zeroing memory might not
> > succeed in all cases and there might be still some parts left on the
> > stack, register, etc. But I think we can still memsetting most and it
> > will be much harder to get sensitive information.
> 
> Any suggestion on how to do this and make sure the compiler doesn't remove
> the memset() ?
> 
> Also, are you suggesting we clear the memory for all frees? If yes, then we
> need to keep track of the sizes somehow and if no, then we need to figure
> out which ones and deal with the appropriately.

It sounds like overkill to me.  Clearing all memory to be freed all the time 
would have severe performance impact for no real benefit.

Kamil

> Can we come up with a way to measure this if we are doing this right or not?


---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-19 Thread Gabriel Zachmann via curl-library



On 19.10.18 08:58, Gabriel Zachmann via curl-library wrote:

get the length. But yes, if all allocated memory should be freed, we 
have to keep track of the size. A simple way to do so is using an custom 
allocator that allocates more memory as requested and saves the size in 
the memory before the pointer it returns. 

I attached some code that should be capable of doing so.

And measuring if the cleared memory was really cleared is also not that 
trivial.


We can use the following commands to do so:
gcore -o /tmp/ $PID
cat /tmp/.$PID | strings | grep yyy

and use a characteristic string for yyy, so we can see if that string 
was really removed from memory.


But that would not be a simple unit test.


--
Gabriel
#include 
#include 

static void* (*const volatile memset_ptr)(void*, int, size_t) = memset;

static void moresecure_memzero(void* p, size_t len) {
  __asm__("" : : "m"(p));
  (memset_ptr)(p, 0, len);
}

void* secAlloc(size_t size) {
  if (size == 0) {
return NULL;
  }
  size_t sizesize = sizeof(size);
  void*  p= calloc(size + sizesize, 1);
  if (p == NULL) {
return NULL;
  }
  *(size_t*)p = size;
  return p + sizesize;
}

#define secFree(ptr) \
  do {   \
_secFree((ptr)); \
(ptr) = NULL;\
  } while (0)
#define secFreeN(ptr, len)   \
  do {   \
_secFreeN((ptr), (len)); \
(ptr) = NULL;\
  } while (0)

void _secFreeN(void* p, size_t len) {
  if (p == NULL) {
return;
  }
  moresecure_memzero(p, len);
  free(p);
}

void _secFree(void* p) {
  if (p == NULL) {
return;
  }
  void*  freeptr = p - sizeof(size_t);
  size_t len = *(size_t*)freeptr;
  secFreeN(freeptr, len);
}

void* secRealloc(void* p, size_t size) {
  if (p == NULL) {
return secAlloc(size);
  }
  if (size == 0) {
secFree(p);
return NULL;
  }
  size_t oldsize = *(size_t*)(p - sizeof(size_t));
  size_t movelen = oldsize < size ? oldsize : size;
  void*  newp= secAlloc(size);
  if (newp == NULL) {
return NULL;
  }
  memmove(newp, p, movelen);
  secFree(p);
  return newp;
}


smime.p7s
Description: S/MIME Cryptographic Signature
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-19 Thread Gabriel Zachmann via curl-library



On 18.10.18 16:00, Daniel Stenberg wrote:
Any suggestion on how to do this and make sure the compiler doesn't 
remove the memset() ?


I'm not that familiar with compiler optimization, so I'm not sure if I 
can bring that many useful ideas. But at least for Windows there seems 
to be a SecureZeroMemory function that can be used. However, this does 
not help for Linux. According to HackerNews [1] we can use inline 
assembler to ensure that the compiler does not optimize the memset away. 
But the presented solution is for GNU C, so it should work for gcc and 
clang, but it is not really a portable solution; depending on your 
requirements.
I"m afraid that a fully portable solution involves hoping that most 
compilers won't optimize that much. In this case the approach from [2] 
still seems viable. It might not be fully foolproof, but it is 
questionable if a compiler will really optimize that much.
I guess there might be a gap between theory and practice. In theory we 
cannot ensure that a compiler does not remove the memset, however in 
practice it should work in almost any cases (at least I hope so). Do you 
know of any case where a compiler really removed a memset (a "normal" 
one as well as something like [2])?


[1]: https://news.ycombinator.com/item?id=8271205
[2]: http://www.daemonology.net/blog/2014-09-05-erratum.html

Also, are you suggesting we clear the memory for all frees? If yes, then 
we need to keep track of the sizes somehow and if no, then we need to 
figure out which ones and deal with the appropriately.


I think it would be best to clear memory for all frees. This way it's 
also easier to ensure that all sensitive memory is cleared and that none 
is forgotten. But this might not be necessary.
I think the most important part are all strings in a request (the body 
as well as all headers). Internal allocations for structs, etc. are not 
that sensitive I guess.
Clearing zero terminated strings is also easier, because we easily can 
get the length. But yes, if all allocated memory should be freed, we 
have to keep track of the size. A simple way to do so is using an custom 
allocator that allocates more memory as requested and saves the size in 
the memory before the pointer it returns. I think malloc does something 
similar, but I guess we cannot rely on that and access the memorysize 
that way.


Can we come up with a way to measure this if we are doing this right or 
not?


I think it's hard to measure. Seems easiest to me to simple use a custom 
free for all allocated memory or for all strings.
And measuring if the cleared memory was really cleared is also not that 
trivial.


--
Gabriel



smime.p7s
Description: S/MIME Cryptographic Signature
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Re: libcurl leaks information in freed memory

2018-10-18 Thread Daniel Stenberg via curl-library

On Wed, 17 Oct 2018, Gabriel Zachmann wrote:


This, plus zeroing buffers is really hard.


I understand that this is not quite easy. However, while we won't find an 
optimal solution, I think we can do better. Zeroing memory might not succeed 
in all cases and there might be still some parts left on the stack, 
register, etc. But I think we can still memsetting most and it will be much 
harder to get sensitive information.


Any suggestion on how to do this and make sure the compiler doesn't remove the 
memset() ?


Also, are you suggesting we clear the memory for all frees? If yes, then we 
need to keep track of the sizes somehow and if no, then we need to figure out 
which ones and deal with the appropriately.


Can we come up with a way to measure this if we are doing this right or not?

--

 / daniel.haxx.se
---
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html