WIP: Fix the RAND_add() reseeding issue

Introduction
The broken 'RAND_add()/RAND_bytes()' pattern
In the thread [openssl-dev] Plea for a new public OpenSSL RNG API I explained in detail that the different reseeding concepts of the classic OpenSSL RNG and the new RAND_DRBG API (pushing entropy via RAND_add() vs. pulling entropy via get_entropy() callback)  make it so difficult to get both APIs work together in harmony, at least as far as reseeding is concerned. The following is a brief excerpt from that thread:

In OpenSSL, the classical way for the RNG consumer to add his own randomness is to call RAND_add() before calling RAND_bytes(). If the new RAND_OpenSSL() method (the "compatibility layer" hiding the public RAND_DRBG instance)  is the default, then this does not work as expected anymore:
The reason is that a call to RAND_add() adds the provided randomness only to a global buffer (rand_bytes), from which it will be pulled during the next reseed. But no reseed is triggered. So the next RAND_bytes() call will be unaffected from the RAND_add(), which is not what the consumer expected. (The same holds for RAND_seed(), since drbg_seed() only calls into drbg_add())
Reseeding of DRBGs occurs only at the following occasions:

immediately after a fork() (new)
if the reseed_counter exceeds the reseed_interval
if RAND_DRBG_generate() is called requesting prediction_resistance
RAND_DRBG_reseed() is called explicitely

Note: Currently it looks like the situation is even worse: if RAND_add() is called multiple times before a reseed occurs, then the result of the previous call is overwritten.

In this pull request I propose a solution to this problem for general discussion.
Some background information on seeding the DRBG
NIST SP 800-90Ar1 has the concept of security_strength. Currently there are three different security strengths for the CTR_DRBG, specified by the NIDs NID_aes_128_ctr, NID_aes_192_ctr, NID_aes_256_ctr. Their parameter values are hardcoded (in ctr_init()) and taken from 'Table 3: Definitions for the CTR_DRBG' on page 49 of NIST SP 800-90Ar1. In particular, there is no need to have an extra RANDOMNESS_NEEDED constant.
The role of the derivation function for seeding
The internal state of the CTR_DRBG consists of two vectors
    unsigned char K[<keylen>];
    unsigned char V[<blocklen>];

so the total number of seed bytes (seedlen in Table 3) equals seedlen = keylen + blocklen. (As mentioned earlier: in the NIST document, everything is in bits, whereas in OpenSSL, entropy is in bits, buffer lengths are in bytes).
Seeding without derivation function
If no derivation function is used, the unmodified input is used to seed (K,V). This means, one has to provide exactly  seedlen=keylen+blocken bytes, which in the case of a high quality entropy source contains much more entropy than needed: 8 * seedlen > security_strength.
Seeding with derivation function
If a derivation function is used (which we always do), then the input can be of variable length between drbg->min_entropylen and drbg->max_entropylen. The DF takes a variable length input and produces a pseudorandom output from which (K,V) is seeded. This is done using AES-CTR. In some sense, the DF acts as a meat grinder, compressing the entropy of the input.
Using the derivation has two advantages:  1) min_entropylen = keylen < seedlen , so you need less input if your input has full entropy  and  2) The input can be of variable size  min_entropylen < inputsize < max_entropylen , which is good if your input has low entropy (say, only 2 bits per byte).
All values can be found in Table 3 and are hardcoded in ctr_init():
	int ctr_init(RAND_DRBG *drbg)
	{
		...

		ctr->keylen = keylen;
		drbg->strength = keylen * 8;
		drbg->seedlen = keylen + 16;

		if (drbg->flags & RAND_DRBG_FLAG_CTR_USE_DF) {
			/* df initialisation */
			static unsigned char df_key[32] = {
				0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,
				0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e,0x0f,
				0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17,
				0x18,0x19,0x1a,0x1b,0x1c,0x1d,0x1e,0x1f
			};
			/* Set key schedule for df_key */
			AES_set_encrypt_key(df_key, drbg->strength, &ctr->df_ks);

			drbg->min_entropylen = ctr->keylen;
			drbg->max_entropylen = DRBG_MAX_LENGTH;
			drbg->min_noncelen = drbg->min_entropylen / 2;
			drbg->max_noncelen = DRBG_MAX_LENGTH;
			drbg->max_perslen = DRBG_MAX_LENGTH;
			drbg->max_adinlen = DRBG_MAX_LENGTH;
		} else {
			drbg->min_entropylen = drbg->seedlen;
			drbg->max_entropylen = drbg->seedlen;
			/* Nonce not used */
			drbg->min_noncelen = 0;
			drbg->max_noncelen = 0;
			drbg->max_perslen = drbg->seedlen;
			drbg->max_adinlen = drbg->seedlen;
		}

		...
	}

The easiest way would be if get_entropy() would allocate a buffer of length max_entropylen and fill it with bytes until the entropy threshold has been reached. However, max_entropylen = DRBG_MAX_LENGTH is an astronomically high value. But this value is only a theoretical upper limit from the NIST document. I changed this members value to a more realistic value:
	drbg->max_entropylen = 32 * drbg_min_entropylen    /* similar for adinlen, etc. */

This corresponds to the very pessimistic estimate that the entropy of the input lies between 1/4 bits-per-byte and 8 bits-per-byte. That should be enough.
Now in the callback, the buffer is filled until the required entropy threshold is reached. If the buffer overflows (which should never happen), get_entropy() fails. To facilitate this, I added a new "class" RAND_POOL which acts as container for this variable sized random input.
This is my vision of how get_entropy() should be used. Even if get_entropy() pulls from /dev/urandom or other full-entropy sources, the implementers should have the freedom to return more than the requested entropy from alternative entropy sources and/or be allowed to return cumulate low entropy input.
Summary of the pull request
The solution is split in two parts:

Commit 1 fixes the entropy pushing part, i.e. drbg_add()
Commit 2 fixes the entropy pulling part, i.e. drbg_get_entropy(), and restores the original semantics of RAND_poll()

This splitting is only for the moment to keep the changes overseeable and facilitate reviewing. Also, to keep things simple as long this is still WIP, I will avoid force pushing and address comments by adding new commits. I will squash the commits with a proper commit message when this goes out of WIP.
Checklist

[x] documentation is added or updated

Commit 1: Make drbg_add() reseed the RAND_DRBG immediately
The obvious solution to use RAND_DRBG_reseed() creates a circular dependency, because now RAND_add() is involved both in pushing and pulling randomness:
RAND_add() -> drbg_add() -> RAND_DRBG_reseed() -> get_entropy()
                       -> get_entropy_from_system() -> RAND_poll_ex() -> RAND_add()

Using RAND_DRBG_generate() with additional input (generating and discarding some dummy output) instead of RAND_DRBG_reseed() does not solve the problem, because RAND_DRBG_generatE() can autoreseed occcasionally.
Instead, we add a new function rand_drbg_add() which is identical to RAND_DRBG_reseed(), except that it does not pull fresh entropy (and consequently does not reset the reseed_counter). This function is used only by the RAND_METHOD drbg_add().
Now the left hand side becomes:
RAND_add() -> drbg_add() -> rand_drbg_add()

The first commit breaks RAND_poll_ex(), but this function is rewritten and replaced by RAND_pool_fill() the second commit.
Commit 2: Cutting the Gordian knot: Decouple the reseeding code from the two RNG APIs
We do this by adding a new API for collecting entropy: The static fixed size RAND_BYTES_BUFFER is replaced by a more elaborate RAND_POOL "class" which acts as a container for collecting variable length randomness input for the derivation function.
The functions RAND_poll() and drbg_get_entropy() manage their own instances of such a RAND_POOL object, which they allocate and pass down the stack to the function RAND_POOL_fill() (formerly: RAND_poll_ex(). Since the lifetime and scope of these RAND_POOL objects is limited to the stack frame of RAND_poll() and drbg_get_entropy(), respectively, there is no need for locking.
Note also, that the RAND_POOL is completely decoupled from the RNG, the RAND_POOL_fill() callback has no notion which kind of RNG is reseeded from the data fed into the pool. Instead, the RAND_POOL class provides a rich API (@richsalz: no pun intended ;-) ) which appears a little oversized if one adheres to the principle that the operating system's RNG is the optimal entropy source with "full entropy" (8 bits per byte) and that it is not necessary to consider additional entropy sources or sources with lower entropy rates.
However, if one seeks for a flexible way of mixing in various entropy resources, as requested by multiple contributers, then such an API may come in handy. Currently, its just a blueprint and proof-of-concept. It provides a set of accessors providing information to the callbacks on which they could base there decisions on how much entropy should be added from the availabe resources.
I don't have a concept yet, how such a variable reseeding scheme could be managed and configured. But that's not part of this PR anyway. Input welcome, here or on openssl-dev.
Remarks
Randomness vs. Entropy
Many occurrances of the term "entropy" have been replaced by the more vague term "randomness" only recently. Although being mathematician, I have no problems using "entropy" synonymously to "randomness" in colloquial language. So originally I considered that discussion vain.
However, now this recent change change turns out to be really handy, because it enables me to consistently distinguish between a double randomness argument and a int entropy argument. The former is a floating point value measured in bytes, whereas the other is an integer value measured in bits and has a strict meaning in the sense of NIST SP800-90Ar1.
int RAND_POOL_add(RAND_POOL *pool, const void *buffer, int num, double randomness)
{
    size_t len  = (size_t)num;
    int entropy = (int)(randomness * 8.0);
    ...
}

TODO(DRBG)
Some places where I seek particular feedback or closer review have been marked with TODO(DRBG). These TODO's need to be resolved and removed before the PR can go out of WIP.
In particular, please have a closer look a the TODO(DRBG) comment preceding rand_drbg_add().
Reviewing and Testing
Since the RNG is a critical part of OpenSSL, any change to the code is like an open heart surgery. I'm not a seasoned committer and don't have the resources to test it on all platforms and under all circumstances. So I am grateful for any independent testing, in particular the DRBG chaining and forking stuff, since I a am only familiar with libcrypto and not such with libssl.
What I tested: I successfully ran the standard tests and did a little debugging with gdb to check the main code paths:
# global DRBG, fetching entropy from os for instantiation     (break at rbg_get_entropy)
./util/shlib_wrap.sh gdb --args ./apps/openssl rand -hex 32

# chained DRBG, fetching entropy from parent DRBGfor instantiation   (break at drbg_get_entropy, then add breakpoint inside 'if (drbg->parent)'-clause)
./util/shlib_wrap.sh gdb --args ./apps/openssl s_client -host www.openssl.org -port 443

# global DRBG, RAND_add()         (break at drbg_add)
echo 'This is a random number' > /tmp/randfile
./util/shlib_wrap.sh gdb --args ./apps/openssl rand -hex -rand /tmp/randfile  12

# test
./util/shlib_wrap.sh gdb --args ./test/drbgtest

Ok, the windows build fails because the polling code in rand_win.c was not updated. I will fix this and have a look at the travis builds, too.

