Modified GnuTLS priority according to standard crypto-policy guideline

[jvymazal]

attempt to solve #57

[rgerhards]

@jvymazal any interest in fixing this?

[jvymazal]

Yes, sorry for delayed response, I will get to this next week latest (hopefully bit sooner).

[jvymazal]

I am afraid I will have to close this, I did some more research and even if using system priority would be feasible for every distro using rsyslog (of which I am not sure, especially for smaller, less-known distros), the travis CI will never work with it as is, and I have no info about whether this will ever change.

[rgerhards]

Do we only have a Travis issue or do we have a real issue? If it is just Travis, we can for sure work around this...

[jvymazal]

As you can see all distros you have separate CI runs for are green, so unless this would affect somebody else we have only travis problem. I tried to find out what priority string the travis environment actually has as system and I am not even sure I got that right. However, it is unsuitable for relp as the test just stalls, I was unable to pinpoint the problem as I cannot reproduce it locally.

[rgerhards]

I can help with that -- but would like to double-check first. Is the repo actually librelp,git but really is rsyslog.git?

[nmav]

I'm not sure if that's related to the travis failure, but I see that the if(pThis->pristring == NULL) { condition is now ignored and the pThis->pristring not taken into account. Shouldn't it be something like:

if(pThis->pristring != NULL) {
r = gnutls_priority_set_direct(pThis->session, pThis->pristring, NULL);
} else {
r = gnutls_set_default_priority(pThis->session);
}

[jvymazal]

Thanks for suggestion, applied!

[jvymazal]

I fixed problem that @nmav spotted, however that does not help with the travis issue, as you can see, CI is unchanged. As for your last question @rgerhards the CI in question is rsylog.git, librelp.git on its own does not have a test case covering this, nor can I imagine how we would write one without client (rsyslog)

[rgerhards]

As for your last question @rgerhards the CI in question is rsylog.git, librelp.git on its own does not have a test case covering this, nor can I imagine how we would write one without client (rsyslog)

I just wondered why travis wanted me to download your librelp.git project in order to merge this rsyslog issue... Strange.

Anyhow, there is a work-around to see why it times out, see https://github.com/rsyslog/rsyslog/blob/master/configure.ac#L31

The actual issue is this here: http://lists.gnu.org/archive/html/autoconf/2017-11/msg00001.html Unfortunately, I got no suggestion on how to do a permanent fix.

[rgerhards]

lol, ok I removed my confusion on the repo -- I never realized this was actually against librelp ;-) The cure posted (configure.ac change) might be a bit harder to do there. You may want to email [email protected], they can grant interactive login permissions for otherwise hard to find issues.

[rgerhards]

sorry for not looking at this sooner - finally I'll try to get this going...

[rgerhards]

#69 should hopefully expose what's going wrong with Travis

[rgerhards]

ok, it looks like we really should work on doing our own testbench. Let me see what I can do for this to happen finally. So I'll now do a release with the already better code in master, get the testbench done, and the finally merge this and have a new release. sorry this all takes so long and thanks for your patience.

[jvymazal]

Thanks for your help with this, sometimes delays are inevitable, I am glad that it will happen, and if it help to improve testbench in the process all the better.

[rgerhards]

@jvymazal just an update: we are coming closer to a native testbench. It's already there, but it's the "pilot version". Needs to be refactored a bit. Hope to soon have it ready so that we can finally get your PR going, together with new tests.

[jvymazal]

Thanks, glad to hear that

[rgerhards]

@jvymazal finally... we now have our native testbench inside librelp. Would you be willing to give the PR a final shot?

[jvymazal]

Should I also rebase it?

[rgerhards]

yes, we need a rebase to get the fixes that make CI work

[rgerhards]

looks like we have a valid CI failure, a regression in anon mode

[jvymazal]

Yes, I will have to look into why the connect function fails...

[rgerhards]

@jvymazal I admit I do not remeber if I forgot to merge or if s/t real was... That said, would you be up for one more rebase so we can check and hopefully merge?

[jvymazal]

@rgerhards, so I have it rebased and made the priority string setting respect system default while enabling ANON-DH only in case of eRelpAuthMode_None (which I think is a right thing to do - we dio not want anonymous mode to be enabled when fingerprint or name authentication is configured), now looking at the test failures I am not sure which config are the tests running under, where can I find that please?

[rgerhards]

The errors look indeed a bit strange. Do they happen locally as well if you rebase to master? @alorbach has added some tests, but they should not fail just because of a new test.

The test environment configs are easiest to describe for the docker containers. For example, the docker-centos7 container is this one:

• https://github.com/rsyslog/rsyslog-docker/tree/master/dev_env/centos/base/7
• https://github.com/rsyslog/rsyslog-docker/tree/master/dev_env/centos/buildbot/7

That repo also has the other docker files. They are online at dockerhub, so you can also pull them to your local env.

@alorbach do you have an idea what might be wrong here?

[rgerhards]

@jvymazal we should probably re-set this PR int the 1.4.0 timeframe (soon). I have heavily restructured things, let's have a quick discussion of what we need and how this works in regard to backward compatibility.