Re: Is ansi_cprng.c supposed to implement X9.17/X9.31's RNG?
Sorry for the duplicate; I had a crash and I thought the mail was lost. First message was not quite finished, second is a rewrite from scratch. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is ansi_cprng.c supposed to be an implmentation of X9.31?
On Fri, Nov 28, 2014 at 06:23:51PM -0500, George Spelvin wrote: I've been trying to understand the crypto layer, and it's a bit of a struggle because I'm trying to learn how it's supposed to work by reading the code, and I keep finding code I want to fix. Patches welcome. ansi_cprng.c is the current itch I'm eager to scratch. Other than enough implementation stupidities to make me scream (particularly the rand_data_valid variable name which is actually a Its actually a counter of the number of valid random data bytes in the buffer being returned to a caller, as well as an index into the internal buffer from which to draw fresh random data. Sorry if you don't get that, but it seems pretty clear. count of INvalid data, and keeping 5 blocks of state, including sensitive previous output, when only 3 are needed), one thing I can't help noticing Not sure where you're getting that from, only 1 block of random data is stored at any one time to return to a caller is that this is definitely NOT conformant with the X9.17/X9.31 spec. This is the document it was based of off: http://csrc.nist.gov/groups/STM/cavp/documents/rng/931rngext.pdf From my read, it seems to be in complete compliance. That's because the spec requires a timestamp for each output block to provide additional entropy, and a counter won't cut it. The document places no constrints on the value or progression of DT. As such a counter is as valid as any other implementation. You're welcome to enhance that however, as I said, patches welcome. I'm fixing the obvious things, but on this point, I have two choices: 1. Add some comments clarifying that the Based on part of the header is anything but a claim of compliance; those specs are for an RNG, while this is a PRNG. Please read more closely, the header clearly states this is a PRNG implementation, and a quick google search of the terms in the header bring up the document referenced above, with which this cprng is in compliance with. And probably delete all the FIPS stuff, as there's no spec to claim compliance with. Or Maybe do some research before making big claims like this: http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexc.pdf Its just a draft, but digging through the NIST site will bring up the approved version. Both show that a 3 DES CPRNG based on ANSI X9.31 is valid, and provides a reference to the paper above as the implementation guideline. 2. Fix the code to use random_get_entropy() and jiffies for the DT seed vector. Sure, knock yourself out. I don't consider it more or less valid to do so, but patches are welcome. In the latter case, I'd have to leave the current deterministic code as an option for self-testing, but I'd drop the recommended seedsize to DEFAULT_PRNG_KSZ + DEFAULT_BLK_SZ (one key and one IV), and have an internal flag indicating whether to use an incrementing DT vector or generate it fresh. Yup. Strictly speaking the API is in-kernel only, so you don't really have to worry about handling backwards compatibility, but if you don't allow for DT to be specified as an initial value, you won't be able to validate against any of the test vectors that NIST provides: http://csrc.nist.gov/groups/STM/cavp/documents/rng/RNGVS.pdf I will also point out that CPRNG's are designed such that peers communicating with a cprng are expected to be able to predect the cprng output, which implies that the DT value needs to remain predictable as well. Using an actual date time vector is going to make communication like that very unstable if there is even a little clock drift on either system. As such, while its less entropy, using a simple arbitrary vector with an increment on each random data generation leads to greater stability and predictability for those with the key. The data provided in the validation test in Appendix B.1.5 of the above document supports that, as the DT value is arbitrary and incremented by one on each iteration. If some code (like the current self-test code) provides an extra DEFAULT_BLK_SZ of seed material, it would go into determinsitic mode, but if it's missing, DT would be generated dynamically. Sure, patches welcome. But that's a question of design intent, and I can't intuit that from the code. Can someome enlighten me as to which option is preferred? Definately keep the ability to support external setting of DT, as you can't pass any validation tests without it. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is ansi_cprng.c supposed to implement X9.17/X9.31's RNG?
On Sat, Nov 29, 2014 at 12:26:49PM -0500, George Spelvin wrote: Sorry for the duplicate; I had a crash and I thought the mail was lost. First message was not quite finished, second is a rewrite from scratch. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Responded to your first note Neil -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is ansi_cprng.c supposed to be an implmentation of X9.31?
Other than enough implementation stupidities to make me scream (particularly the rand_data_valid variable name which is actually a Its actually a counter of the number of valid random data bytes in the buffer being returned to a caller, as well as an index into the internal buffer from which to draw fresh random data. Sorry if you don't get that, but it seems pretty clear. As you can see, I ended up choosing less abrasive wording in the version I *thought* was public; this got launched into the void while in draft form. Sorry about that. Oh, its use as an index into the read_data array is clear enough; it's just that the fact that the number of valid bytes in that array is DEFAULT_BLOCK_SZ - ctx-rand_data_valid seems obviously backward to me. You'd think ctx-rand_data_valid = 0 would mean no data is valid; force update cycle next access, but nope... is that this is definitely NOT conformant with the X9.17/X9.31 spec. This is the document it was based of off: http://csrc.nist.gov/groups/STM/cavp/documents/rng/931rngext.pdf Ah, I actually read that, but I didn't remember that the Based on wording is a direct quote. If you go to the original ANSI specs (which I've read in the past, but din't have a web-accessible copy to link to), they're a bit more explicit on the point. Please read more closely, the header clearly states this is a PRNG implementation, and a quick google search of the terms in the header bring up the document referenced above, with which this cprng is in compliance with. Yes, it was quite clear that a strict reading of the comments said that it was a PRNG, but I lost track of the fact that Random Number Generator Based on ANSI X9.31 Appendix A.2.4 was NIST's wording that was being quoted, and was left with the impression that compliance to the ANSI spec (which *does* call for a high resolution timestamp) was being implied. I either wanted to provide the implied compliance or clarify the absence of it. Sure, knock yourself out. I don't consider it more or less valid to do so, but patches are welcome. Coming right up! Definately keep the ability to support external setting of DT, as you can't pass any validation tests without it. Yes, I've already figured that out when studying the impact of such a change. Since there's already special-case handling of with/without a DT vector, that seemed the obvious thing to hook into. The two changes that affected callers, which I didn't feel very confident about my understanding of, were: 1. Changing the recommended seed size, and 2. Using actual random seed material. The other one, which I have to think *very* hard about, is whether using the same seed material as /dev/random in this much weaker cryptographic construction will introduce a flaw in *it*. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Is ansi_cprng.c supposed to be an implmentation of X9.31?
On Sat, Nov 29, 2014 at 02:32:05PM -0500, George Spelvin wrote: Other than enough implementation stupidities to make me scream (particularly the rand_data_valid variable name which is actually a Its actually a counter of the number of valid random data bytes in the buffer being returned to a caller, as well as an index into the internal buffer from which to draw fresh random data. Sorry if you don't get that, but it seems pretty clear. As you can see, I ended up choosing less abrasive wording in the version I *thought* was public; this got launched into the void while in draft form. Sorry about that. Oh, its use as an index into the read_data array is clear enough; it's just that the fact that the number of valid bytes in that array is DEFAULT_BLOCK_SZ - ctx-rand_data_valid seems obviously backward to me. You'd think ctx-rand_data_valid = 0 would mean no data is valid; force update cycle next access, but nope... is that this is definitely NOT conformant with the X9.17/X9.31 spec. This is the document it was based of off: http://csrc.nist.gov/groups/STM/cavp/documents/rng/931rngext.pdf Ah, I actually read that, but I didn't remember that the Based on wording is a direct quote. If you go to the original ANSI specs (which I've read in the past, but din't have a web-accessible copy to link to), they're a bit more explicit on the point. Please read more closely, the header clearly states this is a PRNG implementation, and a quick google search of the terms in the header bring up the document referenced above, with which this cprng is in compliance with. Yes, it was quite clear that a strict reading of the comments said that it was a PRNG, but I lost track of the fact that Random Number Generator Based on ANSI X9.31 Appendix A.2.4 was NIST's wording that was being quoted, and was left with the impression that compliance to the ANSI spec (which *does* call for a high resolution timestamp) was being implied. I either wanted to provide the implied compliance or clarify the absence of it. Sure, knock yourself out. I don't consider it more or less valid to do so, but patches are welcome. Coming right up! Definately keep the ability to support external setting of DT, as you can't pass any validation tests without it. Yes, I've already figured that out when studying the impact of such a change. Since there's already special-case handling of with/without a DT vector, that seemed the obvious thing to hook into. The two changes that affected callers, which I didn't feel very confident about my understanding of, were: 1. Changing the recommended seed size, and Well, you only need to worry about the users of the API in the kernel, for which I think there is only one currently, so I would say change the seed size if you want, and make sure the one caller matches it appropriately. Not saying I agree with the change (or disagree with it), but I think you don't really need to worry about it. 2. Using actual random seed material. Thats really a call for the allocator of a cprng to make. When you allocate a cprng instance, you have to reset it before you use it, so you have to provide a new IV, DT and KEY value anyway. If the caller wants to provide real random material, they are prefectly able to. If you want you can default to using random data when no seed is provided, but I wouldn't recommend it, since the random pool can block, and we might be resetting the cprng in a non-blocking context. I would say just leave this to the caller. The other one, which I have to think *very* hard about, is whether using the same seed material as /dev/random in this much weaker cryptographic construction will introduce a flaw in *it*. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