Phew! That was a hard job to stop all jobs from complaining. I had a hard time struggling with the subtleties of the OpenSSL build system and with long forgotten differences between C++ and C. Now all CI jobs build, except for the travis job #13671.5, which fails with the message
make: *** No rule to make target `build_tests'.  Stop.

There is nothing much I can do about this error :-|

There is nothing much I can do about this error :-|

Yes - that is a problem unrelated to this PR. Are you able to submit a CLA though to make cla-check happy?

Incidentally - is this submission personal or on behalf of NCP? If the latter then we will need a corporate CLA too (we will need an Individual CLA either way). If the former, it would probably be better to submit from a personal email address if you have one.

A CLA for this company address has been signed long time ago. According to @richsalz, there seems to be a case sensitivity issue that prevents the bot from recognizing it.
A for the second question: This PR has been done entirely in my free time, so in that sense it could be considered personal. On the other hand, it is related to my work because we are currently using the FIPS DRBG with OpenSSL 1.0.2 and have an interest to continue using it after the migration to 1.1.
I will discuss this with my boss and let you know.

A CLA for this company address has been signed long time ago. According to @richsalz, there seems to be a case sensitivity issue that prevents the bot from recognizing it.

Right - thanks. I found the CLA. There was a case-sensitivity issue which I have corrected. However I could only find an Individual CLA for yourself, not a corporate CLA for ncp-e.com. We need both unless this is a purely personal contribution. Ideally if its purely personal then you should use a different (personal) email address in your commits so its always clear who owns the code that is being contributed. For now I've suspended the CLA in our database until we figure out who is doing the contribution!

I think this is good work, and am glad to see it getting thorough review feedback.  I'm a little busy right now; don't wait for me :)

Simplify RAND_POOL_add_begin()/RAND_POOL_add_end() calling sequence
Calling RAND_POOL_add_begin() with num == 0 is now considered a no-op, not an error anymore. As a consequence, some nested if statements like
#   ifdef OPENSSL_RAND_SEED_GETRANDOM
    bytes_needed = RAND_POOL_bytes_needed(pool, 8 /*entropy_per_byte*/);
    if (bytes_needed > 0) {
        buffer = RAND_POOL_add_begin(pool, bytes_needed);
        if (buffer != NULL) {
            int bytes = getrandom(buffer, bytes_needed, 0);
            entropy_available = RAND_POOL_add_end(pool, bytes, bytes);
        }
        if (entropy_available > 0)
            return entropy_available;
    }
#   endif

can be simplified to:
#   ifdef OPENSSL_RAND_SEED_GETRANDOM
    bytes_needed = RAND_POOL_bytes_needed(pool, 8 /*entropy_per_byte*/);
    buffer = RAND_POOL_add_begin(pool, bytes_needed);
    if (buffer != NULL) {
        int bytes = getrandom(buffer, bytes_needed, 0);
        entropy_available = RAND_POOL_add_end(pool, bytes, bytes);
    }
    if (entropy_available > 0)
        return entropy_available;
#   endif

That's it for the moment.
Three TODO's are remaining. If no one has any more comments, than I will clean up the code on friday and put it out of WIP. My company signalled that they are willing to sign a corporate cla, but it will take a few more days until I get it. Probably beginning of next week.
TODO 1
Looks like the implementation of rand_drbg_add() requires less discussion than I thought. If nobody objects, I will just remove the comment and stick to variant 1).
    /*
     * TODO(DRBG): looking at the source code of ctr_update() it
     * becomes obvious that |adin| needs to be passed as |in1| and
     * not  as |in2| parameter of ctr_update():
     *
     *       ctr_update(drbg, adin, adinlen, NULL, 0, NULL, 0);
     *
     * Otherwise the line
     *
     *            ctr_XOR(ctr, ctr->KX, drbg->seedlen);
     *
     * would not get executed. So we have three options
     *
     *  1) pass |adin| as |entropy| argument to ctr_reseed():
     *
     *        ctr_reseed(drbg, adin, adinlen, NULL, 0);
     *
     *  2) call ctr_generate() with |adin| to generate dummy output:
     *
     *        ctr_generate(drbg, dummy, sizeof(dummy), adin, adinlen)
     *
     *  3) make ctr_update() non-static and call it directly
     *
     *        ctr_update(drbg, adin, adinlen, NULL, 0, NULL, 0);
     *
     * Variants 1) and 2) both invoke 3), but 2) is more heavyweigth
     * Variant 3) would be the simplest solution, but the function
     * is currently static. So I chose variant 1), even though
     * using |adin| for |entropy| looks like a hack.
     *
     * Any comments? Suggestions?
     */

TODO 2
I leave this TODO in the code, passing on the torch to others. Anybody with ideas for a good concept may feel free to come up with a PR.
/*
 * Try the various seeding methods in turn, exit when successful.
 *
 * TODO(DRBG): If more than one entropy source is available, is it
 * preferrable to stop as soon as enough entropy has been collected
 * (as favored by @rsalz) or should one rather be defensive and add
 * more entropy than requested and/or from different sources?
 * If yes, how can this be made configurable?
 */

TODO 3
This looks like a simple task for a seasoned OpenSSL contributor, so I'll leave it up to somebody else to post a followup PR.
/*
 * Default security strength (in the sense of [NIST SP 800-90Ar1])
 * of the default OpenSSL DRBG, and the corresponding NID.
 *
 * Currently supported values: 128, 192, 256
 *
 * TODO(DRBG): would be nice to have the strength configurable
 */
# define RAND_DRBG_STRENGTH             128
# define RAND_DRBG_NID                  NID_aes_128_ctr

So I have some comments, but that doesn't mean anything needs to change (yet):

RAND_add() will now be used as additional data for the global public DRBG. It's not used for the private global DRBG. We might need an new API that can say to which one it should go to, but I think we should deprecate those push type APIs, so no need to add an other one.
RAND_add() used to add entropy, now it's only additional data. When OpenSSL doesn't know how to get entropy it was possible to work around this by calling RAND_add(). But with the default changed to the DRBG it will now be impossible instantiate it, and it might stop working for someone who just upgrades from 1.1.0 to 1.1.1. But since we want to have a pull API for the entropy (and additional data), this push API isn't going to work anyway. We might need to do something like fall back to the old RAND method in that case.

Can we use "randomness" not "entropy" in the public documentation?

RAND_add() used to add entropy, now it's only additional data. When OpenSSL doesn't know how to get entropy it was possible to work around this by calling RAND_add(). But with the default changed to the DRBG it will now be impossible instantiate it, and it might stop working for someone who just upgrades from 1.1.0 to 1.1.1. But since we want to have a pull API for the entropy (and additional data), this push API isn't going to work anyway.

You're right, the user currently has no way to add his own entropy. The ultimate solution is to make the RAND_DRBG API public, than he can provide his own get_entropy callback.
int RAND_DRBG_set_callbacks(RAND_DRBG *dctx,
                            RAND_DRBG_get_entropy_fn get_entropy,
                            RAND_DRBG_cleanup_entropy_fn cleanup_entropy,
                            RAND_DRBG_get_nonce_fn get_nonce,
                            RAND_DRBG_cleanup_nonce_fn cleanup_nonce);

However this is a little bit overkill for the standard use case: The user just wants to provide an extra entropy source to be used by the standard get_entropy callback.
Proposal: What do you think about having an additional simplified interface for this special use case, which is based on the RAND_POOL API
typedef size_t (*RAND_DRBG_acquire_entropy_cb)(RAND_POOL *pool);

int RAND_DRBG_add_entropy_source(RAND_DRBG *dctx,
                                 RAND_DRBG_acquire_entropy_cb acquire_entropy);

This could be part of the followup PR for redesigning the entropy gathering: In addition to querying the standard os sources, RAND_POOL_acquire_entropy() could also poll the entropy sources provided added by the user. This could be done before or after polling the standard entropy sources. So there are too many unanswered questions to treat this in this PR.

We might need to do something like fall back to the old RAND method in that case.

What do mean by that? Falling back to RAND_SSLeay()?!

Can we use "randomness" not "entropy" in the public documentation?

The term entropy is a technical term from NIST SP 800-90Ar1 and as such an essential part of the RAND_DRBG interface. As I documented at the beginning of this PR in the section "Randomness vs. Entropy", I consistently use the term randomness for the double parameter  counting the proportion of randomness per byte (as in RAND_add()), and the term entropy for the integer value in the strict sense of NIST SP 800-90Ar1.
int entropy = (int)(randomness * 8.0);

What do mean by that? Falling back to RAND_SSLeay()?!

That doesn't seem to exist (anymore?), but I think the answer is yes. Falling back to a method that doesn't use the DRBG and can work with RAND_add(). It's the only way I see not to break applications in that special case.

Yes, it was removed.

That doesn't seem to exist (anymore?), but I think the answer is yes. Falling back to a method that doesn't use the DRBG and can work with RAND_add(). It's the only way I see not to break applications in that special case.

I think, RAND_SSLeay() should R.I.P and shouldn't be exhumed. There must be a better solution. One possible solution would be that drbg_add triggers a full reseed if 8 * randomness > security_strength
static int drbg_add(const void *buf, int num, double randomness)
{

}

But this is somewhat hacky and means deviating from the NIST standard.

With "triggering a full reseed" I mean simulating a reseed of the DRBG without pulling entropy from the trusted sources, but using the contents of |buf| as entropy input instead,

I really don't want to bring back "entropy" into the documentation or API.
I also don't understand why RAND_add as-is is not sufficient, or why you can't use it as "additional data" and make a generate call.  People who are sophisticated enough to understand the difference and actually require it, might have to modify the source.  I'm okay with that.

I really don't want to bring back "entropy" into the documentation or API.

After rereading the [openssl-dev] DRBG entropy thread, in particular the responses by John Denker and Paul Dale, I can understand your reluctance to use the term "entropy". My problem is that this term is omnipresent in NIST SP 800-90Ar1 and in the RAND_DRBG implementation.
The new RAND_POOL 'class' is a specialized helper class intended primarily for the get_entropy() callback. So it does not make sense to change parameter names in the middle of the callstack. I don't object to the term 'randomness', but I object to inconsistent naming. Changing 'entropy' to 'randomness' makes only sense if you do it for the entire RAND_DRBG API, too. (And I'm looking forward to you draft for NIST SP 800-90Ar2, good luck ;-) ).
So I would ask you to accept the term 'entropy' for the moment, where it is used in the context of the NIST terminology. There is no hurry, since RAND_DRBG and RAND_POOL are not public yet. I don't mind if you do a consistent renaming of both the RAND_DRBG and the RAND_POOL APIs before they become public (hopefully).

I also don't understand why RAND_add as-is is not sufficient, or why you can't use it as "additional data" and make a generate call. People who are sophisticated enough to understand the difference and actually require it, might have to modify the source. I'm okay with that.

RAND_add() is perfect as it is, if you accept the fact that the 'user' can only use it to enhance the randomness of an already instantiated DRBG. If get_entropy() fails (which won't happen in the idealistic scenario where the OS RNG is a 'perfect' source of randomness) then the DRBG remains uninstantiated and the user has no way to fix this. @kroeckx just pointed out that this is a change from previous behaviour:

RAND_add() used to add entropy, now it's only additional data. When OpenSSL doesn't know how to get entropy it was possible to work around this by calling RAND_add(). But with the default changed to the DRBG it will now be impossible instantiate it,

My opinion: The RAND_add() implementation should not be changed just to obtain compatibility with legacy APIs. If you try to be compatible under all circumstances, you end up with an API mess like in Windows ;-)

Perhaps the way to fix what @kroeckx found is to move the RAND_DRBG_instantiate call out of the drbg_setup function and into the drbg_bytes function.

You are probably thinking of a temporary failure of get_entropy(). In this case a just-in-time reinstantiation might be useful to recover from the error. (But things are not so simple, see section Error Recovery below).
I think that @kroeckx was thinking about a systematic failure of the standard entropy source (for whatever reason). In that case the user currently has no way to bypass the standard instantiation process and provide his own entropy for instantiating the DRBG. For this, he would need to call the non-public RAND_DRBG_set_callbacks() (or the simplified RAND_DRBG_add_entropy_source(), which I proposed above) to register a callback which provides his random input for instantiation.
The NIST standard does not provide a way for pushing entropy into the DRBG for instantiation. So one just has to accept that RAND_add() does not work the way it used to (and make the other API public).

Error Recovery after temporary failure of the entropy source
The NIST standard distinguishes between two types of error: a (normal) error and a catastrophic error.

The Get_entropy_input call could return a status of ERROR_FLAG to indicate that entropy is currently unavailable, and could return CATASTROPHIC_ERROR_FLAG to indicate that an entropy source failed.

A catastrophic error can only be healed by a complete uninstantiation followed by another instantiation, see Section 11.4:

When a DRBG fails a health test or a catastrophic error is detected during normal operation, the DRBG shall enter an error state and output an error indicator. The DRBG shall not perform any instantiate, generate or reseed operations while in the error state, and pseudorandom bits shall not be output when an error state exists. When in an error state, user intervention (e.g., power 65NIST SP 800-90A Rev. 1 Recommendation for Random Number Generation Using Deterministic RBGs cycling of the DRBG) shall be required to exit the error state, and the DRBG shall be re-instantiated before the DRBG can be used to produce pseudorandom bits.
errors include:

The DRBG_STATUS has only one error state DRBG_ERROR. However, looking at the source (drbg_lib.c) one sees that currently the state transition diagram of DRBG_ERROR coincides with the one for CATASTROPHIC_ERROR_FLAG:

if get_entropy() fails, enter state DRBG_ERROR
RAND_DRBG_instantiate() fails if state != DRBG_UNINITIALISED, in particular if state == DRBG_ERROR.
If RAND_DRBG_uninstantiate() is called, enter state DRBG_UNITIALIZED.

So the only way to recover from a temporary failure of the entropy source is a RAND_DRBG_uninstantiate()/instantiate().
A simple change turns a failure of get_entropy() into a temporary error

if get_entropy() fails, enter state DRBG_UNINITIALIZED

RAND_DRBG_generate(): if state == DRBG_UNINITIALIZED, call RAND_DRBG_generate() (and check for success)

I can prepare an extra PR for the error recovery, because it should not be squashed with all the other changes here.

The entropy_from_system function pulls in any data sent by RAND_add, so I think my suggestion works.  Where am I wrong?

It doesn't anymore with this PR. RAND_add() is only used for additional data.
My concern is that there are applications that will really stop working. For instance applications that chroot, don't have access to /dev/urandom anymore. When we need to reseed they'll stop working.
I didn't think about it yet, but since we now have a reseed requirement, if it's run on a kernel that doesn't provide a system call to get entropy and the chroot doesn't provide /dev/urandom they will have a problem after some random time that we go in error state.
I understood that at least postfix makes sure that RAND_add() gets called. But then it might not call it with enough data.
I think we should at least try to document this, probably in NEWS.

But if instead of the 'mix it as additional data' we do what I suggested, would that solve the problem you identified?

Rich, I think it is indeed possible to feed the input from RAND_add() into the get_entropy() callback without using a global static buffer. My idea is very similar to how you did it, but works also for low quality random input. This method will work reliably as long as RAND_add() adds sufficient randomness (8 * randomness >= security_strength). Please give me one or two days to work out the details. I hope this will provide the final reconciliation between the 'push' and the 'pull' philosophy.

That would be great news.  I have every confidence in you :)

It's good you can't see me blush... ;-)

Added two more commits, details see commit log.  This time I have a good feeling that I managed to reconcile the two worlds. Please have a first look at it. If I get your thumbs up, I will cleanup and squash the commit and put it out of WIP for final review.
@richsalz: Sorry for throwing away too much when removing the static fixed size random bytes buffer. It turned out that in principle your approach was the right way to go.  ;-)

Ok, I made a newbie error: I shouldn't have pressed this damn "resolve conflicts" button in GitHub just to get rid of these few conflicting lines in libcrypto.num.  I didn't expect this would create a merge commit, which makes squashing the PR impossible :-|.  GUI magic is evil.
I will rewind the last two commits before rebasing and squashing.

Rebased and squashed. Ready for final review.

Travis and AppVeyor seem to hate me :-|

The Travis build logs are full of strange errors. Are these really related to my changes?!?
The first AppVeyor job was killed after hanging for an hour.

@FdaSilvaYY thanks for the review. All unanswered comments will be fixed as requested.

I took me a while to figure out how to debug the failing test, but now I hope I resolved the problem.

@richsalz, there is currently a special treatment (in rand_linux.c only) for the case where OPENSSL_RAND_SEED_NONE is defined. What is the intended behaviour? If this version of  RAND_POOL_acquire_entropy() is called and returns 0 then the instantiation will always fail. Did I miss something?
# if (defined(OPENSSL_SYS_VXWORKS) || defined(OPENSSL_SYS_UEFI)) && \
        !defined(OPENSSL_RAND_SEED_NONE)
#  error "UEFI and VXWorks only support seeding NONE"
# endif

...

size_t RAND_POOL_acquire_entropy(RAND_POOL * pool)
{
#  ifdef OPENSSL_RAND_SEED_NONE
    return 0;
#  else
    ...
}

The intent was that randomness would be provided by something outside openssl.  Maybe that's no longer possible.

Fine-tune just-in-time instantiation (e.g., for UEFI and VXWorks)
In the case where OPENSSL_RAND_SEED_NONE is defined, it should be possible to initialize the DRBG by adding sufficient entropy using RAND_add(), even if the initial instantiation fails.
To achieve this, I replaced the
#  ifdef OPENSSL_RAND_SEED_NONE
    return 0;
	...

by
#  ifdef OPENSSL_RAND_SEED_NONE
    return RAND_POOL_entropy_available(pool);
	...

To simulate the case where the original instantation fails but is healed by an RAND_add() call, I used the following setup:
perl Configure debug-linux-x86_64 --strict-warnings --with-rand-seed=none
make
echo "this is some random input" > /tmp/randfile
./util/shlib_wrap.sh gdb --args ./apps/openssl rand -hex -rand /tmp/randfile  12

Here I noticed that just-in-time instantiation was not implemented correctly yet:

RAND_DRBG_get0_global() returned a NULL pointer, because setup_drbg() failed. So I changed the behaviour that an instantiation error is not considered a failure anymore by setup_drbg(). (RAND_DRBG_generate() now has just-in-time reinstantiation.)

The reinstantiation in rand_drbg_restart() was buggy: It is not enough to call RAND_DRBG_instantiate(). The entire sequence from drbg_setup() needs to be repeated.

The summary of all fixes is contained in the commit
Fine-tune just-in-time instantiation of the DRBG   (to be squashed)

Note: In rand_unix.c, I moved the nested # if in front of the outer #if, because the two conditions contradict:
#if !(defined(OPENSSL_SYS_WINDOWS) || defined(OPENSSL_SYS_WIN32) || defined(OPENSSL_SYS_VMS) || defined(OPENSSL_SYS_VXWORKS) || defined(OPENSSL_SYS_UEFI))

# if (defined(OPENSSL_SYS_VXWORKS) || defined(OPENSSL_SYS_UEFI)) && \
        !defined(OPENSSL_RAND_SEED_NONE)
#  error "UEFI and VXWorks only support seeding NONE"
# endif

I haven't checked whether in the UEFI and VXWorks case OpenSSL does the seeding with RAND_add() itself, or whether this is left to the user. Please delegate this question to the responsible expert for testing.

I don't understand why none is only allowed for UEFI and VXWorks.  It could be useful for any external system that wants to provide its own seeding; SGX for example.

No, it's the other way around:  UEFI and VXWorks only allow OPENSSL_RAND_SEED_NONE.

Help appreciated
Uh, It's really hard to get all the tests right. I would really appreciate some expert help on fixing the tests.
The travis log https://api.travis-ci.org/jobs/275192127/log.txt?deansi=true says that the evp_extra_test failed. According to the log, all 5 testcases succeed, but then a leak check fails.
../test/recipes/30-test_evp_extra.t ................
1..1
    # Subtest: ./evp_extra_test
    1..5
    ok 1 - test_EVP_DigestSignInit
    ok 2 - test_EVP_DigestVerifyInit
        # Subtest: test_d2i_AutoPrivateKey
        1..3
        ok 1 - iteration 1
        ok 2 - iteration 2
        ok 3 - iteration 3
    ok 3 - test_d2i_AutoPrivateKey
    ok 4 - test_EVP_PKCS82PKEY
        # Subtest: test_EVP_PKEY_check
        1..3
        ok 1 - iteration 1
        ok 2 - iteration 2
        ok 3 - iteration 3
    ok 5 - test_EVP_PKEY_check
    # [20:07:39]     1 file=crypto/stack/stack.c, line=108, thread=47367447074816, number=40, address=0x2060740
    # [20:07:39]     2 file=crypto/stack/stack.c, line=110, thread=47367447074816, number=32, address=0x20607e0
    # 72 bytes leaked in 2 chunks
../util/shlib_wrap.sh ./evp_extra_test => 1
not ok 1 - running evp_extra_test

When I run this on test on my machine with valgrind using
./util/shlib_wrap.sh valgrind --leak-check=full --show-leak-kinds=all ./test/evp_extra_test

it doesn't look like the leaks have anything to do with my changes. So what am I doing wrong here ?!
==22538== Memcheck, a memory error detector
==22538== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==22538== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==22538== Command: ./test/evp_extra_test
==22538==
1..5
ok 1 - test_EVP_DigestSignInit
ok 2 - test_EVP_DigestVerifyInit
    # Subtest: test_d2i_AutoPrivateKey
    1..3
    ok 1 - iteration 1
    ok 2 - iteration 2
    ok 3 - iteration 3
ok 3 - test_d2i_AutoPrivateKey
ok 4 - test_EVP_PKCS82PKEY
    # Subtest: test_EVP_PKEY_check
    1..3
    ok 1 - iteration 1
    ok 2 - iteration 2
    ok 3 - iteration 3
ok 5 - test_EVP_PKEY_check
==22538==
==22538== HEAP SUMMARY:
==22538==     in use at exit: 168 bytes in 3 blocks
==22538==   total heap usage: 3,511 allocs, 3,508 frees, 171,291 bytes allocated
==22538==
==22538== 32 bytes in 1 blocks are still reachable in loss record 1 of 3
==22538==    at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22538==    by 0x4FD2781: CRYPTO_malloc (mem.c:192)
==22538==    by 0x4FD27B4: CRYPTO_zalloc (mem.c:200)
==22538==    by 0x503F362: OPENSSL_sk_new (stack.c:110)
==22538==    by 0x4FC8AF4: sk_EVP_PKEY_METHOD_new (evp_int.h:81)
==22538==    by 0x4FC937A: EVP_PKEY_meth_add0 (pmeth_lib.c:289)
==22538==    by 0x10AE02: setup_tests (evp_extra_test.c:522)
==22538==    by 0x10B9FD: main (main.c:50)
==22538==
==22538== 40 bytes in 1 blocks are still reachable in loss record 2 of 3
==22538==    at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22538==    by 0x4FD2781: CRYPTO_malloc (mem.c:192)
==22538==    by 0x4FD27B4: CRYPTO_zalloc (mem.c:200)
==22538==    by 0x503F341: OPENSSL_sk_new (stack.c:108)
==22538==    by 0x4FC8AF4: sk_EVP_PKEY_METHOD_new (evp_int.h:81)
==22538==    by 0x4FC937A: EVP_PKEY_meth_add0 (pmeth_lib.c:289)
==22538==    by 0x10AE02: setup_tests (evp_extra_test.c:522)
==22538==    by 0x10B9FD: main (main.c:50)
==22538==
==22538== 96 bytes in 1 blocks are still reachable in loss record 3 of 3
==22538==    at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22538==    by 0x4FD2781: CRYPTO_malloc (mem.c:192)
==22538==    by 0x4FD27B4: CRYPTO_zalloc (mem.c:200)
==22538==    by 0x4EE720E: BIO_meth_new (bio_meth.c:38)
==22538==    by 0x1108E2: BIO_f_tap (tap_bio.c:28)
==22538==    by 0x10EC16: test_open_streams (basic_output.c:24)
==22538==    by 0x10B9BA: main (main.c:38)
==22538==
==22538== LEAK SUMMARY:
==22538==    definitely lost: 0 bytes in 0 blocks
==22538==    indirectly lost: 0 bytes in 0 blocks
==22538==      possibly lost: 0 bytes in 0 blocks
==22538==    still reachable: 168 bytes in 3 blocks
==22538==         suppressed: 0 bytes in 0 blocks
==22538==
==22538== For counts of detected and suppressed errors, rerun with: -v
==22538== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

wait and rebase.  ping @InfoHunter to look at the memleak :)

valgrind shows the same leaks when I revert to HEAD~, i.e.
commit 48e0f0fcb340e1b79b756f9c565521fb7e41e88d
Author: Dr. Matthias St. Pierre<EMAIL_ADDRESS>Date:   Thu Aug 31 23:16:22 2017 +0200

    Fix reseeding issues introduced by new RAND_OpenSSL() method

So why did the same test succeed last time?

So why did the same test succeed last time?

What the CI infrastructure is building is a temporary merge commit that merges the pull request into the current state of the pull request target.  As a new commit landed on master, that is now included in the CI build, and was not included in the previous CI build.

So if I understand it right, I am having the same issue as @InfoHunter in #4337, which is intended to be fixed in #4356?

That seems likely, though I did not actually look at the valgrind output you posted (or the output that you'd get from using the in-tree enable-crypto-mdebug stuff)

I rebuilt with enable-crypto-mdebug, now I'm getting exactly the same error. The problem was introduced on my branch by rebasing. So I will just wait until the problem is fixed.

I don't understand why none is only allowed for UEFI and VXWorks. It could be useful for any external system that wants to provide its own seeding; SGX for example.

I am still convinced that using RAND_add() is only a temporary workaround for custom seeding. In the long run, there should be a way to register additional entropy sources, which could then be queried by RAND_POOL_acquire_entropy() .

"I'm still unconvinced" -- okay, that's fine with me.

So if I understand it right, I am having the same issue as @InfoHunter in #4337, which is intended to be fixed in #4356?

Yes, this problem is introduced by #4337 and is being fixed in #4356.  I will work on #4356 soon.

@mspncp #4356 is merged and the memleaks should be fixed now : )

Thanks @InfoHunter! Just rebased and squashed my pr.

Just sent ccla and icla to<EMAIL_ADDRESS>

One remaining style issue : some lined are currently exceeding the 80-cols limit. ;)

I didn't have time to go over this yet, but it seems to me that VxWorks, UEFI and SEED_NONE all either need to call RAND_add() before the DRBG gets instantiated, or they need to set up callbacks (which I guess isn't possible yet).

I didn't have time to go over this yet, but it seems to me that VxWorks, UEFI and SEED_NONE all either need to call RAND_add() before the DRBG gets instantiated, or they need to set up callbacks (which I guess isn't possible yet).

That's no problem anymore, because I implemented automatic error recovery from temporary failures of the entropy source: A seeding error during first instantiation is not considered a fatal error by drbg_setup() anymore. The DRBG simply remains in the error state. This will be noticed in RAND_DRBG_generate(), which automatically calls rand_drbg_restart(). The second instantiation will succeed if RAND_add() adds sufficient randomness.

However, one problem remains: If RAND_add()is not called regularly, then at some time automatic reseeding will be triggered and this will fail again.

And there is another problem remaining: There is no API for adding randomness to the private DRBG. There are some possible solutions, e.g.:

Add a new RAND_add_priv() function to seed the private DRBG.
Make the private DRBG the master and chain the public DRBG to the private DRBG. Use RAND_add() to reseed the (private) master and immediately reseed the public DRBG from the master.
Add a dedicated master DRBG, to which the private and public DRBG are chained. (It's the only one which is seeded by polling the OS entropy sources.) Use RAND_add() to reseed the master and immediately reseed the private and public DRBG from the master.

My favourite would be number 3,  but that's beyound the scope of this PR.

Just make sure to state that requirement in the RAND_add documentation.

I also like choice three and saying you'll do it in a separate PR is fine.

... you'll do it in a separate PR is fine.
Nice trick. Well, I'll think about it ;-)

Just make sure to state that requirement in the RAND_add documentation.

I noticed that this is not the only thing that needs be updated in the RAND_add() manual page after my last changes. I'm on it, but I am currently busy with other things, too. For the moment, I unchecked the "documentation is added or updated" box.

Added some notes on the peculiarities of the new DRBG to the RAND_add() manpage. Please bear with me if I violated any style conventions for writing manual pages, it's my first major change to a manual page. Any corrections welcome.

I like the third choice best.

All requested changes are addressed. Rebased and squashed, ready for final review.

@levitte, is the manual page ok for you now?

I'm happy with the layout of the docs.
However, I'm out travelling and am not in a position to look at the rest of the PR in a meaningful way before next week.

 So enjoy your stay, no hurry.

@paulidale thanks for the approval and the 'like', I appreciate it :-)
I rebased and squashed the commit without changes. Shortly, I will create a followup PR based on this commit where I add the master DRBG as discussed above.

Rebased to get bugfix #4395.

Ok, it compiled on linux, so let's give it a try.

All comments by @kaduk and @FdaSilvaYY addressed. Ready for final review.

I planned on reviewing this

@kroeckx no hurry, I'm still working on @kaduk's change requests :-)

Great, thanks.  I will defer to you, @kroeckx

With commit 3bbce72425c80120a17c0828873d7ab4fbdb9f37 all remaining change requests by @kaduk should be addressed now.

I'll review again one these nits are fixed and everything is squashed.

I'll review again one these nits are fixed and everything is squashed.

In keeping with #3963 perhaps it is time to leave squashing to the committer (so that the submitter does not do squashing and the fixup commits are easy to review separately).

That's fine too.  There shouldn't be many commits to squash.

Subject to a rebase ...

The branch  currently has a conflict. @kroeckx could please let me know if you prefer that I rebase (without squashing) before you start reviewing, or not?

Added one commit to address @kaduk's nits.. Since @kroeckx didn't object, I rebased the branch without squashing.

Yes our style says blank line after declarations; that's immutable.
I think it's a good idea to clump related lines together in a "paragraph" if you will, such as assign and then check results.  YMMV.

Since I only changed empty lines, I assume that the failure of the Travis job is unrelated. Does anyone understand what the problem is? https://travis-ci.org/openssl/openssl/jobs/281839898

The test errors are hidden behind the install(docs?) log:
Test Summary Report
-------------------
../../_srcdist/test/recipes/70-test_sslrecords.t             (Wstat: 13 Tests: 3 Failed: 0)
  Non-zero wait status: 13
  Parse errors: Bad plan.  You planned 18 tests but ran 3.

but I'm not seeing much that would have changed that could be responsible for that.
Anyway, assuming that we can decipher the travis issue, are we just waiting for @kroeckx review here, everyone else is happy?

It would be appropriate to add [extended tests] to the last commit to get more checks on these changes.
BTW: it is normal/useful to find that tag in the master commits ?

[extended tests] label added to last commit message as requested by @FdaSilvaYY. No changes to the commit itself.

The travis job https://travis-ci.org/openssl/openssl/jobs/282356530 fails in 05-test_rand, because the MemorySanitizier reports an error (see excerpt below). I would say this is caused by the fact that the input for RAND_add() was malloc'ed and not initialized. Maybe the extended tests haven't been run since the new DRBG has been added?
static int test_rand_add(void)
{
    char *p;

    if (!TEST_ptr(p = malloc(RAND_ADD_SIZE)))
        return 0;
    RAND_add(p, RAND_ADD_SIZE, RAND_ADD_SIZE);
    free(p);
    return 1;
}

Anyway, that error is not my business, and I don't have the capacity to reproduce all these errors which only occur with specific compilers and configurations.  Also the fact that the coverage has decreased by 0.009% does not really bother me.
I fixed the reseeding of the DRBG which was previously broken and it is due time that this pull request finally gets merged into master, because it is blocking other work, like pull request #4402 and issue #4394. This pull request had five reviewers and is around for about a month now. It's time to get it off the shelf. I am looking forward to the approval by @kroeckx and then this pull request should be finalized.
Please somebody of the testing experts step forward and fix the tests, as a different PR.
Excerpt from the test log
../test/recipes/05-test_rand.t .....................
1..1
    # Subtest: ./drbgtest
    1..3
        # Subtest: test_kats
        1..6
        ok 1 - iteration 1
        ok 2 - iteration 2
        ok 3 - iteration 3
        ok 4 - iteration 4
        ok 5 - iteration 5
        ok 6 - iteration 6
    ok 1 - test_kats
        # Subtest: test_error_checks
        1..6
        ok 1 - iteration 1
        ok 2 - iteration 2
        ok 3 - iteration 3
        ok 4 - iteration 4
        ok 5 - iteration 5
        ok 6 - iteration 6
    ok 2 - test_error_checks
==19441==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7fcc3d3d62bb in AES_encrypt /home/travis/build/openssl/openssl/crypto/aes/aes_core.c:887:13
    #1 0x7fcc3d649277 in ctr_BCC_block /home/travis/build/openssl/openssl/crypto/rand/drbg_rand.c:75:5
    #2 0x7fcc3d649277 in ctr_BCC_blocks /home/travis/build/openssl/openssl/crypto/rand/drbg_rand.c:84
    #3 0x7fcc3d649277 in ctr_BCC_update /home/travis/build/openssl/openssl/crypto/rand/drbg_rand.c:123
    #4 0x7fcc3d649277 in ctr_df /home/travis/build/openssl/openssl/crypto/rand/drbg_rand.c:179
    #5 0x7fcc3d649277 in ctr_update /home/travis/build/openssl/openssl/crypto/rand/drbg_rand.c:227
    #6 0x7fcc3d64d8b0 in ctr_reseed /home/travis/build/openssl/openssl/crypto/rand/drbg_rand.c:262:5
    #7 0x7fcc3d64548c in RAND_DRBG_reseed /home/travis/build/openssl/openssl/crypto/rand/drbg_lib.c:225:10
    #8 0x7fcc3d645b87 in rand_drbg_restart /home/travis/build/openssl/openssl/crypto/rand/drbg_lib.c:323:13
    #9 0x7fcc3d647100 in drbg_add /home/travis/build/openssl/openssl/crypto/rand/drbg_lib.c:601:11
    #10 0x48b36f in test_rand_add /home/travis/build/openssl/openssl/test/drbgtest.c:486:5
    #11 0x48c70b in run_tests /home/travis/build/openssl/openssl/test/testutil/driver.c:201:19
    #12 0x48d00d in main /home/travis/build/openssl/openssl/test/testutil/main.c:51:15
    #13 0x7fcc3c473f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #14 0x41b11e in _start (/home/travis/build/openssl/openssl/test/drbgtest+0x41b11e)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/travis/build/openssl/openssl/crypto/aes/aes_core.c:887:13 in AES_encrypt
Exiting
../util/shlib_wrap.sh ./drbgtest => 77
not ok 1

With the last fix, all extended tests, except for the coverage test succeeded.

Time for reapprovals?

I've been following along (and thinking things are fine), but I think we're waiting to hear an actual +1 from Kurt.

Okay, no problem waiting for Kurt's approval.

@kroeckx thanks for reviewing. I'm currently busy, so I want be able to address your comments before this evening (MESZ) or even Saturday. Matthias

One day I'll get to the end of the patch. Hopefully that will be soon. :)

One day I'll get to the end of the patch. Hopefully that will be soon. :)

I hope, too. This pull request is pushing the GitHub interface to its limits. It's already taking ages to load this page on my machine...

This thread is getting very long :)  @kroeckx are you okay with things?
I am going to close #4239

This pull request currently contains three commits. Before merging, please rebase as follows  (or allow me to do it myself):
pick cd0671b751 Fix reseeding issues introduced by new RAND_OpenSSL() method
fixup e0bce4f26e Address @kaduk's comments  (to be squashed)
fixup 76122c83cd Address @kaduk's nits  (to be squashed)
fixup cc0419840b Removed some empty lines before NULL pointer checks  (amended; to be squashed)
fixup d83af76246 Address @kroeckx's comments  (to be squashed)
fixup 5115d6770c Initialize randomness buffer in test_rand_add()   (to be squashed)
fixup 5c28bb82f2 Address more comments from @kroeckx  (to be squashed)
fixup c2934eb355 Address even more comments from @kroeckx  (to be squashed)
fixup 0ef87bfa72 Avoid overflow in RAND_POOL_add_*()    (to be squashed)
pick f84e88771f Remove unnecessary DRBG_RESEED state
pick 7158f2e096 Add missing RAND_DRBG locking

There is no reason why, as the code author, you cannot do the merge/squash yourself :)

Ok, but I'll wait until Kurt gives his approval.

P.S: I was just trying to follow procedural request #3963, as suggested by Ben  ;-)

@kaduk, please have an extra look at your patch, which I adopted in 7158f2e09665cd7e112bd9f7695f5c1bf666d574.

This file util/libcrypto.num is a merging nightmare!

Rebased to resolve conflicts.

ping @kroeckx

Now please, please get this pull request approved and merged finally. It wears me down that this is going on for one and a half months already, and the fact that this pull quest does not get merged blocks other work based on it.

And please let me do the final squashing before you merge it.

Squash as you want; @kroeckx approved and will just need a reconfirmation which should be simple.  Or I can confirm just that part.

Rebased and squashed. Note that the subject of the first commit was changed slightly from

Fix reseeding issues introduced by the new RAND_OpenSSL() method

to

Fix reseeding issues of the public RAND_DRBG

Note: The unsquashed branch was saved at https://github.com/mspncp/openssl/tree/pr-fix-rand-add-unsquashed

Looks good for me

Please give me a few hours to look it over again and reconfirm.

Okay, so we formally have enough review to merge (but should probably wait a bit more, a day at most, in case others want to chime in).
@paulidale @richsalz do you want to reconfirm?
@levitte had looked at a very early version but I don't think he indicated a desire to hold us up for additional review.

I was most concerned that @kroeckx have time to give feedback.

The two nits mentioned by @kaduk and the TODO(DRBG) belong to the first commit (Fix reseeding issues...) and should be fixed there.
For the convenience of the final committer (@levitte ?), I added two more commits: One fixes the two nits of @kaduk and the other removes the TODO(DRBG) in case the VMS check is successful. Both commits are intended to fixup the first commit:
pick 0ee1a83ae4 Fix reseeding issues of the public RAND_DRBG
fixup bd1f9bec47 Fix two nits  [fixup 0ee1a83]
fixup 2eba501b1c Remove TODO(DRBG) from rand_vms.c [fixup 0ee1a83 or discard]
pick 7101ebca43 Remove unnecessary DRBG_RESEED state
pick 7a9d8c848c Add missing RAND_DRBG locking

As a heads-up/last call, I intend to merge this in about 13 hours, unless objections or other volunteers to merge appear before then.

This travis job fails to boot:  https://travis-ci.org/openssl/openssl/jobs/289370926
Certainly not related to my last change ;-)

Pushed, at long last.
@mspncp thank you for adding those last few fixup commits and saving me some work.
Also thank you again for doing this, and your patience through the exhaustive review process.
Maybe we can move on to #4402 now...

Thanks!
Thanks in return to all reviewers for the great effort you put in reviewing this PR. You gave me a hard time and kept me busy for quite a while, but your reviews also give me a good feeling about the final result. The random generator is such a crucial part of the crypto library and requires very much care. And I really don't want to be the guy who is blamed sometimes in the future for having introduced some RANDBLEED bug. ;-)
What's next
Now that pull request #4328 has finally been merged, I have regained new drive to follow my long-term goal which is to make the RAND_DRBG public. I added some thoughts about my private roadmap to Issue #4403. You are welcome to participate and comment.

Because I think it's so important to get right, I'm taking my time to review and understand everything.

To all early reviewers of this pr: I would like to recover commit 7a6a26a0284199f1d0d8494cf6a46c9bd36968dc, which I lost in my local repo a while ago by garbage collection. It is still displayed here on GitHub, together with the entire branch, but github doesn't let me fetch it, because it is not reachable from one of the advertised references.
$ git fetch github 7a6a26a0284199f1d0d8494cf6a46c9bd36968dc
error: Server does not allow request for unadvertised object 7a6a26a0284199f1d0d8494cf6a46c9bd36968dc

(I know that I can download the patches and recreate the branch, but I'd rather have the original commits)
If one of you checked it out for reviewing (or even if you only have a fetch rule for refs/pull/*/head), it is likely that you still have it lying around in your local copy. If yes, could you please be so kind and try to recover it and push it to your github clone for me?
git checkout -b for-msp  a6a26a0284199f1d0d8494cf6a46c9bd36968dc
git push github for-msp

(If you named your github clone differently, you have to replace github accordingly.)
If you succeed, please send me the link. Thank you in advance!

To all early reviewers of this pr: I would like to recover commit 7a6a26a

Get it from this branch:
https://github.com/mattcaswell/openssl/tree/decouple-reseed-code

Cool! Thank you very much, Matt! You can delete the branch in your repository again, I have pushed it to my repository as pr-fix-rand-add-ori, together with the snapshot of the final version pr-fix-rand-add.


Full screen support for dashboard

Dashboard could be present on full screen mode. The dashboard would be perfect to show on larger screen (i.e. tv)

Isn't full screen mode a browser feature? What's not working?

It would be a nice feature if you could hide the navigation and maybe hide or shrink the dashboard title. So I can use the dashboard for a big screen in the office.

You could add some CSS to the board that removes the top navigantion bar, i.e.,
.top
{
    display : none;
}

I suppose the other changes can be done this way as well.

This used to work by adding a get parameter ?standalone=true in the URL, fix here:
https://github.com/apache/incubator-superset/pull/3284


How do I get a published stream server URL?

I want to embed the published stream into my own application using the published stream URL. My stream was initially in RTSP format and I want to stream using WebRTC, is it possible to get a URL such as http://localhost/stream_name/stream1?

the video endpoints are documented here: https://github.com/deepch/RTSPtoWeb/blob/master/docs/api.md#webrtc  an example is also available: https://github.com/deepch/RTSPtoWeb/tree/master/docs/examples/webrtc


E2E - Add to cart from pdp

We must be able to perform an add to cart from pdp, navigate to /checkout and this product to be present on this route

done at https://github.com/vtex-sites/base.store/pull/13


export_as_linked_data with schema argument

curently there is some discussion on the flavours of linked data (e.g. schema.org, codemeta)
https://github.com/citation-file-format/cff-converter-python/pull/62
We could instead have a method export_as_linked_data(flavor='schema.org')
with other flavors as we see fit.

decided to go a different route, with separate exporters for e.g. codemeta and schema.org.
closing the issue.


Turbsim run 5MW_Baseline 90m_12mps_twr.inp

Hi Jonkman,
I downloaded the Turbsim_v2.00.07a-bjj.exe in NREL as you said #581,file 90m_12mps_twr.inp in running 5MW_BASELINE encountered the following problem:
5MW_Baseline\Wind>TurbSim_x64.exe 90m_12mps_twr.inp
Running TurbSim (v2.00.07a-bjj, 14-Jun-2016).
Reading the input file "90m_12mps_twr.inp".
ReadInputFile:Invalid logical input for file "90m_12mps_twr.inp" occurred while trying to read
Echo.
Aborting TurbSim.
When I use the lower version of TurbsimV150, the following problems appear:
Reading the input file "90m_12mps_twr.inp".
RanLux luxury level set by RLuxGo :    3 p =          223
RanLux initialized by RLuxGo from Seeds       13428           0           0
A default value will be used for the power law exponent.
A default value will be used for the roughness length.
You have requested that the following file(s) be generated:
90m_12mps_twr.bts (an AeroDyn/TurbSim full-field wind file)
RanLux luxury level set by RLuxGo :    3 p =          223
RanLux initialized by RLuxGo from Seeds       13428           0           0
Calculating the spectral and transfer function matrices:
u-component matrices
What should I do, please?
Best regards,
Roby Tian

Sorry, I have solved this problem, because it was used for the first time, it was just slow to calculate, so I thought the calculation was wrong.


Also cache _build for Elixir

Ping?

Ping?

Ping?

Pinging as well.
I noticed the example in @mhanberg's https://www.mitchellhanberg.com/ci-cd-with-phoenix-github-actions-and-gigalixir/ uses separate caches for deps and _build, and the _build cache key includes the Elixir and Erlang version. Would that make sense in this example?

It should not be required to include the versions in the cache key since Elixir should recompile when it detects the Elixir version has changed. It does not recompile when the OTP version changes but that is rarely necessary.

@jclem Sorry to bother you – you came to my aid once before when an "Actions" PR got stuck in limbo. Could we get this one considered?

Sorry all, I don't maintain this repo and didn't get notified (it has an owner, just not me).
Is this actually an effective way of caching _build? My understanding is that this will only rebuild the cache when mix.lock changes, and so you'll end up with the same cache for all builds regardless of what others files change.
I'd considered adding this before, myself, but couldn't figure out the right way of doing it.
I'm unfortunately on PTO until a week from this Monday, but I've set a reminder to revisit this when I'm back. I'm sorry for the delay, and thanks for bringing this to my attention.

Is this actually an effective way of caching _build? My understanding is that this will only rebuild the cache when mix.lock changes, and so you'll end up with the same cache for all builds regardless of what other files change.

It will use the cache for dependencies but mix will detect the changes in the project itself and rebuild it accordingly.

Oh yeah, sorry—my point wasn't that the caching itself would be problematic, just that the cache wouldn't be helpful because we're only using the mix.lock hash as the key (so changing application code wouldn't get cached over time unless mix.lock happens to change).
That said, the benefit that that does add, either way, is that you get cached built dependencies, which for a lot of projects is probably the majority of build time in CI, anyway. Given that, 👍 from me!

Thank you so much, @jclem! Really appreciate it.

It will use the cache for dependencies but mix will detect the changes in the project itself and rebuild it accordingly.

out of curiosity, and sorry if it's not the best place to ask such questions, if we have a build matrix (e.g. simply different OSes but you can imagine different Elixir/OTP pairs too) we'd get better cache utilisation with separate one for deps and build, right?
- uses: actions/cache@v2
   with:
     path: deps
     key: mix-deps-${{ hashFiles('**/mix.lock')) }}
     restore-keys: mix-deps-
- uses: actions/cache@v2
   with:
     path: _build
     key: mix-build-${{ runner.os }}

What we have merged in is definitely a better starting point for most users of course.

out of curiosity, and sorry if it's not the best place to ask such questions, if we have a build matrix (e.g. simply different OSes but you can imagine different Elixir/OTP pairs too) we'd get better cache utilisation with separate one for deps and build, right?

Yes. deps/ does not require the OS/OTP/Elixir version in its key but you still want to include mix.lock in the _build/ cache key.


FIX: Overflow content on configuration

This adds overflow-x: auto to prevent wide content from pushing out the secondary sidebar
closes https://github.com/pydata/pydata-sphinx-theme/issues/771

Is there a page in our demo documentation that was showing the same issue as in seaborn one ? From my computer everything looks the same.

wooo amazing, OK I will mark this as closing your issue too then


Javascript error when creating a new criterion in the rubrics controller

It says that sortable doesn't exists.

Fixed

This is a duplicate of: https://github.com/MarkUsProject/Markus/issues/607
Creating a new criterion and drag/dropping was both caused by this js error.


+cexplorer.io (+testnet)

Hello @nahern
logo: https://cexplorer.io/assets/images/cexlogo.png?q
official profile: https://twitter.com/cexplorer_io + managing partner https://twitter.com/0xb_yosef
simply, it's the future successor to adapools.org on a new architecture ( https://cexplorer.io/article/welcome-to-new-adapools-org-cexplorer-io ) .
Thanks!


export 'Item' (imported as 'Item') was not found in '@mui/material'

Duplicates

[X] I have searched the existing issues

Latest version

[X] I have tested the latest version

Steps to reproduce 🕹
Link to live example:
Steps:

Try importing 'Item'  in "@mui/material": "^5.10.13",

Current behavior 😯
Compiling with error in react app
Expected behavior 🤔
No error
Context 🔦
No response
Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

There is no component named Item in Material-UI.

https://mui.com/material-ui/react-grid/

Item component is still in documentation.

https://mui.com/material-ui/react-grid/
Item component is still in documentation.

It is a custom component:
const Item = styled(Paper)(({ theme }) => ({
  backgroundColor: theme.palette.mode === 'dark' ? '#1A2027' : '#fff',
  ...theme.typography.body2,
  padding: theme.spacing(1),
  textAlign: 'center',
  color: theme.palette.text.secondary,
}));


Fix ??

?? doesn't work on unitys objects

:tada: This PR is included in version 3.7.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


[IOS] Freeze and Crash app

Bug
I run on my app and see an error :(
Step 1: Reload the app by terminal (Press R key on the terminal)
Step 2: App State : Foreground -> background -> Foreground
Step 3: App Freeze and crash
Platform: IOS
OS:
"react-native": "0.68.2",
"react-native-video": "^5.2.1",
Some Informations

Error on the Sentry : Rendered fewer hooks than expected. This may be caused by an accidental early return statement
I checked my code and use method of Hook on the top function and without if/else.

I have same issue. Please let me know if you find a solution.

I have same issue. Please let me know if you find a solution.

Is this problem can re-produce on a released version as well?
@thaiphamvan @MinhThu100200  @memberwebm

This will help https://www.youtube.com/watch?v=vVI7ZAZq5e0

any logs ? any reproduction on V6 version ?

android.app.ForegroundServiceStartNotAllowedException: startForegroundService() not allowed due to mAllowStartForeground false
Env:
Android 12,
"react-native-video": "5.0.2",

any update?

Issue on 5.2, no feedback for now. let's close this issue


#10-First bit of formatting

Go ahead and add this, but keep the ticket open…I’ll add more later.

Thanks Rick!


Provide a helpful error message if devPath is not specified

Describe the bug
Provide a helpful error message if devPath is not specified
see #129 for a solution - but we would prefer to do the detection in tauri-config.js

Maybe it is better to leave in the runner, as the Vue CLI Plugin sets the devPath after getting the config from tauri-config.js.

@nklayman was this fixed in #599?

Yep, it was. Thanks for noticing!


Some cookies are misusing the recommended “sameSite“ attribute

how to handle this console error?

Browsers now prefer for this to be specified rather for them to set it by default (which they'll do anyway).
I was trying to answer this one out myself. So I'll put this here for anyone else looking for the answer.
Before calling:
new CookiesEuBanner...
You put:
CookiesEuBanner.prototype.setCookie = function(name, value) {
  var date = new Date();
  date.setTime(date.getTime() + this.cookieTimeout);

  document.cookie = name + '=' + value + ';expires=' + date.toGMTString() + ';path=/' + ';secure;SameSite=Lax';
}

Tested it and it seems to work.

Yes this is indeed going to be an issue in the near future as browsers impose more strict control on cookies.
Firefox flags this issue with the following warning:
Cookie “hasConsent” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute.
To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite

Yes this is indeed going to be an issue in the near future as browsers impose more strict control on cookies.
Firefox flags this issue with the following warning:
Cookie “hasConsent” will be soon rejected because it has the “SameSite” attribute set to “None” or an invalid value, without the “secure” attribute.
To know more about the “SameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite


"verification time" label shows up despite NOLABEL, and on regular time series plots

Reported by andrew wittenberg on 10 Apr 2015 00:01 UTC
The label "verification time", at the bottom-left corner of this plot, doesn't belong there.  Somehow Ferret must think this is a forecast dataset.  Is this tied to the abstract or normal axis of A?
Also, the label shows up even though we've used /NOLABEL.
 	NOAA/PMEL TMAP
 	FERRET v6.941 (PyFerret 1.2)
 	Linux 2.6.32-504.8.1.el6.x86_64 - 04/07/15
 	 9-Apr-15 19:49

yes? use coads_climatology; let a = 1; plot/y=0/x=140w a*sst

For real forecast applications, I'd suggest not auto-labeling the Verification Time and Initialization Time in this way; why not just center such labels along the axis, as has always been done with "longitude", "latitude", "depth", etc.?
/NOLABEL needs to work too, so the user can unlabel or relabel as they fit.
Migrated-From: http://dunkel.pmel.noaa.gov/trac/ferret/ticket/2265

Comment by ansley.b.manke on 10 Apr 2015 00:39 UTC
The Verification Time and Initialization Time labels should not be shown unless it's a FMRC dataset, so just on coads_climatology we should never see that.  (Steve, this all applies to Ferret from the trunk, not just PyFerret.)
When it is actually a forecast/time set, these labels are in the same class as the month labels or the axis-tic labels, and so they don't automatically go away.  But all of this is new and open for discussion.

Comment by steven.c.hankin on 13 Apr 2015 16:20 UTC
Andrew,
I'm onto the /NOLABEL glitch.  My bad.  A negelected detail as a side-effect of the situation I'll explain next.
re: the placement of the labels at the lower left corner.  Automatic placement of annotations is a surprisingly bigger pain in the back side than one might expect.  The labels under discussion are added primarily to annotate the situation of two calendar time axes (verification and initialization) on the same plot.  You'll notice that when either of the times is in the form of a "lag" (e.g. forecast lead time), then the normal labeling is used on that axis. It turns out that calendar time axes present a special layout complication, because at times they require two lines of annotations.  For example, compare
plot T[T[t=1-jan-1900:1-jan-1950:30](t=1-jan-1900:1-jan-1910:30]

to
plot)

In the dual-line horizontal calendar mode, there isn't room for the normal axis label without overwriting the plot title, or re-jiggering the vertical axis origin to make room.  In fact, even if an axis label is defined, the underlying PLOT+ package ignores it if it is in calendar mode.  Which is the back story on why we resorted to the somewhat unusual lower left corner annotations.  All-in-all, the effort needed to get the seemingly obvious annotation style to work right in dual-time-label situations, didn't seem to pass the cost/benefit test.  And there would be some side effects to adjusting the vertical origin to make room for an extra label, too.
If you see this layout issue as high priority we can revisit it.

Comment by andrew.wittenberg on 13 Apr 2015 16:40 UTC
Thanks Steve.  As long as the labels don't show up on non-FMRC datasets (where they didn't before), and we can suppress the labels on FMRC datasets with /NOLABEL and/or PPL AXIS, then I'd be happy.
If those are hard to do, then I'd suggest just removing the labels for now and letting the user add their own as they see fit.

Comment by ansley.b.manke on 13 Apr 2015 17:01 UTC
Another example, from the benchmark script err63_days1900toydmhms.jnl
The script defines a 2-D variable with dimensions Z-T (non-calendar time).  The listing has an extra line with the label  "Verification time increases down the page"
yes? define axis/t=21241:21245:1 tax
yes? let julshift = DAYS1900(1950,1,1)
yes? let julday   = t[gt=tax] + julshift
yes? let tpts     = DAYS1900TOYMDHMS(julday)

yes? sh func DAYS1900TOYMDHMS
DAYS1900TOYMDHMS(day1900)
    Convert Julian day to values y,m,d,h,m,s on k=1,...,6
    day1900: Julian day counted from 1-jan-1900 (none)

yes? list/k=2:3 tpts
             VARIABLE : DAYS1900TOYMDHMS(JULDAY)
             SUBSET   : 2 by 5 points (Z-T)
             Verification time increases down page.
                 2      3
                 2      3
 21241   / 1:   2.00  27.00
 21242   / 2:   2.00  28.00
 21243   / 3:   2.00  29.00
 21244   / 4:   3.00   1.00
 21245   / 5:   3.00   2.00

Comment by steven.c.hankin on 13 Apr 2015 18:14 UTC
The /NOLABEL bug has been fixed
The lower left labels intended to disambiguate FMRC axes that have been showing up in unrelated situations have been fixed.
Regarding the use of those lower left labels I have restricted their usage only to the cases where they are the only reliable clue to knowing whether one is looking at the T or F axis.  For the most commonly used case in which one of the axes is a lag (and therefore labeled) and the other is a calendar, there will be no lower left labels.  But for the diagonal SHADE plots, where there are two calendar axes, and for line plots which involve the T or F calendar axis of an FMRC dataset there will be labels.
If this sounds like an acceptable compromise we can close this ticket.

Comment by andrew.wittenberg on 13 Apr 2015 18:27 UTC
Thanks!  I'm not clear though -- can /NOLABEL now be used to remove the labels in any/all of these contexts?

Comment by steven.c.hankin on 13 Apr 2015 18:52 UTC
Yes.  /NOLABEL now suppresses the new FMRC labels at the lower left (i.e. consistent with how /NOLABELS suppresses other plot labels).
These changes will become available together with the enhancements to support DEFINE DATA/AGGREG/F, which is underway.
I am closing this ticket, on the assumption that we've resolved all of its issues.  Re-open as necessary, of course.


Be more verbose when checkout was not done

As discussed.
An exception should be thrown when no checkout took place at RemoteInstallStrategy.php#L61
Suggested exception
throw new Exception(sprintf('No task configured that checked out release version "%s".', $version) );

Thanks.


Unmatched < meant as Literal < Breaks Highlighting When inline HTML is Present

For example:
|        Thing 1       |      Description      |
|----------------------|-----------------------|
| HTML | First some html.

One

Twp

 |
| `<` | Don't do this, everything goes to hell |
| Other line | RED MODE ACTIVATED |
| `'` | No more red |

