Skip to content
This repository was archived by the owner on Mar 11, 2020. It is now read-only.

xen: support both entropy from dom0 and our weak fallback#8

Merged
pqwy merged 4 commits into
mirage:masterfrom
djs55:use-xentropyd
Feb 4, 2015
Merged

xen: support both entropy from dom0 and our weak fallback#8
pqwy merged 4 commits into
mirage:masterfrom
djs55:use-xentropyd

Conversation

@djs55
Copy link
Copy Markdown
Member

@djs55 djs55 commented Dec 21, 2014

Support 2 kinds of entropy source:

  • FromHost: we receive entropy over a console from domain 0 (or whichever other privileged domain has the entropy). This should be strong, but it requires an entropy server i.e. xentropyd
  • Weak: we use Random.self_init and get entropy from the clock. This is weak but will work anywhere.

In future we should support RDSEED #9 and possibly entropy from interrupt timing #10

Depends on [mirage/mirage#359]

Signed-off-by: David Scott dave.scott@citrix.com

Comment thread xen/entropy_xen.ml Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this need to fallback to weak if the xentropyd isnt listening?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I thought I'd better phrase is as an RFC-- it seems to me that
if you really care about generating good keys, you would rather fail/block
than fall back, otherwise if xentropyd falls over you might not notice the
problem until it's too late. However there are lots of cases where you
won't have a xentropyd (like AWS) where there's no real choice.

Perhaps the app writer should choose some kind of policy?

  • strict
  • dontcare
  • tryabitboforegivingup
    ?

On Sun, Dec 21, 2014 at 9:06 PM, Anil Madhavapeddy <notifications@github.com

wrote:

In xen/entropy_xen.ml
#8 (diff)
:

let connect () =

  • Random.self_init ();
  • print_endline "Entropy_xen_weak: using a weak entropy source seeded only from time.";
  • return (`Ok { handler = None })
  • Printf.printf "Entropy_xen: attempting to connect to Xen entropy source %s\n%!" console_name;
  • Console_xen.connect console_name

doesn't this need to fallback to weak if the xentropyd isnt listening?


Reply to this email directly or view it on GitHub
https://github.com/mirage/mirage-entropy/pull/8/files#r22150306.

Dave Scott

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the default 'weak' isn't that weak though -- we can implement a decent subset of what Amazon VMs already do to mix in entropy -- inter-ring packet times, etc.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for letting the app writer specify a policy, and make the default strict. baking in a weaker default for something that could be as critical as this seems a bad idea. cf what i understood were a number of the issues with the client api to openssl

@talex5
Copy link
Copy Markdown
Contributor

talex5 commented Jan 19, 2015

I get:

Entropy_xen: received [You may treat everything following this message as entropy.\r\n\XXX](1024 bytes) instead of expected handshake messagestate=Connected

(where XXX is random stuff I can't paste into GitHub comments)

@djs55
Copy link
Copy Markdown
Member Author

djs55 commented Jan 19, 2015

Hm, 2 possibilities spring to mind:

  1. I've not cached the 'connection' in connect: the handshake will only work once
  2. I've not guarded it with a mutex either; two parallel threads would interfere

@talex5
Copy link
Copy Markdown
Contributor

talex5 commented Jan 19, 2015

I don't really see how it's supposed to work. It calls Console_xen.read to read the handshake and then calls it again to read the entropy, but there's no reason it shouldn't get some entropy with the first response. Here's my hacky workaround:

https://github.com/talex5/mirage-entropy/compare/djs55:use-xentropyd...handshake?expand=1

@djs55
Copy link
Copy Markdown
Member Author

djs55 commented Jan 19, 2015

ha, yes there is that too :)

We now support 2 types of Xen entropy device:

  `FromHost: connect to an entropy server running over a console channel.
             This will require running xentropyd in dom0
  `Weak:     use Random.self_init which gets entropy only from time.
             This is not suitable for production use.

In future we will hopefully support gathering entropy from system timing
events and from hardware extensions like RDSEED.

Limitations:

- we read exactly one block of entropy data. Should we read as much
  as the backend will offer? Note the xentropyd implementation has a
  rate limiter to avoid starving dom0.

Signed-off-by: David Scott <dave.scott@citrix.com>
David Scott added 2 commits January 31, 2015 12:09
It's now a fatal error to fail to read the host entropy device, if one has
been configured.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
This allows us to read the exact number of bytes that we want, and
cache any extra we might receive (since the backend xentropyd might
be more generous than we need).

This also allows us to read the handshake message even if it is
returned in the same buffer as the first chunk of entropy.

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
The Unix version is unfunctorised and uses Lwt_engine. Unfortunately
we don't have Lwt_engine ("Lwt unix main loop engine") so we fall
back to refeeding unconditionally every <period> (600s).

Signed-off-by: David Scott <dave.scott@eu.citrix.com>
@djs55
Copy link
Copy Markdown
Member Author

djs55 commented Jan 31, 2015

OK, the buffered reader should fix the (embarrassing) problem with the handshake message.

@djs55 djs55 changed the title RFC: xen: read entropy from the Xen entropy device xen: support both entropy from dom0 and our weak fallback Jan 31, 2015
@djs55
Copy link
Copy Markdown
Member Author

djs55 commented Jan 31, 2015

I've tested the following via mirage-skeleton/entropy:

  • Unix still works
  • Xen with weak_entropy works as before
  • Xen with default_entropy depends on xentropyd
    • it prints a message and blocks forever if xentropyd isn't running
    • it connects and works if xentropyd is running

@pqwy
Copy link
Copy Markdown
Contributor

pqwy commented Feb 4, 2015

I'm not a fan of retaining a distinction between Weak and the other option, but I'll go ahead and merge this to get more testing. And it certainly is a big improvement over what we used to have!

Thanks!

@pqwy pqwy merged commit 981b070 into mirage:master Feb 4, 2015
@djs55
Copy link
Copy Markdown
Member Author

djs55 commented Feb 4, 2015

Thanks -- hopefully we can remove the Weak option altogether soon!

On Wed, Feb 4, 2015 at 1:49 AM, David Kaloper notifications@github.com
wrote:

Merged #8 #8.


Reply to this email directly or view it on GitHub
#8 (comment).

Dave Scott

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants