generateFileHash() - If we can't generate a secure, then don't generate any token
authorTim Otten <totten@civicrm.org>
Tue, 12 Feb 2019 23:58:57 +0000 (15:58 -0800)
committerSeamus Lee <seamuslee001@gmail.com>
Fri, 22 Feb 2019 00:15:14 +0000 (11:15 +1100)
Falling back to a constant negates any security benefit of using a hash.

IMHO, the edge-case where `CIVICRM_SITE_KEY` is missing should be
obscure/rare and signifies broader problems for the deployment.  It needs to
be corrected.  If you're worried that having an error-symptom here is too
obscure, then let's add a more prominent error-message via
`CRM_Utils_Check`.

NOTE: There is one pre-existing case in core where (in absence of a key) it
procedes with a constant in lieu of a `CIVICRM_SITE_KEY` .  Specifically,
`CRM_Core_Error::generateLogFileHash()`.  That is not a good example to
follow because it is qualitiatively different:

* In `generateLogFileHash`(), `CIVICRM_SITE_KEY` functions as one of
  multiple redundant security mechanisms -- e.g.  even if
  `CIVICRM_SITE_KEY` is missing, the log file remains hard-to-access because
  (1) the DSN is part of the hash and (2) the httpd protects `ConfigAndLog`.
  (Contrast: The file-hash-code is not *redundant* in the same way.)

* In the context of logging, raising any error (even if it's real error
  condition) can provoke a weird loop (because then that error needs to be
  logged).  The log needs to avoid such loops.  (Contrast:
  `generateFileHash()` is part of the normal post-boot application logic, so
  it's free to register errors normally.)

CRM/Core/BAO/File.php

index 4468dd8c0f2da465d58767675902703c09170074..30e26b9a29b707a29f153faae8e2edb469d1614f 100644 (file)
@@ -775,7 +775,10 @@ AND       CEF.entity_id    = %2";
    */
   public static function generateFileHash($eid = NULL, $fid = NULL, $genTs = NULL, $life = NULL) {
     // Use multiple (but stable) inputs for hash information.
-    $siteKey = defined('CIVICRM_SITE_KEY') ? CIVICRM_SITE_KEY : 'NO_SITE_KEY';
+    $siteKey = CRM_Utils_Constant::value('CIVICRM_SITE_KEY');
+    if (!$siteKey) {
+      throw new \CRM_Core_Exception("Cannot generate file access token. Please set CIVICRM_SITE_KEY.");
+    }
 
     if (!$genTs) {
       $genTs = time();