As it appears in the editor:

Editable version
Note that the < cannot be replaced with < as that displays as a literal < and it's the single quotes in the text description that gets the red treatment, which will continue until either another ' is found (anywhere) or the end of the file is reached. Nor can the ' or '< be escaped (e.g. \' or \<).
This makes it particularly frustrating to edit the markdown page for a custom esolang where < one command and I require a bulleted list in the description for another command (as it does multiple things, based on its inputs) and the syntax highlighting in this case makes the text under it nearly impossible to read.

so what would you recommend to add in the codemirror files to implement the changes? I created the disable warning button but would like to know the functionality so I can attempt this issue.

Hey, I noticed you closed your PR so I pushed a minor fix here. I'd be happy to review any further changes you have in mind for this.

The live version hasn't updated yet (probably github's caching) but injecting the change at least lets me read the text in those regions.

So, that's fine for now.

Yeah, I haven't deployed the changes yet. It should be live soon.
On Sat, Oct 26, 2019, 7:57 PM Draco18s<EMAIL_ADDRESS>wrote:

The live version hasn't updated yet (probably github's caching) but
injecting the change at least lets me read the text in those regions.
[image: image]
https://user-images.githubusercontent.com/1750680/67621086-cdd51f00-f7d2-11e9-979d-d2df87c4bb56.png
So, that's fine for now.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/jbt/markdown-editor/issues/101?email_source=notifications&email_token=AB5JHVWCHSSE3MMZULLXBEDQQRHVFA5CNFSM4GP3ZIDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECKJFQY#issuecomment-546607811,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB5JHVTR4BLCZ2TJW5D7XILQQRHVFANCNFSM4GP3ZIDA
.


