Improve error handling for missing files (#2551)

`os.path.exists` doesn't allow us to distinguish between permissions errors and
the path actually not existing, which repeatedly confuses people. It also means
that we try to overwrite existing key files, which is super-confusing. (cf
issues #2455, #2379). Use os.stat instead.

Also, don't recomemnd the the use of --generate-config, which screws everything
up if you're using debian (cf #2455).
This commit is contained in:
Richard van der Hoff 2017-10-17 14:46:17 +01:00 committed by GitHub
parent dbdfd8967d
commit 7216c76654
3 changed files with 33 additions and 17 deletions

View File

@ -81,22 +81,38 @@ class Config(object):
def abspath(file_path):
return os.path.abspath(file_path) if file_path else file_path
@classmethod
def path_exists(cls, file_path):
"""Check if a file exists
Unlike os.path.exists, this throws an exception if there is an error
checking if the file exists (for example, if there is a perms error on
the parent dir).
Returns:
bool: True if the file exists; False if not.
"""
try:
os.stat(file_path)
return True
except OSError as e:
if e.errno != errno.ENOENT:
raise e
return False
@classmethod
def check_file(cls, file_path, config_name):
if file_path is None:
raise ConfigError(
"Missing config for %s."
" You must specify a path for the config file. You can "
"do this with the -c or --config-path option. "
"Adding --generate-config along with --server-name "
"<server name> will generate a config file at the given path."
% (config_name,)
)
if not os.path.exists(file_path):
try:
os.stat(file_path)
except OSError as e:
raise ConfigError(
"File %s config for %s doesn't exist."
" Try running again with --generate-config"
% (file_path, config_name,)
"Error accessing file '%s' (config for %s): %s"
% (file_path, config_name, e.strerror)
)
return cls.abspath(file_path)
@ -248,7 +264,7 @@ class Config(object):
" -c CONFIG-FILE\""
)
(config_path,) = config_files
if not os.path.exists(config_path):
if not cls.path_exists(config_path):
if config_args.keys_directory:
config_dir_path = config_args.keys_directory
else:
@ -261,7 +277,7 @@ class Config(object):
"Must specify a server_name to a generate config for."
" Pass -H server.name."
)
if not os.path.exists(config_dir_path):
if not cls.path_exists(config_dir_path):
os.makedirs(config_dir_path)
with open(config_path, "wb") as config_file:
config_bytes, config = obj.generate_config(

View File

@ -118,10 +118,9 @@ class KeyConfig(Config):
signing_keys = self.read_file(signing_key_path, "signing_key")
try:
return read_signing_keys(signing_keys.splitlines(True))
except Exception:
except Exception as e:
raise ConfigError(
"Error reading signing_key."
" Try running again with --generate-config"
"Error reading signing_key: %s" % (str(e))
)
def read_old_signing_keys(self, old_signing_keys):
@ -141,7 +140,8 @@ class KeyConfig(Config):
def generate_files(self, config):
signing_key_path = config["signing_key_path"]
if not os.path.exists(signing_key_path):
if not self.path_exists(signing_key_path):
with open(signing_key_path, "w") as signing_key_file:
key_id = "a_" + random_string(4)
write_signing_keys(

View File

@ -126,7 +126,7 @@ class TlsConfig(Config):
tls_private_key_path = config["tls_private_key_path"]
tls_dh_params_path = config["tls_dh_params_path"]
if not os.path.exists(tls_private_key_path):
if not self.path_exists(tls_private_key_path):
with open(tls_private_key_path, "w") as private_key_file:
tls_private_key = crypto.PKey()
tls_private_key.generate_key(crypto.TYPE_RSA, 2048)
@ -141,7 +141,7 @@ class TlsConfig(Config):
crypto.FILETYPE_PEM, private_key_pem
)
if not os.path.exists(tls_certificate_path):
if not self.path_exists(tls_certificate_path):
with open(tls_certificate_path, "w") as certificate_file:
cert = crypto.X509()
subject = cert.get_subject()
@ -159,7 +159,7 @@ class TlsConfig(Config):
certificate_file.write(cert_pem)
if not os.path.exists(tls_dh_params_path):
if not self.path_exists(tls_dh_params_path):
if GENERATE_DH_PARAMS:
subprocess.check_call([
"openssl", "dhparam",