Merge pull request #2715 from cyberark/onyx-31936-config-directory-pe…

…rmission

Write log on Conjur config permission issues

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/errors.rb

@@ -1,5 +1,12 @@
 # frozen_string_literal: true
 
+# This file maintains the collection of possible errors emitted by Conjur using
+# the standard numbering scheme.
+#
+# For the next available code, use the command `rake error_code:next` in the
+# repo root.
+#
+# See also ./logs.rb
 module Errors
   module Conjur
 

app/domain/logs.rb

@@ -1,5 +1,15 @@
 # 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.
+#
+# For the next available code, use the command `rake error_code:next` in the
+# repo root.
+#
+# See also ./errors.rb
 module LogMessages
 
   module Conjur
@@ -787,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/application.rb

@@ -47,15 +47,15 @@ class Application < Rails::Application
       Sequel.extension(:core_extensions, :postgres_schemata)
       Sequel::Model.db.extension(:pg_array, :pg_inet)
     end
-    
+
     #The default connection pool does not support closing connections.
     # We must be able to close connections on demand to clear the connection cache
     # after policy loads [cyberark/conjur#2584](https://github.com/cyberark/conjur/pull/2584)
     # The [ShardedThreadedConnectionPool](https://www.rubydoc.info/github/jeremyevans/sequel/Sequel/ShardedThreadedConnectionPool) does support closing connections on-demand.
     # Sequel is configured to use the ShardedThreadedConnectionPool by setting the servers configuration on
     # the database connection [docs](https://www.rubydoc.info/github/jeremyevans/sequel/Sequel%2FShardedThreadedConnectionPool:servers)
     config.sequel.servers = {}
-    
+
     config.encoding = "utf-8"
     config.active_support.escape_html_entities_in_json = true
 
@@ -73,14 +73,5 @@ class Application < Rails::Application
     config.anyway_config.future.unwrap_known_environments = true
 
     config.anyway_config.default_config_path = "/etc/conjur/config"
-
-    # Create a single instance of the ConjurConfig object for this process that
-    # loads configuration on server startup. This prevents config values from
-    # being loaded fresh every time a ConjurConfig object is instantiated, which
-    # could lead to inconsistent behavior.
-    #
-    # 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
   end
 end

config/initializers/conjur_config.rb

@@ -0,0 +1,9 @@
+Rails.application.configure do
+  # Create a single instance of the ConjurConfig object for this process that
+  # loads configuration on server startup. This prevents config values from
+  # being loaded fresh every time a ConjurConfig object is instantiated, which
+  # could lead to inconsistent behavior.
+  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,88 @@ 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)
+
+      # 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

lib/tasks/next_available_error.rake

@@ -1,8 +1,13 @@
 require 'util/error_code'
 
+# error_code:next is used to find the next available error or log code ID for
+# Conjur standard logging.
 namespace :error_code do
   task :next do
-    error_code = Error::ConjurCode.new('./app/domain/errors.rb')
+    error_code = Error::ConjurCode.new(
+      './app/domain/errors.rb',
+      './app/domain/logs.rb'
+    )
     error_code.print_next_available
   end
 end

lib/util/error_code.rb

@@ -1,20 +1,37 @@
 # frozen_string_literal: true
 
+require 'logger'
+
 module Error
   # helps get information regarding current error codes.
   class ConjurCode
-    def initialize(path)
-      @path = path
-      validate
+    def initialize(
+      *paths,
+      # Injected dependencies
+      logger: Logger.new($stdout),
+      output: $stdout
+    )
+      @logger = logger
+      @output = output
+
+      @paths = valid_paths(paths)
     end
 
     def print_next_available
       id = next_code_id
+
       unless id
-        $stderr.puts("The path doesn't contain any files with Conjur error codes")
+        @logger.error(
+          "The path doesn't contain any files with Conjur error codes"
+        )
         return
       end
-      puts(format("The next available error number is %d ( CONJ%05dE )", id, id))
+
+      @output.puts(
+        format("The next available error number is %d ( CONJ%05d )", id, id)
+      )
+
+      id
     end
 
     # separate data-gathering from printing
@@ -23,18 +40,26 @@ def next_code_id
       max_code ? max_code + 1 : nil
     end
 
+    # This is reported as :reek:NestedIterators, but splitting this apart
+    # does not make it more readable.
     def existing_codes
-      codes = File.foreach(@path).map do |line|
-        match = /code: "CONJ(?<num>[0-9]+)E"/.match(line)
-        match ? match[:num].to_i : nil
-      end
-      codes.compact
+      # For each file given
+      @paths.map do |path|
+        # Convert the lines into codes, if present
+        File.foreach(path).map do |line|
+          match = /code: "CONJ(?<num>[0-9]+)[DIWE]"/.match(line)
+          match ? match[:num].to_i : nil
+        end.compact
+      end.flatten
     end
 
-    def validate
-      return if File.file?(@path)
+    def valid_paths(paths)
+      paths.select do |path|
+        next true if File.file?(path)
 
-      raise format("The following path:%s was not found", @path)
+        @logger.warn("The following path was not found: #{path}")
+        false
+      end
     end
   end
 end