ATX v2 (for ATX merge)

Motivation
Proposal of the new ATX version to support the atx merging.
Description :construction:
New ATX V2 allows identities to marry (join an equivocation set) and publish ATX together (merge).
:wedding: Marriage
Marriage happens when an identity selected by the miner publishes an ATX with the Marriages slice filled with marriage certificates.
A certificate proves the will of the identity that signed it to be married to the identity that publishes the certificate. For example, if identity Alice wants to marry Bob (and vice versa), the owner of these identities first picks one that will publish an ATX with a marriage certificate. Say it's Bob. Alice signs Bob's ID and creates a MarriageCertificate, which Bob includes in his next ATX.
The marriage is instant :zap: and, from the moment Bob published the ATX, Alice and Bob are forever bound by forming a so-called equivocation set.
:family: equivocation set
An equivocation set bounds all identities it contains. If one of them acts maliciously, all IDs in the set become malicious.
Merging
A merge is an act of publishing a single ATX on behalf of many identities. A merged ATX contains non-interactive proofs of space-time (NIPoST) for multiple identities. It is signed by a single ID that publishes it and must include a VRF nonce (that belongs to the ID signing the ATX) that satisfies the combined storage of all included IDs. It becomes active for the target epoch and will participate in the network protocols (i.e. Hare) and receive accumulated rewards on the selected coinbase account according to the weight of all IDs present in the ATX.
:bulb: A merged ATX contains a reference to the ATX in which marriage happened for easy syntactical verification if the included IDs are married.
Restrictions
:one: One merged ATX per epoch
There can be only one merged ATX per equivocation set. Identities that choose not to merge (for example because of software or hardware malfunction, being late with PoST proving, etc.) must publish separate ATXs just for themself. They cannot form a second merged ATX in the same epoch.
For example, given an equivocation set of IDs A,B,C,D,E. If C and E malfunction, the node:

