Skip to content

Commit

Permalink
ipa-backup: Make sure all roles are installed on the current master.
Browse files Browse the repository at this point in the history
ipa-backup does not check whether the IPA master it is running on has
all used roles installed. This can lead into situations where backups
are done on a CAless or KRAless host while these roles are used in the
IPA cluster. These backups cannot be used to restore a complete cluster.

With this change, ipa-backup refuses to execute if the roles installed
on the current host do not match the list of roles used in the cluster.
A --disable-role-check knob is provided to restore the previous behavior.

Fixes: https://pagure.io/freeipa/issue/8217
Signed-off-by: François Cami <fcami@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Mohammad Rizwan Yusuf <myusuf@redhat.com>
  • Loading branch information
fcami committed Apr 1, 2020
1 parent 9324bba commit 3665ba9
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 10 deletions.
5 changes: 4 additions & 1 deletion install/tools/man/ipa-backup.1
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ Include the IPA service log files in the backup.
\fB\-\-online\fR
Perform the backup on\-line. Requires the \-\-data option.
.TP
\fB\-\-disable\-role\-check\fR
Perform the backup even if this host does not have all the roles in use in the cluster. This is not recommended.
.TP
\fB\-\-v\fR, \fB\-\-verbose\fR
Print debugging information
.TP
Expand Down Expand Up @@ -85,4 +88,4 @@ The log file for backups
.PP
.SH "SEE ALSO"
.BR ipa\-restore(1)
.BR gpg2(1)
.BR gpg2(1)
90 changes: 81 additions & 9 deletions ipaserver/install/ipa_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,16 @@ def add_options(cls, parser):
"--online", dest="online", action="store_true",
default=False,
help="Perform the LDAP backups online, for data only.")

parser.add_option(
"--disable-role-check", dest="rolecheck", action="store_false",
default=True,
help="Perform the backup even if this host does not have all "
"the roles used in the cluster. This is not recommended."
)

def setup_logging(self, log_file_mode='a'):
super(Backup, self).setup_logging(log_file_mode='a')


def validate_options(self):
options = self.options
super(Backup, self).validate_options(needs_root=True)
Expand All @@ -279,7 +283,6 @@ def validate_options(self):
self.option_parser.error("You cannot specify --data "
"with --logs")


def run(self):
options = self.options
super(Backup, self).run()
Expand Down Expand Up @@ -312,6 +315,8 @@ def run(self):

self.get_connection()

self.check_roles(raiseonerr=options.rolecheck)

self.create_header(options.data_only)
if options.data_only:
if not options.online:
Expand Down Expand Up @@ -358,6 +363,79 @@ def run(self):
logger.error('Cannot change directory to %s: %s', cwd, e)
shutil.rmtree(self.top_dir)

def check_roles(self, raiseonerr=True):
"""Check that locally-installed roles match the globally used ones.
Specifically: make sure no role used in the cluster is absent
from the local replica ipa-backup is running on.
"""

locally_installed_roles = set()
globally_used_roles = set()

# We need to cover the following roles:
# * DNS: filter="(|(cn=DNS)(cn=DNSKeySync))"
# * CA: filter="(cn=CA)"
# * KRA: filter="(cn=KRA)"
# * AD Trust Controller: filter="(cn=ADTRUST)"
# Note:
# We do not need to worry about AD Trust Agents as Trust
# Controllers are Trust Agents themselves and contain extra,
# necessary Samba configuration. So either the cluster has no
# AD Trust bits installed, or it should be backuped on a Trust
# Controller, not a Trust Agent.
role_names = {
'CA', 'DNS', 'DNSKeySync', 'KRA', 'ADTRUST'
}

search_base = DN(api.env.container_masters, api.env.basedn)
attrs_list = ['ipaconfigstring', 'cn']

for role in role_names:
search_filter = '(cn=%s)' % role
try:
masters = dict()
result = self._conn.get_entries(
search_base,
filter=search_filter,
attrs_list=attrs_list,
scope=self._conn.SCOPE_SUBTREE
)
masters[role] = {e.dn[1]['cn'] for e in result}

if api.env.host in masters[role]:
locally_installed_roles.add(role)
if masters[role] is not None:
globally_used_roles.add(role)
except errors.EmptyResult:
pass

if locally_installed_roles == globally_used_roles:
logger.info(
"Local roles match globally used roles, proceeding."
)
else:
if raiseonerr:
raise admintool.ScriptError(
'Error: Local roles %s do not match globally used '
'roles %s. A backup done on this host would not be '
'complete enough to restore a fully functional, '
'identical cluster.' % (
', '.join(sorted(locally_installed_roles)),
', '.join(sorted(globally_used_roles))
)
)
else:
msg = (
'Warning: Local roles %s do not match globally used roles '
'%s. A backup done on this host would not be complete '
'enough to restore a fully functional, identical cluster. '
'Proceeding as role check was explicitly disabled.' % (
', '.join(sorted(locally_installed_roles)),
', '.join(sorted(globally_used_roles))
)
)
logger.info(msg)

def add_instance_specific_data(self):
'''
Expand Down Expand Up @@ -387,7 +465,6 @@ def add_instance_specific_data(self):

self.logs.append(paths.VAR_LOG_DIRSRV_INSTANCE_TEMPLATE % serverid)


def get_connection(self):
'''
Create an ldapi connection and bind to it using autobind as root.
Expand All @@ -405,7 +482,6 @@ def get_connection(self):

return self._conn


def db2ldif(self, instance, backend, online=True):
'''
Create a LDIF backup of the data in this instance.
Expand Down Expand Up @@ -481,7 +557,6 @@ def db2ldif(self, instance, backend, online=True):
'Unexpected error: %s' % e
)


def db2bak(self, instance, online=True):
'''
Create a BAK backup of the data and changelog in this instance.
Expand Down Expand Up @@ -545,7 +620,6 @@ def db2bak(self, instance, online=True):
'Unexpected error: %s' % e
)


def file_backup(self, options):

def verify_directories(dirs):
Expand Down Expand Up @@ -611,7 +685,6 @@ def compress_file_backup(self):
# Rename the archive back to files.tar to preserve compatibility
os.rename(os.path.join(self.dir, 'files.tar.gz'), self.tarfile)


def create_header(self, data_only):
'''
Create the backup file header that contains the meta data about
Expand Down Expand Up @@ -649,7 +722,6 @@ def create_header(self, data_only):
with open(self.header, 'w') as fd:
config.write(fd)


def finalize_backup(self, data_only=False, encrypt=False, keyring=None):
'''
Create the final location of the backup files and move the files
Expand Down

0 comments on commit 3665ba9

Please sign in to comment.