[GitHub] [couchdb] nickva commented on a diff in pull request #4625: Add optional logging of security issues when replicating

["nickva (via GitHub)" <gi...@apache.org>]

nickva commented on code in PR #4625:
URL: https://github.com/apache/couchdb/pull/4625#discussion_r1207021498


##########
src/couch_replicator/src/couch_replicator_utils.erl:
##########
@@ -276,6 +278,88 @@ seq_encode(Seq) ->
     % object. We are being maximally compatible here.
     ?JSON_ENCODE(Seq).
 
+%% Log uses of http protocol and uses of https protocol where verify_peer
+%% would fail.
+log_security_warnings(#rep{} = Rep) ->

Review Comment:
   A minor nit: `security` seems a bit too general, we're not checking all possible security issues, just the endpoint protocols.



##########
src/couch_replicator/src/couch_replicator_utils.erl:
##########
@@ -276,6 +278,88 @@ seq_encode(Seq) ->
     % object. We are being maximally compatible here.
     ?JSON_ENCODE(Seq).
 
+%% Log uses of http protocol and uses of https protocol where verify_peer
+%% would fail.
+log_security_warnings(#rep{} = Rep) ->
+    case config:get_boolean("couch_replicator", "log_security_warnings", false) of
+        false ->
+            ok;
+        true ->
+            ok = check_security(Rep, source),
+            ok = check_security(Rep, target)
+    end.
+
+check_security(#rep{} = Rep, Type) ->
+    Url =
+        case Type of
+            source -> Rep#rep.source#httpdb.url;
+            target -> Rep#rep.target#httpdb.url
+        end,
+    #url{protocol = Protocol} = ibrowse_lib:parse_url(Url),
+    case Protocol of
+        http ->
+            couch_log:warning("**security warning** replication ~s has insecure ~s at ~s", [
+                rep_principal(Rep), Type, Url
+            ]),
+            ok;
+        https ->
+            CACertFile = config:get("couch_replicator", "cacertfile"),
+            case CACertFile of
+                undefined ->
+                    couch_log:warning("security warnings enabled but no cacertfile configured", []);
+                CACertFile ->
+                    try
+                        ibrowse:send_req(Url, [], head, [], [

Review Comment:
   Here we'd be blocking the scheduler when a node boot as each job tries to make ibrowse calls. Also, in the replicator we typically use our own connection pool and don't rely on ibrowse for that.
   
   It's good that we do a try...catch but some requests start timing out at a minute or so, it would still block for too long and cause the scheduler to misbehave.



##########
src/couch_replicator/src/couch_replicator_utils.erl:
##########
@@ -276,6 +278,88 @@ seq_encode(Seq) ->
     % object. We are being maximally compatible here.
     ?JSON_ENCODE(Seq).
 
+%% Log uses of http protocol and uses of https protocol where verify_peer
+%% would fail.
+log_security_warnings(#rep{} = Rep) ->
+    case config:get_boolean("couch_replicator", "log_security_warnings", false) of
+        false ->
+            ok;
+        true ->
+            ok = check_security(Rep, source),
+            ok = check_security(Rep, target)
+    end.
+
+check_security(#rep{} = Rep, Type) ->
+    Url =
+        case Type of
+            source -> Rep#rep.source#httpdb.url;
+            target -> Rep#rep.target#httpdb.url
+        end,
+    #url{protocol = Protocol} = ibrowse_lib:parse_url(Url),
+    case Protocol of
+        http ->
+            couch_log:warning("**security warning** replication ~s has insecure ~s at ~s", [
+                rep_principal(Rep), Type, Url
+            ]),
+            ok;
+        https ->
+            CACertFile = config:get("couch_replicator", "cacertfile"),

Review Comment:
   This should be `ssl_trusted_certificates_file`? https://docs.couchdb.org/en/stable/config/replicator.html#replicator/ssl_trusted_certificates_file
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org