can publish ATXs [A,B,D] + [C] + [E]
cannot publish ATXs [A,B,D] + [C,E]

:two: There is an upper limit of equivocation set size
Only up to N identities can marry int a single equivocation set. Marrying more is malfeasance and cancels all IDs already present in the set as well as the ones that tried to join it.
:three: The equivocation set cannot grow
Identities that are already married cannot marry again.

bors merge


file:/// type resource path problem

It can't correct load the resource when  the path has special character(like space  ,+  and chinese character) by use   file:/// type path.
example:

the image can't correct load.

@kefengwei please note that use of file protocol is discouraged generally.
https://www.magpcss.org/ceforum/viewtopic.php?f=6&t=17015
https://github.com/chromelyapps/Chromely/blob/master/Documents/loading_html.md#file-protocol
But with a custom resource handler - sample shared with you in #246 you may be able to get around it.
I am curious too, so may try it over the weekend.
Thanks.

@mattkol  Thanks for your response. The local::// protocol has the same problem. I will try the new sample.

@mattkol I try the new sample. it's working when the path used by the StartUrl. But It's not working in file:/// type or my-schema type when I run this in blazor server mode.
This is Demo project(These are changes in the Test.razor and Program.cs):

ServerAppDemo.zip

@kefengwei the custom resource handling process is not meant for file protocol. That was the reason for pointing out that it is "generally discouraged". It is not officially supported either, but I have seen some developers use it successfully.
I have also never used it differently besides the start url.
This is what I will do:

