Log when Conjur config permissions are incorrect

CHANGELOG.md

@@ -11,6 +11,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [1.19.3] - 2023-01-26
 
+### Added
+- Conjur now logs when it detects that the Conjur configuration file
+  (conjur.yml) or directory permissions prevent the Conjur server from
+  successfully reading it. Conjur also now logs at the DEBUG level when it
+  detects that either the directory or file do not exist.
+  [cyberark/conjur#2715](https://github.com/cyberark/conjur/pull/2715)
+
 ## [1.19.2] - 2022-01-13
 
 ### Fixed

app/domain/logs.rb

@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
+require_relative 'util/trackable_log_message_class'
+require_relative 'util/trackable_error_class'
+
 # This file maintains the collection of possible logs emitted by Conjur using
 # the standard numbering scheme.
 #
@@ -794,4 +797,31 @@ module Util
     )
 
   end
+
+  module Config
+    DirectoryDoesNotExist = ::Util::TrackableLogMessageClass.new(
+      msg: "Conjur config directory doesn't exist or has " \
+           "insufficient permission to list it: {0-config-directory}",
+      code: "CONJ00147D"
+    )
+
+    DirectoryInvalidPermissions = ::Util::TrackableLogMessageClass.new(
+      msg: "Conjur config directory exists but is missing " \
+           "search/execute permission required to list the config file: " \
+           "{0-config-directory}",
+      code: "CONJ00148W"
+    )
+
+    FileDoesNotExist = ::Util::TrackableLogMessageClass.new(
+      msg: "Conjur config file doesn't exist or has insufficient " \
+           "permission to list it: {0-config-path}",
+      code: "CONJ00149D"
+    )
+
+    FileInvalidPermissions = ::Util::TrackableLogMessageClass.new(
+      msg: "Conjur config file exists but has insufficient permission to " \
+           "read it: {0-config-path}",
+      code: "CONJ00150W"
+    )
+  end
 end

app/domain/util/trackable_error_class.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'error_class'
+
 # A factory for creating an ErrorClass with an error code prefix
 #
 module Util

app/domain/util/trackable_log_message_class.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require_relative 'log_message_class'
+
 # A factory for creating a LogMessage with a code prefix
 #
 module Util

bin/conjur-cli.rb

@@ -7,11 +7,17 @@
 require 'uri'
 require 'open3'
 require 'conjur/conjur_config'
+require 'logger'
 
 require_relative './conjur-cli/commands'
 
 include GLI::App
 
+# Create a logger instance that may be passed to classes that take injected
+# dependencies. Because many commands output to stdout, we log to stderr.
+cli_logger = Logger.new($stderr)
+cli_logger.level = ENV['CONJUR_LOG_LEVEL'] || :info
+
 program_desc "Command and control application for Conjur"
 version File.read(File.expand_path("../VERSION", File.dirname(__FILE__)))
 arguments :strict
@@ -241,7 +247,9 @@
   Anyway::Settings.default_config_path = "/etc/conjur/config"
 
   begin
-    conjur_config = Conjur::ConjurConfig.new
+    conjur_config = Conjur::ConjurConfig.new(
+      logger: cli_logger
+    )
   rescue Conjur::ConfigValidationError => e
     $stderr.puts e
     exit 1

config/initializers/conjur_config.rb

@@ -6,5 +6,7 @@
   #
   # We create this in application.rb instead of an initializer so that it's
   # guaranteed to be available for other initializers to use.
-  config.conjur_config = Conjur::ConjurConfig.new
+  config.conjur_config = Conjur::ConjurConfig.new(
+    logger: Rails.logger
+  )
 end

lib/conjur/conjur_config.rb

@@ -2,6 +2,14 @@
 # rely on Rails autoloading to make the `Anyway::Config` constant available.
 require 'anyway_config'
 
+# It is a design smell that we have to require files from `app/domain` to use
+# in `/lib` (particularly that we traverse up to a common root directory). This
+# suggests that, if we want to continue using well-defined classes for logs and
+# errors, we should move those classes into `/lib` from `app/domain`. However,
+# that change greatly expands the scope of this PR and so, instead, I used a
+# relative require to include the log definitions here.
+require_relative '../../app/domain/logs'
+
 module Conjur
   # We are temporarily avoiding hooking into the application error system
   # because using it means you have to require about five classes when loading
