Add tunables for TLS versions allowed and whether to check CRLs

The default is to only allow TLS 1.2 and to check the CRL of any client certificates. This addresses some of the concerns in basho/riak#433
basho · Jan 6, 2014 · 09cc769 · 09cc769
1 parent 330110c
commit 09cc769
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 5 deletions.
diff --git a/priv/riak_api.schema b/priv/riak_api.schema
@@ -103,3 +103,72 @@
      end
    end
 }.
+
+%% @doc Determine which SSL/TLS versions are allowed. By default only TLS 1.2
+%% is allowed, but other versions can be enabled if clients don't support the
+%% latest TLS standard. It is *strongly* recommended that SSLv3 is not enabled
+%% unless absolutely necessary. More than one protocol can be enabled at once.
+{mapping, "tls_protocols.sslv3", "riak_api.tls_protocols", [
+  {datatype, {enum, [on, off]}},
+  {default, off}
+]}.
+
+{mapping, "tls_protocols.tlsv1", "riak_api.tls_protocols", [
+  {datatype, {enum, [on, off]}},
+  {default, off}
+]}.
+
+{mapping, "tls_protocols.tlsv1.1", "riak_api.tls_protocols", [
+  {datatype, {enum, [on, off]}},
+  {default, off}
+]}.
+
+{mapping, "tls_protocols.tlsv1.2", "riak_api.tls_protocols", [
+  {datatype, {enum, [on, off]}},
+  {default, on}
+]}.
+
+{ translation,
+  "riak_api.tls_protocols",
+    fun(Conf) ->
+        Protocols = cuttlefish_util:filter_by_variable_starts_with("tls_protocols", Conf),
+        [begin
+             case Key of
+                 ["tls_protocols","sslv3"] ->
+                     sslv3;
+                 ["tls_protocols","tlsv1"] ->
+                     tlsv1;
+                 ["tls_protocols","tlsv1", "1"] ->
+                     'tlsv1.1';
+                 ["tls_protocols","tlsv1", "2"] ->
+                     'tlsv1.2'
+             end
+         end
+         || {Key, Value} <- Protocols, Value == on]
+    end
+}.
+
+%% @doc Whether to check the CRL of a client certificate. This defaults to
+%% true but some CAs may not maintain or define a CRL, so this can be disabled
+%% if no CRL is available.
+{mapping, "check_crl", "riak_api.check_crl", [
+  {datatype, {enum, [on, off]}},
+  {default, on}
+]}.
+
+{translation,
+  "riak_api.check_crl",
+   fun(Conf) ->
+     OTPVer = erlang:system_info(otp_release),
+     CheckCRL = cuttlefish_util:conf_get_value("check_crl", Conf),
+     %% CRL checking is broken in mainline OTP as of R16B02, so Riak will ship
+     %% with a patched SSL/public_key to fix it. This means that we don't want
+     %% to pass this option to a vanilla OTP.
+     case {CheckCRL, string:str(OTPVer, "basho")} of
+       {_, 0} -> false;
+       {on, _} -> true;
+       {off, _} -> false
+     end
+   end
+}.
+
diff --git a/src/riak_api_pb_server.erl b/src/riak_api_pb_server.erl
@@ -129,18 +129,27 @@ wait_for_tls({msg, MsgCode, _MsgData}, State=#state{socket=Socket,
                                                                       keyfile)},
                                          {cacerts, CACerts},
                                          {ciphers, Ciphers},
+                                         {versions,
+                                          app_helper:get_env(riak_api,
+                                                             tls_protocols,
+                                                             ['tlsv1.2'])},
                                          %% force peer validation, even though
                                          %% we don't care if the peer doesn't
                                          %% send a certificate
                                          {verify, verify_peer},
-                                         {verify_fun, {fun validate_function/3,
-                                                       {CACerts, []}}},
                                          {reuse_sessions, false} %% required!
                                         ] ++
                                         %% conditionally include the honor cipher order, don't pass it if it
                                         %% disabled because it will crash unpatched OTP
                                         [{honor_cipher_order, true} ||
-                                         app_helper:get_env(riak_api, honor_cipher_order, false) ]
+                                         app_helper:get_env(riak_api,
+                                                            honor_cipher_order,
+                                                            false) ] ++
+                                        [{verify_fun, {fun validate_function/3,
+                                                       {CACerts, []}}} ||
+                                         app_helper:get_env(riak_api,
+                                                            check_crl, false)]
+
                                ) of
                 {ok, NewSocket} ->
                     CommonName = case ssl:peercert(NewSocket) of

diff --git a/src/riak_api_web.erl b/src/riak_api_web.erl
@@ -67,11 +67,16 @@ spec_from_binding(https, Name, {Ip, Port}) ->
     SslOpts = app_helper:get_env(riak_core, ssl,
                                  [{certfile, filename:join(Etc, "cert.pem")},
                                   {keyfile, filename:join(Etc, "key.pem")}]) 
-        %% conditionally include the honor cipher order, don't pass it if it
+        %% Conditionally include the honor cipher order, don't pass it if it
         %% disabled because it will crash unpatched OTP
         ++ [{honor_cipher_order, true} ||
             app_helper:get_env(riak_api, honor_cipher_order, false) ]
-        ++ [{ciphers, Ciphers}],
+        ++ [{ciphers, Ciphers},
+            {versions,
+             app_helper:get_env(riak_api,
+                                tls_protocols,
+                                ['tlsv1.2'])}
+           ],
 
     lists:flatten([{name, Name},
                    {ip, Ip},