Then in the handler map ""/d/projects/hello world/header.jpg" to "/d:/projects/hello world/header.jpg" and read from actual file in the directory - https://github.com/chromelyapps/Chromely/blob/7f8ba7ee7c350825855a5d58f05c347b348123a5/src/Chromely/Browser/Handlers/DefaultResourceSchemeHandler.cs#L33

This is such a common requirement for a desktop application... I wonder why there is not already some handler for this situation , or at least provide some code to easily paste, for non cef experts...
Now i'm going to investigate and implement a custom resource scheme handler, sigh

@cederron did you succeed with it? i also need the resouces to be editable (filesystem based) and not packed away in dlls.

@simonbuehler attached is a demo of one approach.
I will find time to look at it closely to make it a lot simpler.
CrossPlatDemo.zip
    public class DemoApp : ChromelyBasicApp
    {
        public override void ConfigureServices(IServiceCollection services)
        {
            base.ConfigureServices(services);

            services.AddSingleton(typeof(IChromelySchemeHandler), typeof(CustomSchemeHandler));

            RegisterControllerAssembly(services, typeof(DemoApp).Assembly);
        }
    }

    public class CustomSchemeHandler : IChromelySchemeHandler
    {
        public CustomSchemeHandler()
        {
            Name = "MyCustomResourceSchemeHamdler";
            // Scheme: htpp
            // Host: img- placeholder for folder name/or network path containing resource files
            Scheme = new UrlScheme("mycustomscheme", "http", "img", string.Empty, UrlSchemeType.Resource, false);
            HandlerFactory = new CustomResourceSchemeHandlerFactory(new DefaultErrorHandler());
            IsCorsEnabled = true;
            IsSecure = false;
        }

        public string Name { get; set; }
        public UrlScheme Scheme { get; set; }
        public object Handler { get; set; }
        public object HandlerFactory { get; set; }
        public bool IsCorsEnabled { get; set; }
        public bool IsSecure { get; set; }
    }

    public class CustomResourceSchemeHandlerFactory : DefaultResourceSchemeHandlerFactory
    {
        public CustomResourceSchemeHandlerFactory(IChromelyErrorHandler chromelyErrorHandler) : base(chromelyErrorHandler)
        {
        }

        protected override CefResourceHandler Create(CefBrowser browser, CefFrame frame, string schemeName, CefRequest request)
        {
            return new CustomResourceSchemeHandler(_chromelyErrorHandler);
        }
    }

    public class CustomResourceSchemeHandler : DefaultResourceSchemeHandler
    {
        private const string FOLDER = @"E:\img";

        public CustomResourceSchemeHandler(IChromelyErrorHandler chromelyErrorHandler) : base(chromelyErrorHandler)
        {
        }

        [Obsolete]
        protected override bool ProcessRequest(CefRequest request, CefCallback callback)
        {
            // http://img/chromely_gray.png
            var u = new Uri(request.Url);
            var file = $"{FOLDER}{u.AbsolutePath}";

            _totalBytesRead = 0;
            _chromelyResource.Content = null;
            _completed = false;

            _fileInfo = new FileInfo(file);
            // Check if file exists
            if (!_fileInfo.Exists)
            {
                _chromelyResource = _chromelyErrorHandler.HandleError(_fileInfo);
                callback.Continue();
            }
            // Check if file exists but empty
            else if (_fileInfo.Length == 0)
            {
                _chromelyResource = _chromelyErrorHandler.HandleError(_fileInfo);
                callback.Continue();
            }
            else
            {
                Task.Run(() =>
                {
                    using (callback)
                    {
                        try
                        {
                            var fileBytes = File.ReadAllBytes(file);
                            _chromelyResource.Content = new MemoryStream(fileBytes);

                            string extension = Path.GetExtension(file);
                            _chromelyResource.MimeType = MimeMapper.GetMimeType(extension);
                            _chromelyResource.StatusCode = ResourceConstants.StatusOK;
                            _chromelyResource.StatusText = ResourceConstants.StatusOKText;
                        }
                        catch (Exception exception)
                        {
                            _chromelyResource = _chromelyErrorHandler.HandleError(_fileInfo, exception);
                        }
                        finally
                        {
                            callback.Continue();
                        }
                    }
                });

                return true;
            }

            callback.Dispose();
            return false;
        }
    }

