LGTM.

Thanks,
Jose

On Thu, Feb 06, 2014 at 02:54:29PM +0100, Santi Raffa wrote:
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 4072df4..35bb3c6 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -4766,6 +4766,6 @@ privateParametersBlacklist = [ "osparams_private"
> 
>  -- | Warn the user that the logging level is too low for production use.
>  debugModeConfidentialityWarning :: String
> -debugModeConfidentialityWarning = "ALERT: %s started in debug mode.\n\
> -                                  \ Private and secret parameters WILL\
> -                                  \ be logged!\n"
> +debugModeConfidentialityWarning =
> +  "ALERT: %s started in debug mode.\n\
> +  \ Private and secret parameters WILL be logged!\n"
> diff --git a/lib/daemon.py b/lib/daemon.py
> index efe22bf..b6e2915 100644
> --- a/lib/daemon.py
> +++ b/lib/daemon.py
> @@ -671,7 +671,7 @@ def GenericMain(daemon_name, optionparser,
>                  check_fn, prepare_fn, exec_fn,
>                  multithreaded=False, console_logging=False,
>                  default_ssl_cert=None, default_ssl_key=None,
> -                warn_about_private_values_and_debug_mode=False):
> +                warn_breach=False):
>    """Shared main function for daemons.
> 
>    @type daemon_name: string
> @@ -698,10 +698,10 @@ def GenericMain(daemon_name, optionparser,
>    @param default_ssl_cert: Default SSL certificate path
>    @type default_ssl_key: string
>    @param default_ssl_key: Default SSL key path
> -  @type warn_about_private_values_and_debug_mode: bool
> -  @param warn_about_private_values_and_debug_mode: issue a warning at daemon
> -      launch time, before daemonizing, about the possibility of breaking
> -      parameter privacy invariants through otherwise helpful debug logging.
> +  @type warn_breach: bool
> +  @param warn_breach: issue a warning at daemon launch time, before
> +      daemonizing, about the possibility of breaking parameter privacy
> +      invariants through otherwise helpful debug logging.
> 
>    """
>    optionparser.add_option("-f", "--foreground", dest="fork",
> @@ -806,7 +806,7 @@ def GenericMain(daemon_name, optionparser,
>    log_filename = constants.DAEMONS_LOGFILES[daemon_name]
> 
>    # node-deamon logging in lib/http/server.py, _HandleServerRequestInner
> -  if options.debug and warn_about_private_values_and_debug_mode:
> +  if options.debug and warn_breach:
>      sys.stderr.write(constants.DEBUG_MODE_CONFIDENTIALITY_WARNING %
> daemon_name)
> --- a/lib/server/noded.py
> +++ b/lib/server/noded.py
> @@ -1298,4 +1298,4 @@ def Main():
>                       default_ssl_cert=pathutils.NODED_CERT_FILE,
>                       default_ssl_key=pathutils.NODED_CERT_FILE,
>                       console_logging=True,
> -                     warn_about_private_values_and_debug_mode=True)
> +                     warn_breach=True)
> diff --git a/src/Ganeti/UDSServer.hs b/src/Ganeti/UDSServer.hs
> index 4fd4c78..42dc42c 100644
> --- a/src/Ganeti/UDSServer.hs
> +++ b/src/Ganeti/UDSServer.hs
> @@ -58,14 +58,14 @@ module Ganeti.UDSServer
>  import Control.Applicative
>  import Control.Concurrent (forkIO)
>  import Control.Exception (catch)
> -import Data.IORef
> -import Data.List
> +import Control.Monad
>  import qualified Data.ByteString as B
>  import qualified Data.ByteString.Lazy as BL
>  import qualified Data.ByteString.UTF8 as UTF8
>  import qualified Data.ByteString.Lazy.UTF8 as UTF8L
> +import Data.IORef
> +import Data.List
>  import Data.Word (Word8)
> -import Control.Monad
>  import qualified Network.Socket as S
>  import System.Directory (removeFile)
>  import System.IO (hClose, hFlush, hWaitForInput, Handle, IOMode(..))
> diff --git a/src/Ganeti/Logging.hs b/src/Ganeti/Logging.hs
> index abb8d46..4cc5c62 100644
> --- a/src/Ganeti/Logging.hs
> +++ b/src/Ganeti/Logging.hs
> @@ -50,6 +50,7 @@ module Ganeti.Logging
>    , isDebugMode
>    ) where
> 
> +import Control.Applicative ((<$>))
>  import Control.Monad
>  import Control.Monad.Error (Error(..), MonadError(..), catchError)
>  import Control.Monad.Reader
> @@ -179,9 +180,7 @@ logEmergency = logAt EMERGENCY
>  -- | Check if the logging is at DEBUG level.
>  -- DEBUG logging is unacceptable for production.
>  isDebugMode :: IO Bool
> -isDebugMode = do
> -  logger <- getRootLogger
> -  return $ getLevel logger == Just DEBUG
> +isDebugMode = (Just DEBUG ==) . getLevel <$> getRootLogger
> 
>  -- * Logging in an error monad with rethrowing errors
> diff --git a/src/Ganeti/UDSServer.hs b/src/Ganeti/UDSServer.hs
> index 42dc42c..a1622d1 100644
> --- a/src/Ganeti/UDSServer.hs
> +++ b/src/Ganeti/UDSServer.hs
> @@ -351,6 +351,11 @@ handleRawMessage handler payload =
>          let (status, response) = prepareMsg call_result_json
>          return (close, buildResponse status response)
> 
> +isRisky :: RecvResult -> Bool
> +isRisky msg = case msg of
> +  RecvOk payload -> any (`isInfixOf` payload) privateParametersBlacklist
> +  _ -> False
> +
>  -- | Reads a request, passes it to a handler and sends a response back to the
>  -- client.
>  handleClient
> @@ -378,10 +383,6 @@ handleClient handler client = do
>        sendMsg client outMsg
>        return close
> 
> -isRisky :: RecvResult -> Bool
> -isRisky msg = case msg of
> -  RecvOk payload -> any (`isInfixOf` payload) privateParametersBlacklist
> -  _ -> False
> 
>  -- | Main client loop: runs one loop of 'handleClient', and if that
>  -- doesn't report a finished (closed) connection, restarts itself.
> 
> 
> 
> -- 
> Raffa Santi
> Google Germany GmbH
> Dienerstr. 12
> 80331 München
> 
> 
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
> Geschäftsführer: Graham Law, Christine Elizabeth Flores

-- 
Jose Antonio Lopes
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to