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