@cederron did you succeed with it? i also need the resouces to be editable (filesystem based) and not packed away in dlls.

Yes, But i ditched scheme handlers and all that stuff.
I assign img src to a controller returning a FileStream result. Works great and much simpler.
(I'm using Chromely + Blazor)

[Route("image")]         public async Task<FileStreamResult> Image(string filename)         {             FileStream stream = null;             stream = System.IO.File.OpenRead(filename);             FileStreamResult result = File(             fileStream: stream,             contentType: new MediaTypeHeaderValue("image/" + Path.GetExtension(filename).Remove(0, 1)).MediaType              );             return result;         }

thanks for the feedback i will check it out!

Please read! - https://github.com/chromelyapps/Chromely/issues/357


Translate picture description

When you tap the label where PictureDescription is output, the TranslationResultPage modal is displayed.

When displaying the PictureDescription, I wanted to support multiple languages, so I used TranslationResultPage.

Thanks for completing the challenge! Fill out this form so we can send you the swag! https://aka.ms/cog-svcs-swag


Internal CRDs

I have an operator that uses a number of internal CRDs to maintain state, in addition to user-facing CRDs which define the external API. Is there a way to differentiate internal vs external CRDs in the CSV definition such that users are not guided to create / modify the internal CRDs manually?

closed in favor of https://github.com/operator-framework/operator-lifecycle-manager/issues/975


zIndex

Fixed a small issue with UICollectionViewLayoutAttributes' zIndex whe n cards overlap each other.

To get a sense of what the problem is, change this:
UPCarouselFlowLayoutSpacingMode.fixed(spacing: 40)

To this:
UPCarouselFlowLayoutSpacingMode.fixed(spacing: 160)

:)

