Bug 1123435
When I fix this and rebuild build as several of the test fails.
This is not surprising because the patch is skipping tests for unsupported features and we must take into account the the servers are not going to start in some cases.
I fix the Bug-1001841-disable-sslv2-tests.patch by adding these lines:
diff --git a/tests/chains/chains.sh b/tests/chains/chains.sh
--- a/tests/chains/chains.sh
+++ b/tests/chains/chains.sh
@@ -35,17 +35,21 @@ is_httpserv_alive()
if [ "${OS_ARCH}" = "WINNT" ] && \
[ "$OS_NAME" = "CYGWIN_NT" -o "$OS_NAME" = "MINGW32_NT" ]; then
PID=${SHELL_HTTPPID}
else
PID=`cat ${HTTPPID}`
fi
echo "kill -0 ${PID} >/dev/null 2>/dev/null"
+ if [ "${NSS_NO_SSL2}" = "1" ]; then
+ echo "skipping kill because NSS_NO_SSL=${NSS_NO_SSL2}"
+ else
kill -0 ${PID} >/dev/null 2>/dev/null || Exit 10 "Fatal - httpserv process not detectable"
+ fi
echo "httpserv with PID ${PID} found at `date`"
}
########################### wait_for_httpserv ##########################
# local shell function to wait until httpserver is running and initialized
########################################################################
wait_for_httpserv()
@@ -54,17 +58,21 @@ wait_for_httpserv()
echo "tstclnt -p ${NSS_AIA_PORT} -h ${HOSTADDR} -q -v"
${BINDIR}/tstclnt -p ${NSS_AIA_PORT} -h ${HOSTADDR} -q -v
if [ $? -ne 0 ]; then
sleep 5
echo "retrying to connect to httpserv at `date`"
echo "tstclnt -p ${NSS_AIA_PORT} -h ${HOSTADDR} -q -v"
${BINDIR}/tstclnt -p ${NSS_AIA_PORT} -h ${HOSTADDR} -q -v
if [ $? -ne 0 ]; then
- html_failed "Waiting for Server"
+ if [ "${NSS_NO_SSL2}" = 1 ]; then
+ html_passed "Waiting for Server is supposed to fail"
+ else
+ html_failed "Waiting for Server"
+ fi
fi
fi
is_httpserv_alive
}
########################### kill_httpserv ##############################
# local shell function to kill the httpserver after the tests are done
########################################################################
@@ -1174,17 +1182,21 @@ parse_config()
;;
"break")
break
;;
"check_ocsp")
TESTNAME="Test that OCSP server is reachable"
check_ocsp ${VALUE}
if [ $? -ne 0 ]; then
+ if [ "${NSS_NO_SSL2}" = "1" ]; then
+ html_passed "$TESTNAME"
+ else
html_failed "$TESTNAME"
+ fi
break;
else
html_passed "$TESTNAME"
fi
;;
"ku")
EXT_KU="${VALUE}"
;;
diff --git a/tests/ssl/ssl.sh b/tests/ssl/ssl.sh
--- a/tests/ssl/ssl.sh
+++ b/tests/ssl/ssl.sh
@@ -115,17 +115,21 @@ is_selfserv_alive()
if [ "${OS_ARCH}" = "WINNT" ] && \
[ "$OS_NAME" = "CYGWIN_NT" -o "$OS_NAME" = "MINGW32_NT" ]; then
PID=${SHELL_SERVERPID}
else
PID=`cat ${SERVERPID}`
fi
echo "kill -0 ${PID} >/dev/null 2>/dev/null"
+ if [ "${NSS_NO_SSL2}" = "1" ]; then
+ ;
+ else
kill -0 ${PID} >/dev/null 2>/dev/null || Exit 10 "Fatal - selfserv process not detectable"
+ fi
echo "selfserv with PID ${PID} found at `date`"
}
########################### wait_for_selfserv ##########################
# local shell function to wait until selfserver is running and initialized
########################################################################
wait_for_selfserv()
@@ -138,17 +142,21 @@ wait_for_selfserv()
if [ $? -ne 0 ]; then
sleep 5
echo "retrying to connect to selfserv at `date`"
echo "tstclnt -p ${PORT} -h ${HOSTADDR} ${CLIENT_OPTIONS} -q \\"
echo " -d ${P_R_CLIENTDIR} -v < ${REQUEST_FILE}"
${BINDIR}/tstclnt -p ${PORT} -h ${HOSTADDR} ${CLIENT_OPTIONS} -q \
-d ${P_R_CLIENTDIR} -v < ${REQUEST_FILE}
if [ $? -ne 0 ]; then
+ if [ "${NSS_NO_SSL2}" = "1" ]; then
+ html_passed "Server never started"
+ else
html_failed "Waiting for Server"
+ fi
fi
fi
is_selfserv_alive
}
and try again. An the test still fail.
Let's look at Bug-1001841-disable-sslv2-libssl.patch patch but the done with diff -up ./nss/lib/ssl/sslsock.c.disableSSL2 ./nss/lib/ssl/sslsock.c doesn't provide enough context.
Better do diff -C 8 sslsock.c.disableSSL2 sslsock.c which shows:
case SSL_ENABLE_SSL2:
+ #ifdef NSS_NO_SSL2
+ if (on) {
+ PORT_SetError(SSL_ERROR_SSL2_DISABLED);
+ rv = SECFailure; /* not allowed */
+ }
+ break;
+ ss->opt.enableSSL2 = on;
+ #else
if (IS_DTLS(ss)) {
if (on) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
rv = SECFailure; /* not allowed */
}
break;
}
ss->opt.enableSSL2 = on;
if (on) {
ss->opt.v2CompatibleHello = on;
}
+ #endif
ss->preferredCipher = NULL;
if (ss->cipherSpecs) {
PORT_Free(ss->cipherSpecs);
ss->cipherSpecs = NULL;
ss->sizeCipherSpecs = 0;
}
break;
This doesn't like quite right. I think we should instead have something like
....
break;
case SSL_ENABLE_SSL2:
+ #ifdef NSS_NO_SSL2
+ if (on) {
+ PORT_SetError(SSL_ERROR_SSL2_DISABLED);
+ rv = SECFailure; /* not allowed */
+ }
+ #else
if (IS_DTLS(ss)) {
if (on) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
rv = SECFailure; /* not allowed */
}
break;
}
ss->opt.enableSSL2 = on;
if (on) {
ss->opt.v2CompatibleHello = on;
}
ss->preferredCipher = NULL;
if (ss->cipherSpecs) {
PORT_Free(ss->cipherSpecs);
ss->cipherSpecs = NULL;
ss->sizeCipherSpecs = 0;
}
+ #endif /* NSS_NO_SSL2 */
break;
Created attachment 921077 [details]
All changes needed to fix the flaws
This is a bit hard on the eyes as it has differences between version of patches so let me attach the individual patches next.
Created attachment 921080 [details]
fix disabling of ssl2/export support - conditional compilation part
Created attachment 921082 [details]
fix skipping of ssl2/export tests that arent supported - testscripts part
If anyone is interested, a scratch build with these changes applied can be found at https://brewweb.devel.redhat.com/taskinfo?taskID=7738232 Being scratch it gets garbage collected in a week or so. Comment on attachment 921077 [details]
All changes needed to fix the flaws
r+ for this set. NOTE: I think it's confusing having 2 patches with the same .suffices, I would prefer if you distiguished them.
NOTE2: While what I can see in this patch is fine, the disable tests code is NOT fine. I don't know if it's an existing problem, but none of those show up in this patch here even though it sall all changes.
Comment on attachment 921082 [details]
fix skipping of ssl2/export tests that arent supported - testscripts part
r- This doesn't turn off SSL2 ciphers, it ignores all problems in all ciphers if SSL2 is turned off.
What you want is to create new versions of test/ssl/sslcov.txt and test/ssl/sslstress.txt which do not have the SSL2 tests, and then switch to those either unconditionally or in the script.
bob
Comment on attachment 921080 [details]
fix disabling of ssl2/export support - conditional compilation part
r+ rrelyea
Created attachment 922338 [details]
removal from to sslcov.txt of SSL2 and export tests cases
Created attachment 922339 [details]
removal from sslstress.txt of SSL2 and export tests cases
Created attachment 922341 [details]
Fix skipping of ssl2 & export test which aren't supprted - test scripts part
Created attachment 922344 [details]
changes to nss.spc file in patch format
With different suffixes for the libssl and test parts as requested.
Comment on attachment 922341 [details] Fix skipping of ssl2 & export test which aren't supprted - test scripts part (In reply to Bob Relyea from comment #7) > Comment on attachment 921082 [details] > What you want is to create new versions of test/ssl/sslcov.txt and > test/ssl/sslstress.txt which do not have the SSL2 tests, and then switch to > those either unconditionally or in the script. > I opted to add new versions of test/ssl/sslcov.txt and test/ssl/sslstress.txt and in the script checks NSS_NO_SSL2 to switch to the new ones, otherwise the originals are used. Comment on attachment 922338 [details]
removal from to sslcov.txt of SSL2 and export tests cases
r+
Comment on attachment 922341 [details]
Fix skipping of ssl2 & export test which aren't supprted - test scripts part
r- You'll need to explain to me why these are needed. The still look like disabling generic ssl tests.
Comment on attachment 922344 [details]
changes to nss.spc file in patch format
r+ rrelyea
Comment on attachment 922339 [details]
removal from sslstress.txt of SSL2 and export tests cases
r- You are diabling to many tests here. There is only 1 SSL2 test and no export tests in this suite. If you are turning off MD5, please change them to SHA1 then.
bob
(In reply to Bob Relyea from comment #16) > Comment on attachment 922341 [details] > Fix skipping of ssl2 & export test which aren't supprted - test scripts part > > r- You'll need to explain to me why these are needed. The still look like > disabling generic ssl tests. Are you suggesting - noECC 0 _ -c_1000_-C_A Stress SSL2 RC4 128 with MD5 with - noECC 0 _ -c_1000_-C_A Stress SSL2 RC4 128 with MD5 + noECC 0 _ -c_1000_-C_A Stress SSL2 RC4 128 with SHA1 and so on? I dout it. (In reply to Bob Relyea from comment #18) > Comment on attachment 922339 [details] > removal from sslstress.txt of SSL2 and export tests cases > > r- You are diabling to many tests here. There is only 1 SSL2 test and no > export tests in this suite. If you are turning off MD5, please change them > to SHA1 then. > > bob Not sure if I fully unedestand this part but I think you are right. 1) The changes to tests/chains/chains.sh, aren't needed at all. is_httpserv_alive(), wait_for_httpserv(), and kill server or not need to go away. I removed them and it builds just fine. 2) I do need this part diff --git a/tests/ssl/ssl.sh b/tests/ssl/ssl.sh --- a/tests/ssl/ssl.sh +++ b/tests/ssl/ssl.sh @@ -57,18 +57,23 @@ ssl_init() fi PORT=${PORT-8443} NSS_SSL_TESTS=${NSS_SSL_TESTS:-normal_normal} nss_ssl_run="stapling cov auth stress" NSS_SSL_RUN=${NSS_SSL_RUN:-$nss_ssl_run} # Test case files - SSLCOV=${QADIR}/ssl/sslcov.txt + SSLCOV=[ "${NSS_NO_SSL2}" = "1" ] \ + && ${QADIR}/ssl/sslcov.txt.disableSSL2test \ + || ${QADIR}/ssl/sslcov.txt SSLAUTH=${QADIR}/ssl/sslauth.txt + SSLSTRESS=[ "${NSS_NO_SSL2}" = "1" ] \ + && ${QADIR}/ssl/sslstress.txt.disableSSL2test \ + || ${QADIR}/ssl/sslstress.txt SSLSTRESS=${QADIR}/ssl/sslstress.txt REQUEST_FILE=${QADIR}/ssl/sslreq.dat 2) On /tests/ssl/ssl.sh b/tests/ssl/ssl.sh we need that part in ###################### wait_for_selfserv ####################### and ########################### kill_selfserv ################# but the tests the script does should be changed from + if [ "${NSS_NO_SSL2}" = "1" ]; then ... to + if [ "${NSS_NO_SSL2}" = "1" ] && [ -n ${EXP} -o -n ${SSL2} ]; then ... just like further down in loop where skip + # skip export and ssl2 tests when build has disabled SSL2 + [ "${NSS_NO_SSL2}" = "1" ] && [ -n ${EXP} -o -n ${SSL2} ] && continue + I hope I have understood you correctly. I new patch is comming. Created attachment 922640 [details]
Fix skipping of ssl2 & export test which aren't supported - test scripts part - V3
RE: the SSL stress test patch. *** sslstress.txt 2014-06-24 13:45:27.000000000 -0700 --- sslstress.txt.disableSSL2test 2014-07-29 13:15:57.463569838 -0700 *************** *** 3,41 **** # file, You can obtain one at http://mozilla.org/MPL/2.0/. # # This file defines the stress tests for SSL/TLS. # # expected # Enable return server client Test Case name # ECC value params params # ------- ------ ------ ------ --------------- - noECC 0 _ -c_1000_-C_A Stress SSL2 RC4 128 with MD5 - noECC 0 _ -c_1000_-C_c_-V_:ssl3 Stress SSL3 RC4 128 with MD5 - noECC 0 _ -c_1000_-C_c Stress TLS RC4 128 with MD5 - noECC 0 _ -c_1000_-C_c_-g Stress TLS RC4 128 with MD5 (false start) - noECC 0 -u -V_ssl3:_-c_1000_-C_c_-u Stress TLS RC4 128 with MD5 (session ticket) - noECC 0 -z -V_ssl3:_-c_1000_-C_c_-z Stress TLS RC4 128 with MD5 (compression) - noECC 0 -u_-z -V_ssl3:_-c_1000_-C_c_-u_-z Stress TLS RC4 128 with MD5 (session ticket, compression) - noECC 0 -u_-z -V_ssl3:_-c_1000_-C_c_-u_-z_-g Stress TLS RC4 128 with MD5 (session ticket, compression, false start) - SNI 0 -u_-a_Host-sni.Dom -V_tls1.0:_-c_1000_-C_c_-u Stress TLS RC4 128 with MD5 (session ticket, SNI) # # add client auth versions here... # - noECC 0 -r_-r -c_100_-C_A_-N_-n_TestUser Stress SSL2 RC4 128 with MD5 (no reuse, client auth) - noECC 0 -r_-r -c_100_-C_c_-V_:ssl3_-N_-n_TestUser Stress SSL3 RC4 128 with MD5 (no reuse, client auth) - noECC 0 -r_-r -c_100_-C_c_-N_-n_TestUser Stress TLS RC4 128 with MD5 (no reuse, client auth) - noECC 0 -r_-r_-u -V_ssl3:_-c_100_-C_c_-n_TestUser_-u Stress TLS RC4 128 with MD5 (session ticket, client auth) - noECC 0 -r_-r_-z -V_ssl3:_-c_100_-C_c_-n_TestUser_-z Stress TLS RC4 128 with MD5 (compression, client auth) - noECC 0 -r_-r_-z -V_ssl3:_-c_100_-C_c_-n_TestUser_-z_-g Stress TLS RC4 128 with MD5 (compression, client auth, false start) - noECC 0 -r_-r_-u_-z -V_ssl3:_-c_100_-C_c_-n_TestUser_-u_-z Stress TLS RC4 128 with MD5 (session ticket, compression, client auth) - noECC 0 -r_-r_-u_-z -V_ssl3:_-c_100_-C_c_-n_TestUser_-u_-z_-g Stress TLS RC4 128 with MD5 (session ticket, compression, client auth, false start) - SNI 0 -r_-r_-u_-a_Host-sni.Dom -V_tls1.0:_-c_1000_-C_c_-u Stress TLS RC4 128 with MD5 (session ticket, SNI, client auth, default virt host) - SNI 0 -r_-r_-u_-a_Host-sni.Dom_-k_Host-sni.Dom -V_tls1.0:_-c_1000_-C_c_-u_-a_Host-sni.Dom Stress TLS RC4 128 with MD5 (session ticket, SNI, client auth, change virt host) Only one of those tests above is SSL2. I don't understand way you are removing SSL3 and TLS ciphers, and my only guess was you were trying to remove MD5. Removing the single SSL2 test above is fine, removing the large number on unrelated tests is not. > 2) I do need this part Agreed, this part is fine. The other part of the chain.sh test looked like it was silencing all test errors. > 2) On /tests/ssl/ssl.sh b/tests/ssl/ssl.sh we need that part in > ###################### wait_for_selfserv ####################### > and > ########################### kill_selfserv ################# These changes are acceptable, thought I'd like to understand why they are needed now that you've removed the ssl2 tests from the source file (at least you aren't silencing non-ssl2 tests anymore). bob (In reply to Bob Relyea from comment #21) ..... > Only one of those tests above is SSL2. I don't understand way you are > removing SSL3 and TLS ciphers, and my only guess was you were trying to > remove MD5. Removing the single SSL2 test above is fine, removing the large > number on unrelated tests is not. Yes, I was trying to remove MD5. I did a build removing only SSL2 and it works just fine. I did another build where I changed MD5 to SHA1 in the stress test, as I think you suggested earlier, and that works fine. If I'm going to stress testing I better do it with SHA1 instead of with MD5. Version 4 coming next. Created attachment 922673 [details]
Fix skip of unsupprted ssl2/export tests - test scripts part - V4
> Yes, I was trying to remove MD5. I did a build removing only SSL2 and it
> works just fine.
For this bug why don't we just remove ths ssl2 tests. We haven't turned off all uses of MD5, only MD5 used in signatures, so doing so isn't related to this bug.
Created attachment 922728 [details]
Fix disabling: test scripts part - V5 - just remove ths ssl2 tests
Created attachment 922970 [details]
fix skipping of ssl2/export tests that aren't supported - testscripts part V6
Comment on attachment 922970 [details]
fix skipping of ssl2/export tests that aren't supported - testscripts part V6
r+ rrelyea
Created attachment 985681 [details]
fix shell syntax errors ssl2/export test skipping patch and dbtest patch
Created attachment 986851 [details]
fixes most of the shell syntax errors
A got a scratch build to complete with this patch but I had to disable the fix for cipher.sh. It's a work in progress. I just wanted to save the work done so far so there is no need to review it. By the time we start working on rhel-7.2 a lot will have changed.
Created attachment 988982 [details]
fixes all reported shell syntax errors
Created attachment 989305 [details]
fixes all reported shell syntax errors and more
changes pushed to git: http://pkgs.devel.redhat.com/cgit/rpms/nss/commit/?h=rhel-7.2&id=2de7324289c8efca7be7fde403fc2a6308e636d8 Comment on attachment 989305 [details]
fixes all reported shell syntax errors and more
One flaw (among others)
+ if [[ "${NSS_NO_SSL2}" = "1" ] && [ -n ${EXP} -o -n ${SSL2} ]]; then
First mistake is the double brackets "[[" and "]]" but even with single brakes
+ if [ "${NSS_NO_SSL2}" = "1" ] && [ -n ${EXP} -o -n ${SSL2} ]; then
is still is wrong. -n is a string comparison. -n is true if it's nonzero length, the integer is always a nonzero string which means, the expression is always true
Thanks to Kai that pointed it out in the context of another bug investigation and another part of the ssl.sh script. More analysis and fix proposals to come.
Created attachment 1058571 [details] Fix checks for SSL2 and EXPORT ciphers suite tests Patch provided by Kaie adds a lot of of tracing and fixes some serious errors in the way I check in the ssl test suite script for SSL2 and Export cipher suites. Test cases when SSL2/EXPORT is disabled as has been the case since rhel-7.0. Recently a regression was reported, that the NULL cipher suites are broken on RHEL 7. Kai thoroughly analyzed the problem and traced the error to bad commits I made. I'll copy from what he told me that's right on spot. My first commit on 2014-07-10, which introduced the error + # skip export and ssl2 tests when build has disabled SSL2 -+ if [ "${NSS_NO_SSL2}" = "1" ] && [ -n ${EXP} -o -n ${SSL2} ]; then <-1 -+ echo "SKIPPED" -+ continue -+ fi ++ if [ "${NSS_NO_SSL2}" = 1 ] && [ -n ${EXP} -o -n ${SSL2} ] && continue + if [ "${SSL2}" -eq 0 ] ; then <- 2 The first check is in error, see below for why. ------------------------------------------------ Another commit, on 2014-08-01, changed style but kept the error in the check. + # skip export and ssl2 tests when build has disabled SSL2 -+ if [ "${NSS_NO_SSL2}" = 1 ] && [ -n ${EXP} -o -n ${SSL2} ] && continue ++ [ "${NSS_NO_SSL2}" = "1" ] && [ -n ${EXP} -o -n ${SSL2} ] && continue + if [ "${SSL2}" -eq 0 ] ; then # We cannot use asynchronous cert verification with SSL2 SSL2_FLAGS=-O + VMIN="ssl2" Kai states: as you can see below the line that you had added, the correct way to check the value of those variables is with the arithmetic comparison operators there is an existing line: if [ "${SSL2}" -eq 0 ] ; then this one does it right But exactly one line above it, you added your new test with -n -n is a string comparison. -n is true if it's nonzero length. The integer is always a nonzero string. That means, the expression is always true, and all ciphers go to continue and are skipped I think you should have checked the logs after you added this new code to ensure that these tests are still being executed for the remaining ciphers. Today we got a bug report, a regression, that the NULL ciphersuites are broken on RHEL 7" The reference is to Bug 1245627 (TLS_RSA_WITH_NULL_SHA256 nor working in RHEL7) I'm currently working on a patch for that bug which calls for enabling some additional cipher suites by default and fixing this one is required in order to properly verify the fix for that other one. Kai's patch as it is fixes the errors. It may need some minor changes. Received very good advise on an email discussion I had with Kai and Hubert. some snippets from Hubert's email.
.....
if [ "${NSS_NO_SSL2}" = "1" ] && [ "${EXP}" -eq 1 -o "${SSL2}" -eq 1 ]; then
continue
fi
Though we will have problems if the variables are empty:
EXP=
[ "${EXP}" -eq 1 ]
bash: [: : integer expression expected
For that
if [ "${NSS_NO_SSL2}" = "1" ] && [[ ${EXP} -eq 1 || ${SSL2} -eq 1 ]]; then
continue
fi
would be better.
That is because [] uses the POSIX test(1) syntax, while [[]] is the bash
extension (see bash(1), look for '\[\[ expression \]\]' or 'CONDITIONAL
EXPRESSIONS'), where the shell knows that it is comparing variables, not
parameters.
I tested this in various ways and with additional logs and it solves the outstanding issues. I'll submit a new patch next, the copious, yet useful, tracing has been reduced to a bare minimum.
Created attachment 1059558 [details]
Fix checks for SSL2 and EXPORT cipher suites tests
Elio, I used the "yyy" output markers only for debugging purposes. I think they shouldn't be contained in the production patch (if you want debug output, use something better than yyy). You've asked for a review. I have to say that it's easy to confuse these statements. I wouldn't trust my own review without having actually tested it. My suggestion is that you test it and inspect the output to ensure that it works as expected. I think you should test the patch prior to asking for review. I would like to suggest that you use the identical expression in all three places you're changing. Please remove the "yyy" from the test output, you can keep the rest. Looking at the code, you have changed my "-eq 0" to "-eq 1", which is wrong. You want to skip (continue) if at least one value was found. grep returns 0 for "found". Created attachment 1061267 [details] Fix checks for SSL2 and EXPORT cipher suites tests This not for review yet. The three checks are consistent now are compare against 0. One tracing echo line remains as I'm not done yet with the testing. If should mention that along with this patch I'm also testing using one that implements your recommendation from https://bugzilla.redhat.com/show_bug.cgi?id=1245627#c15 and takes care of all many failures (the ..._RSA_WITH_NULL_ ... ones), there are others. More details coming later to that other bug where the topic properly belongs. Created attachment 1061281 [details]
Fix checks for SSL2 and EXPORT cipher suites tests - not for checkin
This is not ready for review is merly for information. One tracing line remains as I am still testing and the testing includes a fix for the bug this one blocks. I'll provide details in that other bug were the topic properly belongs
*** Bug 1204404 has been marked as a duplicate of this bug. *** Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHBA-2015-2121.html |
Description of problem: A syntax error in the test to skip tests for SSL2 and export suites which are no longer supported. Version-Release number of selected component (if applicable): nss-3.15.4-7.el7 How reproducible: Always Steps to Reproduce: 1. Checkout the sources and run rhpkg prep 2. cd ./nss-3.15.4/nss/tests/ssl and run 3. $ sh -e ssl.sh Actual results: ssl.sh: line 356: syntax error near unexpected token `done' Expected results: No ouput Additional info: The faulty line is found via $ grep "NSS_NO_SSL2" ssl.sh if [ "${NSS_NO_SSL2}" = 1 ] && [ -n ${EXP} -o -n ${SSL2} ] && continue This should be [ "${NSS_NO_SSL2}" = 1 ] && [ -n ${EXP} -o -n ${SSL2} ] && continue In Bug-1001841-disable-sslv2-tests.patch this are the faulty lines + + # skip export and ssl2 tests when build has disabled SSL2 + if [ "${NSS_NO_SSL2}" = 1 ] && [ -n ${EXP} -o -n ${SSL2} ] && continue The problem remains latest build for rhel-7.1.