| Message ID | 6f29805d8bbde1af112d48201d5fde9a7776027f.1417951990.git.linux@horizon.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Herbert Xu |
| Headers | show |
By the way, this patch includes a bug due to a last minute "oh, I can make that more efficient!" which I realized after a night's sleep. (The v1 patch worked, FWIW.) Anyway, it's an RFC; I'm not even sure if I want this personally, but it's a bit of extra paranoia to always genreate fresh seed per read. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Dec 07, 2014 at 05:49:59PM -0500, George Spelvin wrote: > By the way, this patch includes a bug due to a last minute "oh, I can > make that more efficient!" which I realized after a night's sleep. > (The v1 patch worked, FWIW.) > > Anyway, it's an RFC; I'm not even sure if I want this personally, > but it's a bit of extra paranoia to always genreate fresh seed per > read. Wait, I'm confused. You mention in this note that this is an RFC patch, but not anywhere else in the series. Are you proposing this for inclusion or not? Neil > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Wait, I'm confused. You mention in this note that this is an RFC patch, but not > anywhere else in the series. Are you proposing this for inclusion or not? Er, in the 0/25, I mentioned that I put the least certain stuff last, and in particular I wasn't sure if the the last three patches were wanted or not: >> Pending issues: >> * Is non-deterministic mode (last three patches) wanted? I certainly wouldn't be unhappy if they went in, but with the comment clarification just before, I wouldn't be unhappy if they didn't, either. They're "If we wanted to do this, this is how it could be done. Is this something we want to do?" Sorry if my motivations are confusing. I did indeed start with wanting to add the seeding because I misunderstood the comments: I thought this was claiming to be X9.31 *and* I haven't seen the later versions of the standaed (which you have) that back off on the requirements for the DT[] vector. Since you've patiently explained both of those to me, I'm more interested in the other, more generic code cleanups. You also sent me two detailed explanations of the consequences of making the generator non-determinsitic in a way that gave me a general impression of disliking of the idea. So I've been weaning myself off the idea. I put those patches at the end so they can easily be dropped from the series. Or, as I also mentioned, simply postponed until there's been more discussion. Since that's an actual semantic change, collecting a few other opinions would be valuable. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 08, 2014 at 11:43:13AM -0500, George Spelvin wrote: > > Wait, I'm confused. You mention in this note that this is an RFC patch, but not > > anywhere else in the series. Are you proposing this for inclusion or not? > > Er, in the 0/25, I mentioned that I put the least certain stuff last, > and in particular I wasn't sure if the the last three patches were wanted > or not: > > >> Pending issues: > >> * Is non-deterministic mode (last three patches) wanted? > > I certainly wouldn't be unhappy if they went in, but with the comment > clarification just before, I wouldn't be unhappy if they didn't, either. > > They're "If we wanted to do this, this is how it could be done. Is this > something we want to do?" > > Sorry if my motivations are confusing. I did indeed start with wanting Not your motivations, just the posting mechanics. If you just want to discuss a patch, and aren't yet proposing it for inclusion, you should put RFC in the prefix of every patch header. > to add the seeding because I misunderstood the comments: I thought > this was claiming to be X9.31 *and* I haven't seen the later versions > of the standaed (which you have) that back off on the requirements for > the DT[] vector. > > Since you've patiently explained both of those to me, I'm more interested > in the other, more generic code cleanups. > > You also sent me two detailed explanations of the consequences of making > the generator non-determinsitic in a way that gave me a general impression > of disliking of the idea. So I've been weaning myself off the idea. > Not particularly opposed to the idea, I just know that several use cases rely on deterministic behavior for those entities that share the secret information, so I need to be sure that the deterministic behavior remains and is the default. > I put those patches at the end so they can easily be dropped from the series. > > Or, as I also mentioned, simply postponed until there's been more discussion. > Since that's an actual semantic change, collecting a few other opinions > would be valuable. I'll look at this series in detail shortly. Neil > -- > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Not your motivations, just the posting mechanics. If you just want to > discuss a patch, and aren't yet proposing it for inclusion, you should > put RFC in the prefix of every patch header. I understand the principle, and I should have on those patches (mea culpa), but really *all* patch postings are for comment; I think of RFC as "comment only; please don't apply this". But it wasn't marked RFC, so that's why I posted a note downgrading it when I realized I messed it up. The note was basically "oh, shit, I introduced a bug at the last minute; thankfully that was the most RFC of the entire series, so nobody's likely to have merged it." But it certainly is the case that for any significant patch series, I really don't expect v1 to get merged as-is. I'm serious about the changes, and it wouldn't have been a problem if you had applied v1, but it would have surprised me. Realistically, I expect a couple of rounds of discussion and tweaking of the specific form of the changes before people agree it's ready to go in. And I think that's the case here; I adjusted a lot of details based on feedback, but at a high level nothing changed; v2 makes the same changes that v1 did. > Not particularly opposed to the idea, I just know that several use cases > rely on deterministic behavior for those entities that share the secret > information, so I need to be sure that the deterministic behavior remains > and is the default. Right, because it's advertised as a PRNG. Thinking about it, would a separate crypto_alg with a different seedsize be a better solution than obscure rules about seed size? And something in the cra_flags to indicate it's nondeterminsitic? -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c index d39ce301..b88d3446 100644 --- a/crypto/ansi_cprng.c +++ b/crypto/ansi_cprng.c @@ -198,12 +198,14 @@ static int get_prng_bytes(u8 *buf, unsigned int nbytes, read_pos = ctx->rand_read_pos; if (byte_count > DEFAULT_BLK_SZ - read_pos) { - /* Leading partial block */ - unsigned int avail = DEFAULT_BLK_SZ - read_pos; + if (ctx->flags & PRNG_DETERMINISTIC) { + /* Return leftover data from previous call */ + unsigned int avail = DEFAULT_BLK_SZ - read_pos; - memcpy(ptr, ctx->rand_data.b + read_pos, avail); - ptr += avail; - byte_count -= avail; + memcpy(ptr, ctx->rand_data.b + read_pos, avail); + ptr += avail; + byte_count -= avail; + } read_pos = 0; /* Intermediate full blocks */
The standards documents are silent on how to handle multi-part output, so this is an RFC for something in the spirit of the source specifications, but not actually required by them. Signed-off-by: George Spelvin <linux@horizon.com> --- crypto/ansi_cprng.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)