Indeed I hadn't noticed that, thanks for your feedback!


[Bug] Category of bridged devices is always Other

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug
I was inspecting individual devices exposed through homebridge-z2m, and noticed that all devices have the same HAP accessory category of Other.
This behavior can be attributed to the following line:
https://github.com/itavero/homebridge-z2m/blob/cb71fd79e0890c9f6e8bf59214d27d6036e949d6/src/platform.ts#L598
Because we're not explicitly specifying an accessory category, a default category is used by Homebridge. I'm not sure how relevant this is for bridged devices, but it might help in cases like #869... 🤔
Related devices
No response
Related Devices
No response
Steps To Reproduce
No response
Expected behavior
No response
Device entry
No response
Status update
No response
Messages from this plugin
No response
This plugin
1.11.0-beta.6
Homebridge
1.7.0
Zigbee2MQTT
1.39.0-dev
Homebridge Config UI X (if applicable)
No response

Any feature request you are proposing?

maybe we can use the Z2M converter type as a hint for the correct accessory category?

Yes, but a single accessory can use multiple converters, so they should somehow have a priority between them.


getBuffer() exits the process when callback finished?

server.on("imagerequest", message => {

  if(message.content != "!image") return;

  Jimp.read("./image.png", function(err, image) {
    image.invert();
    image.getBuffer(Jimp.AUTO, function(err, png) {
      message.uploadFile(png); //takes a buffer and uploads it to the user
    });
  });
});

the program just stops right after completing the getBuffer callback.
The image is successfully uploaded and everything, but the server closes right after.
I can't have that as I need this to run multiple times due to the nature of this being a Discord bot

@zfwman can you provide a repo with a minimal implementation?

@hipstersmoothie Isn't that code enough? It's just an event. Also, I found it might be caused due to the fact I was running it on the ARM architecture which has some problems with nodejs


TaskBar brokes

If change the resolution or increase the number of taskbar rows, the display breaks before restarting TaskBarX

Windows 10 21H2, Same on all my devices.

I'm also experiencing this issue when the laptop is in sleep mode, then a higher resolution monitor is plugged in via USB-C and a dongle before turning on the laptop.

I think mine is related to this:

I have the same bug with any version above <IP_ADDRESS>.


How to install osquery and cnquery using pkgx

Hi,
I tried to install osquery and cnquery using
pkgx install osquery
pkgx install cnquery

However, getting the below error:
× nothing provides: osquery
│ we haven’t pkgd this yet. can you?
╰─➤ https://docs.pkgx.sh/pantry

---------
× nothing provides: cnquery
│ we haven’t pkgd this yet. can you?
╰─➤ https://docs.pkgx.sh/pantry

This issue should be opened in https://github.com/pkgxdev/pantry


General update of the dataset

Can you reimport updated versions of the original datasets?
I think East Germany doesn't exist as a sovereign state anymore (Or did I miss something?)...
https://github.com/umpirsky/country-list/blob/3db06e2cdb78401c1feab9ca45385342749ffc93/country/cldr/en/country.csv#L67

@Jadaw1n Fixed, thanks.


EventLog logger for K

Create a new package for an EventLog sink and expose the API that the user would put in their startup class: app.UseEventLog();
Figure out how to register the category name that requires elevated privilege. P/Invoke for accessing event log.

Dupe of #83


Implement event::Source for DataReader and DataWriter

IMHO this would allow DataReader and DataWriter to be wrapped in Arc<Mutex<>>

I do not understand this request. There are several unclear points:

In a previous issue #202 you state that you are already doing this wrapping.
How does it fail now?
What makes you think that implementing mio::event::Source would help?
Implementing Source requires an underlying system file/socket handle. How would you arrange this for a DataReader?

This seems to be done since Release 0.8.x


Use env for resources

New environment variables determine how the resource files (cargoes.json and handling_events.json) are used depending on the  Mix environment: either :dev or :test.
If :dev then copy the files to a dev directory (creating the directory if necessary) if there are none in this directory.
If :test then always copy the files to a test directory (creating the directory if necessary). This action ensures that tests based on the contents of these resources are reproducible.

This pull request is closed. This solution is prone to unecessary test failure if resource files are changed. Modify tests with a setup that creates the test cargoes and handling events.


Bump minimum Rust version to 1.73

bumpalo requires Rust 1.73 now

This feels high to me. Is bumpalo a required dependency?

Yeah unfortunately it's a transitive dependency through wasm-bindgen -> wasm-bindgen-macro -> wasm-bindgen-macro-support -> wasm-bindgen-backend -> bumpalo


svg.pan-zoom.js 2.0.0

[x] Move the graph programmatically. Fixes #3.
[x] Zoom the graph programmatically.
[ ] Zoom in center, programmatically. :x:
:bug: Fixed the zoom issue. Fixed #2.

:fireworks: :ship:

:boom: :sunny: :+1:


[Bug]: DialogClose overrides accessible name given by child content

Environment
Developement/Production OS: macOS 11.6.2
Node version: (whatever Stackblitz uses)
Package manager: (whatever Stackblitz uses)
Radix Vue version: 1.2.3
Vue version: latest
Nuxt version: -
Nuxt mode: -
Nuxt target: -
CSS framework: -
Client OS: macOS 11.6.2
Browser: Chrome 119.0.6045.159

Link to minimal reproduction
https://www.radix-vue.com/components/dialog.html
Steps to reproduce

Use DialogClose to wrap any button that closes the modal, whether as-child or <MyCustomButton :as="DialogClose">

Describe the bug
The resulting HTML is <button aria-label="Close" ...>{{ the button's content }}</button>.
This means that the button's accessible name is overridden by the aria-label="Close", and "Close" will be what is announced by screen readers, for example.
Expected behavior
I'm not really sure what the best solution is. Personally, I interpret DialogClose as the preferred way of closing a Radix Vue Dialog. It doesn't necessarily have to be a "Close" button, it could be a "Cancel" or an "Okay" button (which otherwise behave the same as a "Close" button). I would like to be able to use DialogClose in such cases too, instead of needing to manually control the state myself.
Maybe it should be the consumer's responsibility to give the 'Close' button an accessible name, like in Radix UI?
Conext & Screenshots (if applicable)
It's also a problem for AlertDialogAction and AlertDialogCancel.

Here's a Stackblitz: https://stackblitz.com/edit/tcw2rz?file=src%2FApp.vue
Sorry, I'm not sure how to best get my message across in a repro, it's essentially the same as the example in the docs, so I removed the unimportant content and added some comments 😅

What I want to get at is that when I write:
<template>
  ...
    <DialogClose as-child>
      <button type="button"> My button </button>
    </DialogClose>
  ...
</template>

I expect the resulting HTML:
<button type="button"> My button </button>

Which screen readers would announce as "My button".
Instead, this is the resulting HTML:
<button type="button" aria-label="Close"> My button </button>

And screen readers announce it as "Close" instead of "My button", which is problematic because the phrase announced by the screen reader doesn't match or contain the button's visual label. This means that screen reader users are misled as to what button actually does.

Maybe I can bring up this line:
https://github.com/radix-vue/radix-vue/blob/523c323b517bcb7010646cd8980a78cd87d797b7/packages/radix-vue/src/Dialog/DialogClose.vue#L22
I believe it enforces aria-label="Close" on DialogClose. This makes sense if the DialogClose is used as a "Close" button, but it does not make sense in all other cases, such as when the button has a visual label of "Cancel", or if the web page/app is in a language other than English.

Personally, I don't know what the best solution is.
I think it should be the user's responsibility to give 'x' buttons an accessible name. I think this is preferable for i18n considerations too.
If not, the docs need to better explain how a user is supposed to close a Dialog if they don't want aria-label="Close".

just a proposal
Adding boolean accessible prop to Primitive component to filter out aria-* attributes,
with accessible: false prop, it's user duty to provide the accessibility to the component

@hoonweiting
Another way is to remove aria-label="Close" is to add :aria-label="null" to the DialogClose.vue component

I see your use case now @hoonweiting . I have 3 solution for you.

set aria-label to null or undefined (easy)

<DialogClose as-child>
    <button type="button"  :aria-label="undefined">My button</button>
</DialogClose>

set your own aria-label (easy)

<DialogClose as-child>
    <button  aria-label="My button">My button</button>
</DialogClose>

close the modal programmatically using your own button (medium)
assume your have v-model:open="open"

 <button  type="button" @click="open = false">My button</button>

I believe this is what you need right? If so we can "close" this issue 😁

@zernonia I suppose so, but I would rather not need to set aria-label at all. I suppose I expect DialogClose to close the modal, without imposing any specific label. This is what PopoverClose and ToastClose do, they don't impose aria-label="Close".
If we would still like to continue imposing aria-label="Close" in DialogClose, then perhaps we can update the docs to better inform users that they'll need to (un)set aria-label themselves?

Your comment on PopoverClose/ToastClose made me wonder why the aria-label was set in DialogClose. Then I check the original design from Radix UI and realise it doesn't impose the aria-label!
Let's remove it then in our source code. @hoonweiting  do you want to help contribute to this changes? 😁

@zernonia Sure, that sounds good! I'll make a PR when I get some time today.

Appreciate it @hoonweiting ! 🙌🏼


Is it possible to suppress a rule on a block of code?

I would like to disable/suppress ruff (or certain rules of ruff) on a block of code. I know that I can do this for single lines (by using # noqa: <rule_code> at the end of the line) or for entire files/folder (by specifying them in the settings file).
However, I would like to disable ruff on an entire function or on a multi-line block of code, but not an entire file. Is there a way to do this, without adding the noqa to the end of each line?
Thanks

Unfortunately we don't support block-level suppression right now. I believe this is similar to #3711 so I'm going to merge into that issue.