create doesn't zero the struct

[talex5]

The structure returned by create isn't initialised. In a Mirage service, it might contain confidential data (e.g. TLS key material), which could easily be leaked.

Possibly this should be fixed in OCaml's Bigarray (and String.create), but at the very least, the documentation should mention this.

[avsm]

Heh, I was looking into this just now while typing a reply to your email. I think it's better to be safe rather than sorry and specifically zero out structures created via Cstruct.create (no guarantee can be made for of_bigarray, but this is generally used with pre-allocated pages or mmaped files).

[hannesm]

I assume this can be closed, now that we have memset!? Maybe the documentation for create should explicitly say that the buffer is not zeroed out?

[talex5]

The problem is that it isn't cleared by default. Perhaps we should make create zero it, and have a separate create_unsafe for people who want the current behaviour?

[hannesm]

I don't see the benefit of zeroing it out - I usually need to fill the byte vector with some data (which I control), maybe a ?fill keyword for create would be good?

If you're concerned about confidential data, I don't think the Cstruct.create should worry about it (since there may be any number of functions which expose the contents of raw memory, such as Bigarray/String.create). Instead, a way to properly erase key material (+derived data) is needed (this can only work with modifications to the runtime afaics). IMHO zeroing out the cstruct gives a false sense of security, the potential leak should be prevented at the origin, not at a later point when reusing the piece of memory.

[talex5]

It's not just key material that's confidential (though manually erasing that at free time might be a good idea anyway). Any information leak is a potential problem. It's also a reliability problem, since you tend to get zeroes when testing but other values after things have been running for a while.

In my opinion, all sources of uninitialised data should be marked unsafe. We should probably patch String.create to zero things out too, although it's unlikely we use it much in Mirage.

It would be good if there was a safe way to create and initialise a cstruct. e.g. in mirage-qubes I have a function:

let make_msg_header ~ty ~path ~data_len =
  let msg = Cstruct.create sizeof_msg_header in
  set_msg_header_ty msg (qdb_msg_to_int ty);
  set_fixed_string (get_msg_header_path msg) path;
  Cstruct.memset (get_msg_header_padding msg) 0;
  set_msg_header_data_len msg (Int32.of_int data_len);
  msg

Perhaps cstruct could auto-generate such functions?