@@ -33,8 +41,21 @@ class ConjurConfig < Anyway::Config
       extensions: []
     )
 
-    def initialize(*args)
-      super(*args)
+    def initialize(
+      *args,
+      logger: Rails.logger,
+      **kwargs
+    )
+      # The permissions checks emit log messages, so we need to initialize the
+      # logger before verifying permissions.
+      @logger = logger
+
+      # First verify that we have the permissions necessary to read the config
+      # file.
+      verify_config_is_readable
+
+      # Initialize Anyway::Config
+      super(*args, **kwargs)
 
     # If the config file is not a valid YAML document, we want
     # to raise a user-friendly ConfigValidationError rather than
@@ -101,6 +122,91 @@ def extensions=(val)
 
     private
 
+    def verify_config_is_readable
+      return unless verify_config_directory_exists
+
+      return unless verify_config_directory_permissions
+
+      return unless verify_config_file_exists
+
+      return unless verify_config_file_permissions
+
+      # If no issues are detected, log where the config file is being read from.
+      @logger.info("Loading Conjur config file: #{config_path}")
+    end
+
+    def verify_config_directory_exists
+      return true if File.directory?(config_directory)
+
+      # It may be expected that the config directory not exist, so this is
+      # a log message for debugging the Conjur config, rather than alerting the
+      # user to an issue.
+      @logger.debug(
+        LogMessages::Config::DirectoryDoesNotExist.new(config_directory).to_s
+      )
+      false
+    end
+
+    def verify_config_directory_permissions
+      return true if File.executable?(config_directory)
+
+      # require 'pry'
+      # binding.pry
+
+      # If the config direct does exist, we want to alert the user if the
+      # permissions will prevent Conjur from reading a config file in that
+      # directory.
+      @logger.warn(
+        LogMessages::Config::DirectoryInvalidPermissions.new(
+          config_directory
+        ).to_s
+      )
+
+      false
+    end
+
+    def verify_config_file_exists
+      return true if File.file?(config_path)
+
+      # Similar to the directory, if the config file doesn't exist, this becomes
+      # debugging information.
+      @logger.debug(
+        LogMessages::Config::FileDoesNotExist.new(config_path).to_s
+      )
+
+      false
+    end
+
+    def verify_config_file_permissions
+      return true if File.readable?(config_path)
+
+      # If the file exists but we can detect it's not readable, let the user
+      # know. Conjur will also fail to start in this configuration.
+      @logger.warn(
+        LogMessages::Config::FileInvalidPermissions.new(config_path).to_s
+      )
+
+      false
+    end
+
+    def config_directory
+      File.dirname(config_path)
+    end
+
+    # Because we call this before the base class initialized, we need to move the
+    # implementation here.
+    #
+    # Derived from:
+    # https://github.com/palkan/anyway_config/blob/83d79ccaf5619889c07c8ecdf8d66dcb22c9dc05/lib/anyway/config.rb#L358
+    #
+    # :reek:DuplicateMethodCall due to `self.class`
+    def config_path
+      @config_path ||= resolve_config_path(
+        self.class.config_name,
+        self.class.env_prefix
+      )
+    end
+
     def str_to_list(val)
       val.is_a?(String) ? val.split(',') : val
     end

spec/conjurctl/configuration_show_spec.rb

@@ -33,8 +33,8 @@
       "conjurctl configuration show --output invalid"
     )
 
-    expect(stderr).to eq(
-      "error: Unknown configuration output format 'invalid'\n"
+    expect(stderr).to include(
+      "error: Unknown configuration output format 'invalid'"
     )
   end
 
@@ -43,8 +43,8 @@
       "CONJUR_TRUSTED_PROXIES=boop conjurctl configuration show"
     )
 
-    expect(stderr).to eq(
-      "Invalid values for configured attributes: trusted_proxies\n"
+    expect(stderr).to include(
+      "Invalid values for configured attributes: trusted_proxies"
     )
 
     expect(status.exitstatus).to eq(1)