You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "nickva (via GitHub)" <gi...@apache.org> on 2023/06/02 17:31:55 UTC

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

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


##########
src/couch_replicator/src/couch_replicator_utils.erl:
##########
@@ -276,6 +278,98 @@ 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("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 ->
+            VerifyEnabled = config:get_boolean("replicator", "verify_ssl_certificates", false),
+            CACertFile = config:get("replicator", "ssl_trusted_certificates_file"),

Review Comment:
   We can check if it's a TSL configuration with `{is_ssl, true}` so we don't have to re-parse the URL and if `{verify, verify_none}` is set. But you're right, we'd still have to fetch `CACertFile = config:get("replicator", "ssl_trusted_certificates_file")` separately as we don't keep that bit around. (Keeping the cacertfile result in the ssl options even with `verify_none` could also be an option but there is a chance we might break the fragile ssl app options if we pass a {cacertfile, undefined} into it).
   



-- 
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