[#1111] Refuse an HTTPS configuration without a key the handler can present, and leave a running HTTP handler as it is when one is applied

[vharseko]

Fixes #1111

Stacked on #1110. The branch sits on the head of #1110 (b4b0cad), so until #1110 is merged this PR also shows its two commits. The change of this PR is the last commit, 3dd05f8. It replaces the forUse flag that #1110 adds to HTTPConnectionHandler.

Problem

The SSL settings of the HTTP connection handler take effect only when it restarts: Grizzly takes the SSL engine configuration when the handler starts its HTTP server, and every SSL property is in anyChangeRequiresRestart. The LDAP handlers are different: they build an SSL engine for each new connection, so their SSL changes take effect at once.

A change that leaves the HTTP handler without a key it can present (no key manager provider registered under the configured DN, a key store without a key, or none of the ssl-cert-nickname aliases in it) is accepted by the check, because wrapping an empty alias set does not throw. Then:

• on a running handler, the apply logs Disabling … and sets enabled = false, and a few lines later sets it back from the configuration. The handler keeps serving with its old certificate, and is down only after the restart;
• on a handler that its start disabled for want of a key, the apply sets enabled = true, and the handler starts listening with an SSL context that has no key.

Change

HTTPConnectionHandler.createSSLContext no longer touches enabled. It collects why the configuration leaves the handler without a key, and each caller decides what that means:

• check (isConfigurationAcceptable): refuses the configuration with these reasons. This covers a change to a running handler and a handler being added or enabled, which ConnectionHandlerConfigManager checks on a new instance. A key store that cannot be loaded is still refused with ERR_CONNHANDLER_SSL_CANNOT_INITIALIZE, as in [#1109] Keep a connection handler listening when a change to it is rejected #1110. Disabling the handler is not affected: the SSL part of the check runs only for an enabled configuration;
• start (initializeConnectionHandler): logs the reasons and Disabling …, and disables the handler, as before;
• apply (applyConfigurationChange): this path runs when the key store changed after the check. The apply adds the reasons and the new WARN_CONNHANDLER_NO_KEY_UNTIL_RESTART to the result and asks for administrative action. It logs no Disabling. A running handler keeps serving with the SSL settings it started with, and a handler that is down stays down (enabled = config.isEnabled() && (noKeyReasons.isEmpty() || isListening())). A change that gives a handler disabled at its start a key still starts it, as before.

Smaller changes in the same method:

• the reasons name the configuration by config.name() rather than by friendlyName, which is null on the new instance the check of an added or enabled handler runs on;
• a missing alias is reported by its own name rather than by the whole alias set ('no-such-cert' instead of '[no-such-cert]');
• while another configured alias is in the key store, a missing alias is only logged, as before.

The LDAP handlers are left as they are: an applied change without a key disables them at once, which matches their SSL changes taking effect at once (appliedChangeWithoutItsCertificateStopsListening from #1110 pins it for LDAPConnectionHandler2).

New message: WARN_CONNHANDLER_NO_KEY_UNTIL_RESTART_1541 in protocol.properties. No open PR claims 1541.

Tests

RejectedSSLConfigurationChangeTestCase gains 11 HTTP cases (24 in all):

• httpCheckRefusesAConfigurationWithoutAKey: for each missing key (alias, key store loaded but empty, no provider registered), on a running handler and on a new instance. The check refuses the configuration with exactly one reason of the expected kind, the reason names the handler, and a running handler keeps serving TLS;
• httpCheckAcceptsAConfigurationWithOneOfItsCertificates: ssl-cert-nickname: no-such-cert together with server-cert is accepted;
• httpAppliedChangeWithoutAKeyKeepsServingUntilTheRestart: covers a changed nickname, and a key store emptied under an unchanged configuration, where nothing but the missing key asks for administrative action. The result is SUCCESS and carries adminActionRequired, the warning and the reason. The handler keeps serving TLS for 3 s, and no Disabling is logged;
• httpHandlerDisabledAtItsStartStaysDownOnAChangeStillWithoutAKey: the port stays closed for 3 s after the apply;
• httpHandlerDisabledAtItsStartStartsOnAChangeThatGivesItAKey: the check accepts a change to server-cert, and after the apply the handler serves TLS.

Results:

• against the HTTPConnectionHandler of the head of [#1109] Keep a connection handler listening when a change to it is rejected #1110, 9 of the 11 new cases fail, each for the reason it pins:

  • the check accepts all six configurations;
  • the apply result for a changed nickname carries no warning;
  • the apply result for an emptied key store asks for no administrative action;
  • the handler disabled at its start listens after the apply.

  The case with one of the aliases present and the case with a key given back pass there too, because they pin behaviour that already exists. With the change, 24/24 pass, and HTTPConnectionHandlerTestCase passes;

• 14 mutants of HTTPConnectionHandler, each run as a separate class run, are each caught:

  • the check accepting a configuration without a key;
  • in the apply: enabled taken from the configuration, disabling at once as the LDAP handlers do, no warning in the result, no administrative action (caught only by the emptied key store case), no reasons in the result, and Disabling logged;
  • the start not disabling;
  • friendlyName in place of config.name() in each of the three reasons (caught by the new-instance cases);
  • a partly missing alias set counted as no key;
  • an empty key store not counted;
  • a missing provider not counted;

• package with attach-javadocs (doclint) passes.

Once, handlerWithoutItsCertificateDoesNotListen[LDAP2] failed with Address already in use on the port findFreePort had returned. No case of the class held that port, and it passed on the rerun. That is the known race of the port scan, not this change.

Related

• A rejected change to the LDAPS connection handler stops the handler listening until a later change is accepted or the server restarts #1109 / PR [#1109] Keep a connection handler listening when a change to it is rejected #1110: the check no longer disables a running handler. This PR builds on it.
• The HTTP connection handler and the JMX RMI connector ignore listen-address and listen on every interface #1112: the HTTP handler binds Grizzly to the wildcard address and ignores listen-address.

[maximthomas]

praise: The change is measured to pin what it fixes.

• Against the HTTPConnectionHandler of #1110's head (b4b0cad), RejectedSSLConfigurationChangeTestCase at 3dd05f8 fails 9 of 24, exactly the nine cases the description names and each for the reason it gives; the other 15 pass. At the head it passes 24/24 on CI (build-maven ubuntu-latest, 11).
• enabled = config.isEnabled() && (noKeyReasons.isEmpty() || isListening()) (HTTPConnectionHandler.java:263) keeps a running handler serving and a handler that is down for want of a key down, in one line.
• The reasons name config.name(), so the check on the new instance of an added or enabled handler (where friendlyName is null) still names it, and the new-instance rows pin that.