[Freeipa-devel] [PATCH] 365 Fix CA certificate backup and restore
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4711.
Honza
--
Jan Cholasta
From 5c00f80cce0e0952252df4f7ec3922d71e8f2cc9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 16:24:22 +
Subject: [PATCH] Fix CA certificate backup and restore
Backup and restore /etc/pki/ca-trust/source/ipa.p11-kit.
Create /etc/ipa/nssdb after restore if necessary.
https://fedorahosted.org/freeipa/ticket/4711
---
ipaplatform/base/paths.py| 2 +-
ipaplatform/base/tasks.py| 9 +
ipaplatform/redhat/tasks.py | 43
ipaserver/install/ipa_backup.py | 1 +
ipaserver/install/ipa_restore.py | 35 +++-
5 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index af50262..e28147a 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -92,7 +92,7 @@ class BasePathNamespace(object):
PAM_LDAP_CONF = /etc/pam_ldap.conf
PASSWD = /etc/passwd
ETC_PKI_CA_DIR = /etc/pki-ca
-SYSTEMWIDE_CA_STORE = /etc/pki/ca-trust/source/anchors/
+SYSTEMWIDE_IPA_CA_CRT = /etc/pki/ca-trust/source/anchors/ipa-ca.crt
IPA_P11_KIT = /etc/pki/ca-trust/source/ipa.p11-kit
NSS_DB_DIR = /etc/pki/nssdb
PKI_TOMCAT = /etc/pki/pki-tomcat
diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index f2ba81f..9b15119 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -55,6 +55,15 @@ class BaseTaskNamespace(object):
return
+def reload_systemwide_ca_store(self):
+
+Reloads the systemwide CA store.
+
+Returns True if the operation succeeded, False otherwise.
+
+
+return True
+
def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
Adds CA certificates from 'ca_certs' to the systemwide CA store
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 3f5fc90..d0e3cde 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -158,8 +158,19 @@ class RedHatTaskNamespace(BaseTaskNamespace):
auth_config.add_option(nostart)
auth_config.execute()
+def reload_systemwide_ca_store(self):
+try:
+ipautil.run([paths.UPDATE_CA_TRUST])
+except CalledProcessError, e:
+root_logger.error(
+Could not update systemwide CA trust database: %s, e)
+return False
+else:
+root_logger.info(Systemwide CA database updated.)
+return True
+
def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
-new_cacert_path = os.path.join(paths.SYSTEMWIDE_CA_STORE, 'ipa-ca.crt')
+new_cacert_path = paths.SYSTEMWIDE_IPA_CA_CRT
if os.path.exists(new_cacert_path):
try:
@@ -248,24 +259,18 @@ class RedHatTaskNamespace(BaseTaskNamespace):
f.close()
# Add the CA to the systemwide CA trust database
-try:
-ipautil.run([paths.UPDATE_CA_TRUST])
-except CalledProcessError, e:
-root_logger.info(Failed to add CA to the systemwide
- CA trust database: %s % str(e))
-else:
-root_logger.info('Added the CA to the systemwide CA trust '
- 'database.')
-return True
+if not self.reload_systemwide_ca_store():
+return False
-return False
+return True
def remove_ca_certs_from_systemwide_ca_store(self):
-ipa_ca_crt = os.path.join(paths.SYSTEMWIDE_CA_STORE, 'ipa-ca.crt')
+result = True
update = False
# Remove CA cert from systemwide store
-for new_cacert_path in (paths.IPA_P11_KIT, ipa_ca_crt):
+for new_cacert_path in (paths.IPA_P11_KIT,
+paths.SYSTEMWIDE_IPA_CA_CRT):
if not os.path.exists(new_cacert_path):
continue
try:
@@ -273,21 +278,15 @@ class RedHatTaskNamespace(BaseTaskNamespace):
except OSError, e:
root_logger.error(
Could not remove %s: %s, new_cacert_path, e)
+result = False
else:
update = True
if update:
-try:
-ipautil.run([paths.UPDATE_CA_TRUST])
-except CalledProcessError, e:
-root_logger.error(
-Could not update systemwide CA trust database: %s, e)
+if not self.reload_systemwide_ca_store():
return False
-else:
-root_logger.info(Systemwide CA database updated.)
-return True
-return False
+return result
def backup_and_replace_hostname(self, fstore, statestore, hostname):
old_hostname = socket.gethostname()
diff --git
[Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Hi,
the attached patches provide additional fixes for
https://fedorahosted.org/freeipa/ticket/4651.
I'm not 100% sure if the fixes for ipa-sam and ipa-kdb are correct,
please check them carefully.
Honza
--
Jan Cholasta
From a195644143042a0161de81472646f41f503ffe48 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 17:20:18 +
Subject: [PATCH 1/7] Remove redefinition of LOG from ipa-otp-lasttoken
https://fedorahosted.org/freeipa/ticket/4651
---
daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
index d20fca1..15b404d 100644
--- a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
+++ b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
@@ -47,9 +47,6 @@
#include util.h
#define PLUGIN_NAME ipa-otp-lasttoken
-#define LOG(sev, ...) \
-slapi_log_error(SLAPI_LOG_ ## sev, PLUGIN_NAME, \
-%s: %s\n, __func__, __VA_ARGS__), -1
static void *plugin_id;
static const Slapi_PluginDesc preop_desc = {
--
2.1.0
From b5800224806278ed0d1a165acbe7a12fdd74fbf6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 17:33:23 +
Subject: [PATCH 2/7] Unload P11_Helper object's library when it is finalized
in ipap11helper
https://fedorahosted.org/freeipa/ticket/4651
---
ipapython/ipap11helper/library.c | 5 +
ipapython/ipap11helper/p11helper.c | 9 +++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/ipapython/ipap11helper/library.c b/ipapython/ipap11helper/library.c
index 51e24eb..619604d 100644
--- a/ipapython/ipap11helper/library.c
+++ b/ipapython/ipap11helper/library.c
@@ -70,6 +70,11 @@ CK_C_GetFunctionList loadLibrary(char* module, void** moduleHandle)
// Retrieve the entry point for C_GetFunctionList
pGetFunctionList = (CK_C_GetFunctionList) dlsym(pDynLib, C_GetFunctionList);
+ if (pGetFunctionList == NULL)
+ {
+ dlclose(pDynLib);
+ return NULL;
+ }
// Store the handle so we can dlclose it later
*moduleHandle = pDynLib;
diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c
index 038c26c..558185e 100644
--- a/ipapython/ipap11helper/p11helper.c
+++ b/ipapython/ipap11helper/p11helper.c
@@ -66,6 +66,7 @@ PyObject_HEAD
CK_SLOT_ID slot;
CK_FUNCTION_LIST_PTR p11;
CK_SESSION_HANDLE session;
+void *module_handle;
} P11_Helper;
typedef enum {
@@ -478,6 +479,7 @@ P11_Helper_new(PyTypeObject *type, PyObject *args, PyObject *kwds) {
self-slot = 0;
self-session = 0;
self-p11 = NULL;
+self-module_handle = NULL;
}
return (PyObject *) self;
@@ -496,12 +498,12 @@ static int P11_Helper_init(P11_Helper *self, PyObject *args, PyObject *kwds) {
CK_C_GetFunctionList pGetFunctionList = loadLibrary(library_path,
module_handle);
if (!pGetFunctionList) {
-if (module_handle != NULL)
-unloadLibrary(module_handle);
PyErr_SetString(ipap11helperError, Could not load the library.);
return -1;
}
+self-module_handle = module_handle;
+
/*
* Load the function list
*/
@@ -567,9 +569,12 @@ P11_Helper_finalize(P11_Helper* self) {
*/
self-p11-C_Finalize(NULL);
+unloadLibrary(self-module_handle);
+
self-p11 = NULL;
self-session = 0;
self-slot = 0;
+self-module_handle = NULL;
return Py_None;
}
--
2.1.0
From 26d61f0284dba1ac98ae02260536772465da8819 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 17:40:35 +
Subject: [PATCH 3/7] Fix Kerberos error handling in ipa-sam
https://fedorahosted.org/freeipa/ticket/4651
---
daemons/ipa-sam/ipa_sam.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 3b69f9e..e711299 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -4233,7 +4233,7 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, vo
krb5_free_principal(data.context, in_creds.server);
krb5_free_principal(data.context, in_creds.client);
- if (rc) {
+ if (rc != 0 rc != KRB5KRB_AP_ERR_TKT_NYV rc != KRB5KRB_AP_ERR_TKT_EXPIRED) {
rc = bind_callback_obtain_creds(data);
if (rc) {
bind_callback_cleanup(data, rc);
--
2.1.0
From 6a386daa320c0db9c47709330d793881bda3 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 18:10:27 +
Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb
https://fedorahosted.org/freeipa/ticket/4651
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index c8f6c76..debcd1b 100644
--- a/daemons/ipa-kdb
Re: [Freeipa-devel] [PATCH] 354 Modififed NSSConnection not to shutdown existing database.
Hi, Dne 28.10.2014 v 23:17 Endi Sukma Dewata napsal(a): On 10/22/2014 9:15 AM, Endi Sukma Dewata wrote: The NSSConnection class has been modified not to shutdown the existing NSS database if the database is already opened to establish an SSL connection, or is already opened by another code that uses an NSS database without establishing an SSL connection such as vault CLIs. Ticket #4638 New patch attached. It's identical except for the ticket URL in the commit log. ACK. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 365 Fix CA certificate backup and restore
Dne 10.11.2014 v 17:46 Jan Cholasta napsal(a):
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4711.
Honza
Forgot to include /etc/pki/ca-trust/source/anchors/ipa-ca.crt in backup.
Updated patch attached.
--
Jan Cholasta
From f61c3b242e9eb83fa585b091da4d60b7262d124f Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 16:24:22 +
Subject: [PATCH] Fix CA certificate backup and restore
Backup and restore /etc/pki/ca-trust/source/ipa.p11-kit.
Create /etc/ipa/nssdb after restore if necessary.
https://fedorahosted.org/freeipa/ticket/4711
---
ipaplatform/base/paths.py| 2 +-
ipaplatform/base/tasks.py| 9 +
ipaplatform/redhat/tasks.py | 43
ipaserver/install/ipa_backup.py | 2 ++
ipaserver/install/ipa_restore.py | 35 +++-
5 files changed, 67 insertions(+), 24 deletions(-)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index af50262..e28147a 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -92,7 +92,7 @@ class BasePathNamespace(object):
PAM_LDAP_CONF = /etc/pam_ldap.conf
PASSWD = /etc/passwd
ETC_PKI_CA_DIR = /etc/pki-ca
-SYSTEMWIDE_CA_STORE = /etc/pki/ca-trust/source/anchors/
+SYSTEMWIDE_IPA_CA_CRT = /etc/pki/ca-trust/source/anchors/ipa-ca.crt
IPA_P11_KIT = /etc/pki/ca-trust/source/ipa.p11-kit
NSS_DB_DIR = /etc/pki/nssdb
PKI_TOMCAT = /etc/pki/pki-tomcat
diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index f2ba81f..9b15119 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -55,6 +55,15 @@ class BaseTaskNamespace(object):
return
+def reload_systemwide_ca_store(self):
+
+Reloads the systemwide CA store.
+
+Returns True if the operation succeeded, False otherwise.
+
+
+return True
+
def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
Adds CA certificates from 'ca_certs' to the systemwide CA store
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 3f5fc90..d0e3cde 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -158,8 +158,19 @@ class RedHatTaskNamespace(BaseTaskNamespace):
auth_config.add_option(nostart)
auth_config.execute()
+def reload_systemwide_ca_store(self):
+try:
+ipautil.run([paths.UPDATE_CA_TRUST])
+except CalledProcessError, e:
+root_logger.error(
+Could not update systemwide CA trust database: %s, e)
+return False
+else:
+root_logger.info(Systemwide CA database updated.)
+return True
+
def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
-new_cacert_path = os.path.join(paths.SYSTEMWIDE_CA_STORE, 'ipa-ca.crt')
+new_cacert_path = paths.SYSTEMWIDE_IPA_CA_CRT
if os.path.exists(new_cacert_path):
try:
@@ -248,24 +259,18 @@ class RedHatTaskNamespace(BaseTaskNamespace):
f.close()
# Add the CA to the systemwide CA trust database
-try:
-ipautil.run([paths.UPDATE_CA_TRUST])
-except CalledProcessError, e:
-root_logger.info(Failed to add CA to the systemwide
- CA trust database: %s % str(e))
-else:
-root_logger.info('Added the CA to the systemwide CA trust '
- 'database.')
-return True
+if not self.reload_systemwide_ca_store():
+return False
-return False
+return True
def remove_ca_certs_from_systemwide_ca_store(self):
-ipa_ca_crt = os.path.join(paths.SYSTEMWIDE_CA_STORE, 'ipa-ca.crt')
+result = True
update = False
# Remove CA cert from systemwide store
-for new_cacert_path in (paths.IPA_P11_KIT, ipa_ca_crt):
+for new_cacert_path in (paths.IPA_P11_KIT,
+paths.SYSTEMWIDE_IPA_CA_CRT):
if not os.path.exists(new_cacert_path):
continue
try:
@@ -273,21 +278,15 @@ class RedHatTaskNamespace(BaseTaskNamespace):
except OSError, e:
root_logger.error(
Could not remove %s: %s, new_cacert_path, e)
+result = False
else:
update = True
if update:
-try:
-ipautil.run([paths.UPDATE_CA_TRUST])
-except CalledProcessError, e:
-root_logger.error(
-Could not update systemwide CA trust database: %s, e)
+if not self.reload_systemwide_ca_store():
return False
-else:
-root_logger.info(Systemwide CA database updated.)
-return True
-return False
+return
Re: [Freeipa-devel] [PATCH] 0671 ipa-restore: Don't crash if AD trust is not installed
Dne 11.11.2014 v 09:08 Petr Viktorin napsal(a): On 11/10/2014 04:36 PM, Martin Kosek wrote: On 11/10/2014 02:52 PM, Petr Viktorin wrote: This is a fix for: https://fedorahosted.org/freeipa/ticket/4668 And the patch is...? Here, sorry. Thanks, ACK. Pushed to: master: a8e2a242bec1ce68d6b14be27e1b5b8d94f0deb9 ipa-4-1: d6b79a3ce7dd253737c41854e81273b01bc4a4c4 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Dne 10.11.2014 v 19:25 Jan Cholasta napsal(a):
Hi,
the attached patches provide additional fixes for
https://fedorahosted.org/freeipa/ticket/4651.
I'm not 100% sure if the fixes for ipa-sam and ipa-kdb are correct,
please check them carefully.
Honza
Changed the ticket to https://fedorahosted.org/freeipa/ticket/4713.
Updated patches attached.
--
Jan Cholasta
From c2a03a9e062df5691431babeb55119dbda6b2c67 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 17:20:18 +
Subject: [PATCH 1/7] Remove redefinition of LOG from ipa-otp-lasttoken
https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
index d20fca1..15b404d 100644
--- a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
+++ b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
@@ -47,9 +47,6 @@
#include util.h
#define PLUGIN_NAME ipa-otp-lasttoken
-#define LOG(sev, ...) \
-slapi_log_error(SLAPI_LOG_ ## sev, PLUGIN_NAME, \
-%s: %s\n, __func__, __VA_ARGS__), -1
static void *plugin_id;
static const Slapi_PluginDesc preop_desc = {
--
2.1.0
From 10b309f53852665050465df8aa44290dfe232291 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 17:33:23 +
Subject: [PATCH 2/7] Unload P11_Helper object's library when it is finalized
in ipap11helper
https://fedorahosted.org/freeipa/ticket/4713
---
ipapython/ipap11helper/library.c | 5 +
ipapython/ipap11helper/p11helper.c | 9 +++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/ipapython/ipap11helper/library.c b/ipapython/ipap11helper/library.c
index 51e24eb..619604d 100644
--- a/ipapython/ipap11helper/library.c
+++ b/ipapython/ipap11helper/library.c
@@ -70,6 +70,11 @@ CK_C_GetFunctionList loadLibrary(char* module, void** moduleHandle)
// Retrieve the entry point for C_GetFunctionList
pGetFunctionList = (CK_C_GetFunctionList) dlsym(pDynLib, C_GetFunctionList);
+ if (pGetFunctionList == NULL)
+ {
+ dlclose(pDynLib);
+ return NULL;
+ }
// Store the handle so we can dlclose it later
*moduleHandle = pDynLib;
diff --git a/ipapython/ipap11helper/p11helper.c b/ipapython/ipap11helper/p11helper.c
index 038c26c..558185e 100644
--- a/ipapython/ipap11helper/p11helper.c
+++ b/ipapython/ipap11helper/p11helper.c
@@ -66,6 +66,7 @@ PyObject_HEAD
CK_SLOT_ID slot;
CK_FUNCTION_LIST_PTR p11;
CK_SESSION_HANDLE session;
+void *module_handle;
} P11_Helper;
typedef enum {
@@ -478,6 +479,7 @@ P11_Helper_new(PyTypeObject *type, PyObject *args, PyObject *kwds) {
self-slot = 0;
self-session = 0;
self-p11 = NULL;
+self-module_handle = NULL;
}
return (PyObject *) self;
@@ -496,12 +498,12 @@ static int P11_Helper_init(P11_Helper *self, PyObject *args, PyObject *kwds) {
CK_C_GetFunctionList pGetFunctionList = loadLibrary(library_path,
module_handle);
if (!pGetFunctionList) {
-if (module_handle != NULL)
-unloadLibrary(module_handle);
PyErr_SetString(ipap11helperError, Could not load the library.);
return -1;
}
+self-module_handle = module_handle;
+
/*
* Load the function list
*/
@@ -567,9 +569,12 @@ P11_Helper_finalize(P11_Helper* self) {
*/
self-p11-C_Finalize(NULL);
+unloadLibrary(self-module_handle);
+
self-p11 = NULL;
self-session = 0;
self-slot = 0;
+self-module_handle = NULL;
return Py_None;
}
--
2.1.0
From 7ca2b25b93f1e3bff1e4c54ef1794cad462e533a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 17:40:35 +
Subject: [PATCH 3/7] Fix Kerberos error handling in ipa-sam
https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-sam/ipa_sam.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 3b69f9e..e711299 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -4233,7 +4233,7 @@ static int bind_callback(LDAP *ldap_struct, struct smbldap_state *ldap_state, vo
krb5_free_principal(data.context, in_creds.server);
krb5_free_principal(data.context, in_creds.client);
- if (rc) {
+ if (rc != 0 rc != KRB5KRB_AP_ERR_TKT_NYV rc != KRB5KRB_AP_ERR_TKT_EXPIRED) {
rc = bind_callback_obtain_creds(data);
if (rc) {
bind_callback_cleanup(data, rc);
--
2.1.0
From 82d7d37ca310af015018ebb2da2f9a72c4dabcaa Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 18:10:27 +
Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb
https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions
Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Dne 11.11.2014 v 11:40 Alexander Bokovoy napsal(a):
On Tue, 11 Nov 2014, Jan Cholasta wrote:
From 82d7d37ca310af015018ebb2da2f9a72c4dabcaa Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 18:10:27 +
Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb
https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
b/daemons/ipa-kdb/ipa_kdb_mspac.c
index c8f6c76..debcd1b 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2071,6 +2071,9 @@ krb5_error_code ipadb_sign_authdata(krb5_context
context,
ipactx-kdc_hostname,
strlen(ipactx-kdc_hostname),
NULL, NULL, result) == 0) {
kerr = ipadb_reinit_mspac(ipactx, true);
+if (kerr != 0 kerr != ENOENT) {
+goto done;
+}
}
}
I'm not sure we should drop the sign_authdata request here. If we were
able to re-initialize our view of trusted domains, we simply cannot
re-sign incoming PAC but this is handled in ipadb_verify_pac() and
ipadb_sign_pac() and if the former returns NULL value for PAC, we exit
with a return code of 0 while this change will fail a cross-realm TGT
request unconditionally.
OK, what would be a proper fix? Just ignore the return value of
ipadb_reinit_mspac here?
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] Fix dyndb-ldap working dir permission
Hi, Dne 11.11.2014 v 16:22 Martin Basti napsal(a): Using specfile to create file doesn't work if named user is not on system. Appropriate permission have to be set during ipa-dns installation. Patch attached Why is the directory set up in dnskeysyncinstance instead of bindinstance? The original patch was released with 4.1.1, shouldn't there be update in ipa-upgradeconfig? Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0027 Produce better error in group-add command.
Dne 10.11.2014 v 13:24 David Kupka napsal(a): On 11/10/2014 08:20 AM, Jan Cholasta wrote: Hi, Dne 7.11.2014 v 15:26 David Kupka napsal(a): https://fedorahosted.org/freeipa/ticket/4611 I think you should use MutuallyExclusiveError. Honza Thanks, that's the error class I was searching for. Unfortunately, I didn't know this one so I used more abstract error class. Fixed patch attached. Given the messages that are used for MutuallyExclusiveError everywhere else in the framework, I think a better message would be gid cannot be set for external group. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0672 - ipaplatform: Use the dirsrv service, not target
Hi, Dne 11.11.2014 v 17:03 Petr Viktorin napsal(a): https://fedorahosted.org/freeipa/ticket/4709 With this patch, IPA should no longer call systemctl on dirsrv.tagret, but rather on its concrete service. Since systemctl stop waits for shutdown of services (but not targets), we can assume dirsrv will be down (and the database unlocked) after stop() is done. There's a DS ticket [0] to explicitly tell systemd which process to wait on, but in my experience, systemd guesses correctly even without that fix. [0] https://fedorahosted.org/389/ticket/47951 Thanks, ACK. Pushed to: master: e60ef1fe029594876bd0e075cd5efc0173743138 ipa-4-1: 082485c2832a51cf6018bc172881e67e979de69c Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0159] FIX: DNS policy upgrade raises assertion error
Hi, Dne 13.11.2014 v 13:00 Martin Basti napsal(a): On 07/11/14 15:15, Martin Basti wrote: Ticket: https://fedorahosted.org/freeipa/ticket/4708 Patch attached. As I and Honza discussed personally, the updated patch modifies place of the fix Updated patch attached Thanks, ACK. Pushed to: ipa-4-0: e5ec47992cd641def024cc77c07f98ca66b7b673 ipa-4-1: 1b22a53717cd2ead8a8f3fec84d04dac698d8925 master: 40ea328a78bec511377b464700e3add09cedc2b9 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] Fix dyndb-ldap working dir permission
Dne 12.11.2014 v 13:33 Martin Basti napsal(a): On 11/11/14 16:58, Jan Cholasta wrote: Hi, Dne 11.11.2014 v 16:22 Martin Basti napsal(a): Using specfile to create file doesn't work if named user is not on system. Appropriate permission have to be set during ipa-dns installation. Patch attached Why is the directory set up in dnskeysyncinstance instead of bindinstance? Because, dnskeysyncinstance is the daemon which requires permission change. (dir is created by dyndb-ldap plugin) OK. But please rename the method to something more suitable (fix_dyndb_ldap_workdir_permissions?) and add a docstring/comment. Also please change the ticket link to https://fedorahosted.org/freeipa/ticket/4716 (cloned from BZ). The original patch was released with 4.1.1, shouldn't there be update in ipa-upgradeconfig? Cases: 1) fresh RPM install, no named user during RPM install - named doesn't start, user had to fix it immediately, can't wait until next release. 2) fresh RPM install, named user - no impact 3) upgrade IPA with DNS - no impact 4) upgrade IPA without DNS - after DNS installation, same as 1) 5) IPA 4.1.0 with installed DNS, upgrade to 4.1.2 - DNSSEC will not work (If user doesnt use DNSSEC) Only 5) looks serious for me, so here is updated patch. Could you do the update without the code duplication? In similar code an appropriate *instance method is usually called. Martin^2 Honza Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0027 Produce better error in group-add command.
Dne 13.11.2014 v 13:45 David Kupka napsal(a): On 11/13/2014 11:24 AM, Jan Cholasta wrote: Dne 10.11.2014 v 13:24 David Kupka napsal(a): On 11/10/2014 08:20 AM, Jan Cholasta wrote: Hi, Dne 7.11.2014 v 15:26 David Kupka napsal(a): https://fedorahosted.org/freeipa/ticket/4611 I think you should use MutuallyExclusiveError. Honza Thanks, that's the error class I was searching for. Unfortunately, I didn't know this one so I used more abstract error class. Fixed patch attached. Given the messages that are used for MutuallyExclusiveError everywhere else in the framework, I think a better message would be gid cannot be set for external group. Fixed patch attached. Thanks, ACK. Pushed to: master: b032debd233c70541e52d3ee677cb82f9840b291 ipa-4-1: cef8e06f8adc116936238264abf5883a0e49ec0a Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Dne 12.11.2014 v 08:58 Petr Spacek napsal(a):
On 11.11.2014 12:27, Jan Cholasta wrote:
Dne 11.11.2014 v 11:40 Alexander Bokovoy napsal(a):
On Tue, 11 Nov 2014, Jan Cholasta wrote:
From 82d7d37ca310af015018ebb2da2f9a72c4dabcaa Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 18:10:27 +
Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb
https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
b/daemons/ipa-kdb/ipa_kdb_mspac.c
index c8f6c76..debcd1b 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2071,6 +2071,9 @@ krb5_error_code ipadb_sign_authdata(krb5_context
context,
ipactx-kdc_hostname,
strlen(ipactx-kdc_hostname),
NULL, NULL, result) == 0) {
kerr = ipadb_reinit_mspac(ipactx, true);
+if (kerr != 0 kerr != ENOENT) {
+goto done;
+}
}
}
I'm not sure we should drop the sign_authdata request here. If we were
able to re-initialize our view of trusted domains, we simply cannot
re-sign incoming PAC but this is handled in ipadb_verify_pac() and
ipadb_sign_pac() and if the former returns NULL value for PAC, we exit
with a return code of 0 while this change will fail a cross-realm TGT
request unconditionally.
OK, what would be a proper fix? Just ignore the return value of
ipadb_reinit_mspac here?
Guys, I did not see the code but all instances of ignore return code I have
seen were wrong, including cases where code comment explicitly said we ignore
return code on purpose :-)
At least log an error message if you can't think of anything better ...
I don't disagree, if that's the proper fix.
Alexander, or someone else, could you please finish the review of the
patches? Thanks.
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] Fix dyndb-ldap working dir permission
Hi, Dne 13.11.2014 v 14:50 Martin Basti napsal(a): On 13/11/14 13:59, Jan Cholasta wrote: Dne 12.11.2014 v 13:33 Martin Basti napsal(a): On 11/11/14 16:58, Jan Cholasta wrote: Hi, Dne 11.11.2014 v 16:22 Martin Basti napsal(a): Using specfile to create file doesn't work if named user is not on system. Appropriate permission have to be set during ipa-dns installation. Patch attached Why is the directory set up in dnskeysyncinstance instead of bindinstance? Because, dnskeysyncinstance is the daemon which requires permission change. (dir is created by dyndb-ldap plugin) OK. But please rename the method to something more suitable (fix_dyndb_ldap_workdir_permissions?) and add a docstring/comment. Also please change the ticket link to https://fedorahosted.org/freeipa/ticket/4716 (cloned from BZ). The original patch was released with 4.1.1, shouldn't there be update in ipa-upgradeconfig? Cases: 1) fresh RPM install, no named user during RPM install - named doesn't start, user had to fix it immediately, can't wait until next release. 2) fresh RPM install, named user - no impact 3) upgrade IPA with DNS - no impact 4) upgrade IPA without DNS - after DNS installation, same as 1) 5) IPA 4.1.0 with installed DNS, upgrade to 4.1.2 - DNSSEC will not work (If user doesnt use DNSSEC) Only 5) looks serious for me, so here is updated patch. Could you do the update without the code duplication? In similar code an appropriate *instance method is usually called. The uid/gid resolution in ipa-upgradeconfig still looks like duplicated code to me. I would suggest doing something along these lines in ipa-upgradeconfig: dnskeysync = dnskeysyncinstance.DNSKeySyncInstance() dnskeysync.set_dyndb_ldap_workdir_permissions() and have DNSKeySyncInstance.set_dyndb_ldap_workdir_permissions() do all the real work. Martin^2 Honza Honza Thanks. updated patch attached. Martin^2 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] Fix dyndb-ldap working dir permission
Dne 18.11.2014 v 16:53 Martin Basti napsal(a): On 18/11/14 15:01, Jan Cholasta wrote: Hi, Dne 13.11.2014 v 14:50 Martin Basti napsal(a): On 13/11/14 13:59, Jan Cholasta wrote: Dne 12.11.2014 v 13:33 Martin Basti napsal(a): On 11/11/14 16:58, Jan Cholasta wrote: Hi, Dne 11.11.2014 v 16:22 Martin Basti napsal(a): Using specfile to create file doesn't work if named user is not on system. Appropriate permission have to be set during ipa-dns installation. Patch attached Why is the directory set up in dnskeysyncinstance instead of bindinstance? Because, dnskeysyncinstance is the daemon which requires permission change. (dir is created by dyndb-ldap plugin) OK. But please rename the method to something more suitable (fix_dyndb_ldap_workdir_permissions?) and add a docstring/comment. Also please change the ticket link to https://fedorahosted.org/freeipa/ticket/4716 (cloned from BZ). The original patch was released with 4.1.1, shouldn't there be update in ipa-upgradeconfig? Cases: 1) fresh RPM install, no named user during RPM install - named doesn't start, user had to fix it immediately, can't wait until next release. 2) fresh RPM install, named user - no impact 3) upgrade IPA with DNS - no impact 4) upgrade IPA without DNS - after DNS installation, same as 1) 5) IPA 4.1.0 with installed DNS, upgrade to 4.1.2 - DNSSEC will not work (If user doesnt use DNSSEC) Only 5) looks serious for me, so here is updated patch. Could you do the update without the code duplication? In similar code an appropriate *instance method is usually called. The uid/gid resolution in ipa-upgradeconfig still looks like duplicated code to me. I would suggest doing something along these lines in ipa-upgradeconfig: dnskeysync = dnskeysyncinstance.DNSKeySyncInstance() dnskeysync.set_dyndb_ldap_workdir_permissions() and have DNSKeySyncInstance.set_dyndb_ldap_workdir_permissions() do all the real work. Updated patch attached. Martin^2 Thanks, ACK. Pushed to: master: 7c176b708eb855ea8774ad36ba72fd31952a8895 ipa-4-1: ba124045b9f39f8264a974c977beba6f15b1b1fb -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 373 Update Requires on pki-ca to 10.2.1-0.1
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4645.
Honza
--
Jan Cholasta
From d022389ef15101fca108ec2be9b88b417f369dc3 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 18 Nov 2014 14:57:17 +
Subject: [PATCH] Update Requires on pki-ca to 10.2.1-0.1
https://fedorahosted.org/freeipa/ticket/4645
---
freeipa.spec.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index b2ff97a..52e6436 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -130,7 +130,7 @@ Requires(post): systemd-units
Requires: selinux-policy = %{selinux_policy_version}
Requires(post): selinux-policy-base
Requires: slapi-nis = 0.54.1-1
-Requires: pki-ca = 10.2.0-3
+Requires: pki-ca = 10.2.1-0.1
%if 0%{?rhel}
Requires: subscription-manager
%endif
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 374 Fix wrong expiration date on renewed IPA CA certificates
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4717.
Honza
--
Jan Cholasta
From 871217e002b8a2ee4f58c42977ac680a5305de1a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 18 Nov 2014 14:01:59 +
Subject: [PATCH] Fix wrong expiration date on renewed IPA CA certificates
The expiration date was always set to the expiration date of the original
certificate.
https://fedorahosted.org/freeipa/ticket/4717
---
freeipa.spec.in | 4 ++--
install/certmonger/dogtag-ipa-ca-renew-agent-submit | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index af36703..b464189 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -140,7 +140,7 @@ Requires: python-dns = 1.11.1
Requires: zip
Requires: policycoreutils = 2.1.12-5
Requires: tar
-Requires(pre): certmonger = 0.75.13
+Requires(pre): certmonger = 0.76.8
Requires(pre): 389-ds-base = 1.3.3.5
Requires: fontawesome-fonts
Requires: open-sans-fonts
@@ -227,7 +227,7 @@ Requires: wget
Requires: libcurl = 7.21.7-2
Requires: xmlrpc-c = 1.27.4
Requires: sssd = 1.12.2
-Requires: certmonger = 0.75.6
+Requires: certmonger = 0.76.8
Requires: nss-tools
Requires: bind-utils
Requires: oddjob-mkhomedir
diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index e5ad963..0a2cff1 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -146,6 +146,8 @@ def request_cert():
path = paths.DOGTAG_IPA_RENEW_AGENT_SUBMIT
args = [path] + sys.argv[1:]
+if os.environ.get('CERTMONGER_CA_PROFILE') == 'caCACert':
+args += ['-O', 'bypassCAnotafter=true']
stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ)
sys.stderr.write(stderr)
sys.stderr.flush()
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 373 Update Requires on pki-ca to 10.2.1-0.1
Dne 19.11.2014 v 13:55 Petr Vobornik napsal(a): On 18.11.2014 23:29, Nathaniel McCallum wrote: On Tue, 2014-11-18 at 19:56 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4645. ACK Shouldn't the version be 10.1.2-4 ? http://koji.fedoraproject.org/koji/buildinfo?buildID=594223 No. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0286] baseldap: Handle missing parent objects properly in *-find
Dne 19.11.2014 v 13:44 Tomas Babej napsal(a): On 11/19/2014 12:51 PM, Martin Kosek wrote: On 11/19/2014 12:41 PM, Tomas Babej wrote: On 11/19/2014 12:24 PM, Martin Kosek wrote: On 11/19/2014 12:03 PM, Tomas Babej wrote: Hi, When constructing a parent DN in LDAPSearch, we should always check that the parent object exists (hence use get_dn_if_exists), rather than search on unexistant containers (which can happen with get_dn). Replaces get_dn calls with get_dn_if_exists in *-find commands and makes sure proper error message is raised. https://fedorahosted.org/freeipa/ticket/4659 Doesn't it produce extra LDAP search thus making all our search commands slower? Is that what we want? No it does not make all of our LDAP search slower. It only happens for the objects that have parent objects, such as idoverrides or dnsrecords. ... and makes them slower. What I was pointing out here is that this is not a issue for ALL *-find commands (e.g user-find, group-find will not suffer from it), as you incorrectly stated. Wouldn't it be better to distinguish between LDAP search with no results and LDAP search with missing parent DN? The reply looks different, at least in CLI: Up to discussion. We would probably need to introduce a new exception, like ParentObjectNotFound. # search result search: 4 result: 0 Success # search result search: 4 result: 32 No such object matchedDN: cn=accounts,dc=mkosek-f20,dc=test Also, I do not think you can just stop using get_dn(), some commands override this call to get more complex searches (like host-find searching for shortname). Look into the get_dn_if_exists, it just wraps around get_dn, so no issue here. Any custom behaviour is preserved. Ah, ok, thanks for info. To sum up, I think this is worth changing this behaviour by default, ignoring a non-matching value of the parent object is not a correct general approach in my opinion. Well, that's the question. Whether we would leave DS to validate the search itself or do all the pre-check ourselves. To me, doing just one LDAP search and processing the error correctly looks better. But I can live even with your version then, I will leave the framework guardians like Honza or Petr3 to decide. +1 on single LDAP search and proper error processing. I see now what you're trying to suggest. However, the reason boils down to ipaldap.find_entries method not differentiating between a LDAP search that returns error code 32 (No such object) and LDAP search returning error code 0 (Success), but returning no results. In both cases errors.NotFound is raised. The reason I did not go this way is that changing the find_entries method is quite more invasive as this is the method subsenqently called by almost any command. You can always derive the new error (ParentNotFound or whatever) on NotFound, so old code won't break. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 373 Update Requires on pki-ca to 10.2.1-0.1
Dne 19.11.2014 v 14:07 Petr Vobornik napsal(a): On 19.11.2014 13:59, Jan Cholasta wrote: Dne 19.11.2014 v 13:55 Petr Vobornik napsal(a): On 18.11.2014 23:29, Nathaniel McCallum wrote: On Tue, 2014-11-18 at 19:56 +0100, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4645. ACK Shouldn't the version be 10.1.2-4 ? http://koji.fedoraproject.org/koji/buildinfo?buildID=594223 No. ok, nevermind, f21 has 10.2.1-0.1, but it doesn't list the fix in the changelog. But the patch needs a rebase because of Martin's patch. http://koji.fedoraproject.org/koji/buildinfo?buildID=588670 Rebased and pushed to ipa-4-1: 4e1193119b3e7f7c13f504f24445509958887927 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 374 Fix wrong expiration date on renewed IPA CA certificates
Dne 19.11.2014 v 15:02 David Kupka napsal(a): On 11/19/2014 08:32 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4717. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Works for me, thanks, ACK. Pushed to: master: 52b141ca6a257b8f12d9ad2ade812ec1bfebf0d7 ipa-4-1: 7aa855a37b1996588d7d2084176e38145b1587be -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0030 Fix --{user, group}-ignore-attribute in migration plugin.
Dne 20.11.2014 v 09:51 David Kupka napsal(a): https://fedorahosted.org/freeipa/ticket/4620 IMO changing the loop to: for attr in attr_blacklist: entry_attrs.pop(attr, None) would be better, because LDAPEntry already handles case insensitivity in attribute names. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 376 Stop tracking certificates before restoring them in ipa-restore
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4727.
Honza
--
Jan Cholasta
From 2cf85ec35cf4618279af81ba16d4a4805e8c590e Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 20 Nov 2014 13:57:46 +
Subject: [PATCH] Stop tracking certificates before restoring them in
ipa-restore
https://fedorahosted.org/freeipa/ticket/4727
---
ipaserver/install/ipa_restore.py | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 7276ed3..a9a3cbf 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -26,7 +26,7 @@ import pwd
from ConfigParser import SafeConfigParser
from ipalib import api, errors
-from ipapython import version, ipautil, certdb
+from ipapython import version, ipautil, certdb, dogtag
from ipapython.ipautil import run, user_input
from ipapython import admintool
from ipapython.dn import DN
@@ -36,7 +36,7 @@ from ipaserver.install.cainstance import PKI_USER, create_ca_user
from ipaserver.install.replication import (wait_for_task, ReplicationManager,
get_cs_replication_manager)
from ipaserver.install import installutils
-from ipaserver.install import httpinstance
+from ipaserver.install import dsinstance, httpinstance, cainstance
from ipapython import ipaldap
import ipapython.errors
from ipaplatform.tasks import tasks
@@ -663,6 +663,12 @@ class Restore(admintool.AdminTool):
self.log.error('%s', e)
def cert_restore_prepare(self):
+cainstance.CAInstance().stop_tracking_certificates(
+dogtag.configured_constants())
+httpinstance.HTTPInstance().stop_tracking_certificates()
+dsinstance.DsInstance().stop_tracking_certificates(
+realm_to_serverid(api.env.realm))
+
for basename in ('cert8.db', 'key3.db', 'secmod.db', 'pwdfile.txt'):
filename = os.path.join(paths.IPA_NSSDB_DIR, basename)
try:
@@ -692,3 +698,5 @@ class Restore(admintool.AdminTool):
(nickname, paths.IPA_NSSDB_DIR, e))
tasks.reload_systemwide_ca_store()
+
+services.knownservices.certmonger.restart()
--
2.1.0
From 70bfeade55dd359f722b6d6cfb5efd6842e5c1ba Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 20 Nov 2014 13:57:46 +
Subject: [PATCH] Stop tracking certificates before restoring them in
ipa-restore
https://fedorahosted.org/freeipa/ticket/4727
---
ipaserver/install/ipa_restore.py | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 8b1e80f..bf1e5fd 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -26,7 +26,7 @@ import pwd
from ConfigParser import SafeConfigParser
from ipalib import api, errors
-from ipapython import version, ipautil, certdb
+from ipapython import version, ipautil, certdb, dogtag
from ipapython.ipautil import run, user_input
from ipapython import admintool
from ipapython.dn import DN
@@ -36,7 +36,7 @@ from ipaserver.install.cainstance import PKI_USER, create_ca_user
from ipaserver.install.replication import (wait_for_task, ReplicationManager,
get_cs_replication_manager)
from ipaserver.install import installutils
-from ipaserver.install import httpinstance
+from ipaserver.install import dsinstance, httpinstance, cainstance
from ipapython import ipaldap
import ipapython.errors
from ipaplatform.tasks import tasks
@@ -664,6 +664,12 @@ class Restore(admintool.AdminTool):
self.log.error('%s', e)
def cert_restore_prepare(self):
+cainstance.stop_tracking_certificates(
+dogtag.configured_constants())
+httpinstance.HTTPInstance().stop_tracking_certificates()
+dsinstance.DsInstance().stop_tracking_certificates(
+realm_to_serverid(api.env.realm))
+
for basename in ('cert8.db', 'key3.db', 'secmod.db', 'pwdfile.txt'):
filename = os.path.join(paths.IPA_NSSDB_DIR, basename)
try:
@@ -693,3 +699,5 @@ class Restore(admintool.AdminTool):
(nickname, paths.IPA_NSSDB_DIR, e))
tasks.reload_systemwide_ca_store()
+
+services.knownservices.certmonger.restart()
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0030 Fix --{user, group}-ignore-attribute in migration plugin.
Dne 20.11.2014 v 14:51 Martin Basti napsal(a):
On 20/11/14 11:53, David Kupka wrote:
On 11/20/2014 10:03 AM, Jan Cholasta wrote:
Dne 20.11.2014 v 09:51 David Kupka napsal(a):
https://fedorahosted.org/freeipa/ticket/4620
IMO changing the loop to:
for attr in attr_blacklist:
entry_attrs.pop(attr, None)
would be better, because LDAPEntry already handles case insensitivity in
attribute names.
This seems better, thanks.
IMO the same problem is with objectclasses.
# do not migrate all object classes
if 'objectclass' in entry_attrs:
for object_class in kwargs.get('oc_blacklist', []):
try:
entry_attrs['objectclass'].remove(object_class) #
this is case sensitive
except ValueError: # object class not present
pass
Am I right?
Yes. LDAPEntry does not support case insensitivity in this case,
although I plan to implement it in 4.2.
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0286] baseldap: Handle missing parent objects properly in *-find
Dne 19.11.2014 v 15:12 Tomas Babej napsal(a): On 11/19/2014 02:03 PM, Jan Cholasta wrote: Dne 19.11.2014 v 13:44 Tomas Babej napsal(a): On 11/19/2014 12:51 PM, Martin Kosek wrote: On 11/19/2014 12:41 PM, Tomas Babej wrote: On 11/19/2014 12:24 PM, Martin Kosek wrote: On 11/19/2014 12:03 PM, Tomas Babej wrote: Hi, When constructing a parent DN in LDAPSearch, we should always check that the parent object exists (hence use get_dn_if_exists), rather than search on unexistant containers (which can happen with get_dn). Replaces get_dn calls with get_dn_if_exists in *-find commands and makes sure proper error message is raised. https://fedorahosted.org/freeipa/ticket/4659 Doesn't it produce extra LDAP search thus making all our search commands slower? Is that what we want? No it does not make all of our LDAP search slower. It only happens for the objects that have parent objects, such as idoverrides or dnsrecords. ... and makes them slower. What I was pointing out here is that this is not a issue for ALL *-find commands (e.g user-find, group-find will not suffer from it), as you incorrectly stated. Wouldn't it be better to distinguish between LDAP search with no results and LDAP search with missing parent DN? The reply looks different, at least in CLI: Up to discussion. We would probably need to introduce a new exception, like ParentObjectNotFound. # search result search: 4 result: 0 Success # search result search: 4 result: 32 No such object matchedDN: cn=accounts,dc=mkosek-f20,dc=test Also, I do not think you can just stop using get_dn(), some commands override this call to get more complex searches (like host-find searching for shortname). Look into the get_dn_if_exists, it just wraps around get_dn, so no issue here. Any custom behaviour is preserved. Ah, ok, thanks for info. To sum up, I think this is worth changing this behaviour by default, ignoring a non-matching value of the parent object is not a correct general approach in my opinion. Well, that's the question. Whether we would leave DS to validate the search itself or do all the pre-check ourselves. To me, doing just one LDAP search and processing the error correctly looks better. But I can live even with your version then, I will leave the framework guardians like Honza or Petr3 to decide. +1 on single LDAP search and proper error processing. I see now what you're trying to suggest. However, the reason boils down to ipaldap.find_entries method not differentiating between a LDAP search that returns error code 32 (No such object) and LDAP search returning error code 0 (Success), but returning no results. In both cases errors.NotFound is raised. The reason I did not go this way is that changing the find_entries method is quite more invasive as this is the method subsenqently called by almost any command. You can always derive the new error (ParentNotFound or whatever) on NotFound, so old code won't break. Thanks for the suggestsions. Attached is a new patch which hooks into find_entries method and differentiates between the cases. Why are you special casing base scope search? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 377 Use correct service name in cainstance.backup_config
Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4754. Honza -- Jan Cholasta From c1db9d92088436234d2a00c946a8e470d740745b Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Fri, 21 Nov 2014 07:52:24 + Subject: [PATCH] Use correct service name in cainstance.backup_config https://fedorahosted.org/freeipa/ticket/4754 --- ipaserver/install/cainstance.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py index 6ccbe41..6b4317f 100644 --- a/ipaserver/install/cainstance.py +++ b/ipaserver/install/cainstance.py @@ -1861,7 +1861,8 @@ def backup_config(dogtag_constants=None): if dogtag_constants is None: dogtag_constants = dogtag.configured_constants() -if services.knownservices.dogtag.is_running(): +if services.knownservices[dogtag_constants.SERVICE_NAME].is_running( +dogtag_constants.PKI_INSTANCE_NAME): raise RuntimeError(Dogtag must be stopped when creating backup of %s % dogtag_constants.CS_CFG_PATH) shutil.copy(dogtag_constants.CS_CFG_PATH, -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0169] Fix: read_ip_address should return CheckedIPAddress instance instead of string
Hi, Dne 20.11.2014 v 17:52 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4747 Patch attached: ACK! Pushed to: master: 7de424f42541e73ed24a95f1dd90ff4a61e111fa ipa-4-1: 5b397dced1ef4a1eea7b3636fc71c2b7108a0b25 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0286] baseldap: Handle missing parent objects properly in *-find
Dne 21.11.2014 v 11:28 Tomas Babej napsal(a): On 11/20/2014 04:01 PM, Jan Cholasta wrote: Dne 19.11.2014 v 15:12 Tomas Babej napsal(a): On 11/19/2014 02:03 PM, Jan Cholasta wrote: Dne 19.11.2014 v 13:44 Tomas Babej napsal(a): On 11/19/2014 12:51 PM, Martin Kosek wrote: On 11/19/2014 12:41 PM, Tomas Babej wrote: On 11/19/2014 12:24 PM, Martin Kosek wrote: On 11/19/2014 12:03 PM, Tomas Babej wrote: Hi, When constructing a parent DN in LDAPSearch, we should always check that the parent object exists (hence use get_dn_if_exists), rather than search on unexistant containers (which can happen with get_dn). Replaces get_dn calls with get_dn_if_exists in *-find commands and makes sure proper error message is raised. https://fedorahosted.org/freeipa/ticket/4659 Doesn't it produce extra LDAP search thus making all our search commands slower? Is that what we want? No it does not make all of our LDAP search slower. It only happens for the objects that have parent objects, such as idoverrides or dnsrecords. ... and makes them slower. What I was pointing out here is that this is not a issue for ALL *-find commands (e.g user-find, group-find will not suffer from it), as you incorrectly stated. Wouldn't it be better to distinguish between LDAP search with no results and LDAP search with missing parent DN? The reply looks different, at least in CLI: Up to discussion. We would probably need to introduce a new exception, like ParentObjectNotFound. # search result search: 4 result: 0 Success # search result search: 4 result: 32 No such object matchedDN: cn=accounts,dc=mkosek-f20,dc=test Also, I do not think you can just stop using get_dn(), some commands override this call to get more complex searches (like host-find searching for shortname). Look into the get_dn_if_exists, it just wraps around get_dn, so no issue here. Any custom behaviour is preserved. Ah, ok, thanks for info. To sum up, I think this is worth changing this behaviour by default, ignoring a non-matching value of the parent object is not a correct general approach in my opinion. Well, that's the question. Whether we would leave DS to validate the search itself or do all the pre-check ourselves. To me, doing just one LDAP search and processing the error correctly looks better. But I can live even with your version then, I will leave the framework guardians like Honza or Petr3 to decide. +1 on single LDAP search and proper error processing. I see now what you're trying to suggest. However, the reason boils down to ipaldap.find_entries method not differentiating between a LDAP search that returns error code 32 (No such object) and LDAP search returning error code 0 (Success), but returning no results. In both cases errors.NotFound is raised. The reason I did not go this way is that changing the find_entries method is quite more invasive as this is the method subsenqently called by almost any command. You can always derive the new error (ParentNotFound or whatever) on NotFound, so old code won't break. Thanks for the suggestsions. Attached is a new patch which hooks into find_entries method and differentiates between the cases. Why are you special casing base scope search? Since base search is performed only on the entry specified by the DN, there is no need to differentiate between ContainerNotFound and NotFound. So the logic is as follows: subtree search - ContainerNotFound is raised when container does not exist, NotFound if search provided no results onelevel search - the same base search - NotFound is raised if the specified DN does not exist There is a difference between a search on a non-existent entry and a search on an existent entry with a non-matching filter. This difference exists on LDAP level and applies to all search scopes, not just the base scope. I don't think hiding this difference is useful at all. Remember that this bug exists because we were hiding the difference in the first place. Also, after giving this some thought, I think it would be better to create a new error for the case where there is no match instead of the case where the entry does not exist. NotFound pretty much corresponds to LDAP's NO_SUCH_OBJECT, something like NoMatchingEntry or EmptyResult would fit the no-match result better. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1111 Use NSS protocol range setter
Hi, Dne 20.11.2014 v 23:26 Rob Crittenden napsal(a): Use new capability in python-nss-0.16 to use the NSS protocol range setter. This lets us enable TLSv1.1 and TLSv1.2 for client connections. I made this configurable via tls_protocol_range in case somebody wants to override it. There isn't a whole ton of error handling on bad input but there is enough, I think, to point the user in the the right direction. Added a couple more lines of debug output to include the negotiated protocol and cipher. rob 1) The patch needs a rebase on top of ipa-4-1 (applies fine on master) 2) Could you split the option into two options, say tls_version_min and tls_version_max? IMO it would be easier to manage the version range that way, when for example you have to lower just the minimal version on a client to make it able to connect to a SSL3-only server. 3) Would it make sense to print a warning when the configured minimal TLS version is not safe and the connection uses a safe TLS version? This is for the case when you have to lower the minimal version on the client because of an old server, then the server gets updated, then you probably no longer want to have unsafe minimal version configured on the client. Functionally the patch is OK. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0032 Fix error message for nonexistent members and add tests.
Hi, Dne 21.11.2014 v 16:11 David Kupka napsal(a): https://fedorahosted.org/freeipa/ticket/4643 You probably don't want to change get_dn_if_exists to get_dn, as the latter does not usually raise NotFound when the entry does not exist. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1111 Use NSS protocol range setter
Dne 21.11.2014 v 16:09 Rob Crittenden napsal(a): Jan Cholasta wrote: Hi, Dne 20.11.2014 v 23:26 Rob Crittenden napsal(a): Use new capability in python-nss-0.16 to use the NSS protocol range setter. This lets us enable TLSv1.1 and TLSv1.2 for client connections. I made this configurable via tls_protocol_range in case somebody wants to override it. There isn't a whole ton of error handling on bad input but there is enough, I think, to point the user in the the right direction. Added a couple more lines of debug output to include the negotiated protocol and cipher. rob 1) The patch needs a rebase on top of ipa-4-1 (applies fine on master) Attached. 2) Could you split the option into two options, say tls_version_min and tls_version_max? IMO it would be easier to manage the version range that way, when for example you have to lower just the minimal version on a client to make it able to connect to a SSL3-only server. Sure. I waffled back and forth before deciding on a single value. Separate values are probably less error-prone. 3) Would it make sense to print a warning when the configured minimal TLS version is not safe and the connection uses a safe TLS version? This is for the case when you have to lower the minimal version on the client because of an old server, then the server gets updated, then you probably no longer want to have unsafe minimal version configured on the client. I see what you're saying but I think it could end up being just spam that user's get used to. That and given that I'd probably want to set it up to require tls1.1 as a minimum but we can't do that because dogtag only supports through tls1.0 right now AFAICT. That'd be a lot of warnings. You are probably right about the spam. Nevermind then. Functionally the patch is OK. rob Thanks for the patch, ACK. Fixed option names in commit message and pushed to: master: 5c0ad221e815e8c7b95c1d1095ebd6cf18e7e11c ipa-4-1: 8ef191448f0511b9c1749f47615437d649db0777 BTW before we can close the ticket, we are going to need a couple more fixes: 1) Bump required versions of 389-ds-base, pki-core and openldap, once the necessary fixes are available. 2) Configure mod_nss to also support TLS 1.2. It should be done on both server install and upgrade. This requires a new version of mod_nss. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0171] Fix encoding detection of zonemgr option
Hi, Dne 24.11.2014 v 14:01 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4762 Patch attached. Thanks, ACK. Pushed to: master: 230df95ed9e043069da0008d046b6b0135b0a8d1 ipa-4-1: 880f1e5c277a8826e3334723cd840cae4e65dfb8 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Dne 24.11.2014 v 14:44 Alexander Bokovoy napsal(a):
On Tue, 18 Nov 2014, Jan Cholasta wrote:
Dne 12.11.2014 v 08:58 Petr Spacek napsal(a):
On 11.11.2014 12:27, Jan Cholasta wrote:
Dne 11.11.2014 v 11:40 Alexander Bokovoy napsal(a):
On Tue, 11 Nov 2014, Jan Cholasta wrote:
From 82d7d37ca310af015018ebb2da2f9a72c4dabcaa Mon Sep 17 00:00:00
2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 10 Nov 2014 18:10:27 +
Subject: [PATCH 4/7] Fix unchecked return value in ipa-kdb
https://fedorahosted.org/freeipa/ticket/4713
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c
b/daemons/ipa-kdb/ipa_kdb_mspac.c
index c8f6c76..debcd1b 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2071,6 +2071,9 @@ krb5_error_code
ipadb_sign_authdata(krb5_context
context,
ipactx-kdc_hostname,
strlen(ipactx-kdc_hostname),
NULL, NULL, result) == 0) {
kerr = ipadb_reinit_mspac(ipactx, true);
+if (kerr != 0 kerr != ENOENT) {
+goto done;
+}
}
}
I'm not sure we should drop the sign_authdata request here. If we were
able to re-initialize our view of trusted domains, we simply cannot
re-sign incoming PAC but this is handled in ipadb_verify_pac() and
ipadb_sign_pac() and if the former returns NULL value for PAC, we exit
with a return code of 0 while this change will fail a cross-realm TGT
request unconditionally.
OK, what would be a proper fix? Just ignore the return value of
ipadb_reinit_mspac here?
Guys, I did not see the code but all instances of ignore return
code I have
seen were wrong, including cases where code comment explicitly said
we ignore
return code on purpose :-)
At least log an error message if you can't think of anything better ...
I don't disagree, if that's the proper fix.
Alexander, or someone else, could you please finish the review of the
patches? Thanks.
I've ACKed other patches but I'm not going to accept this patch. Rest is
fine.
That's fine with me, but I'm still going to need a little hint on how
this should in fact be fixed.
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Dne 11.11.2014 v 11:13 Jan Cholasta napsal(a):
Dne 10.11.2014 v 19:25 Jan Cholasta napsal(a):
Hi,
the attached patches provide additional fixes for
https://fedorahosted.org/freeipa/ticket/4651.
I'm not 100% sure if the fixes for ipa-sam and ipa-kdb are correct,
please check them carefully.
Honza
Changed the ticket to https://fedorahosted.org/freeipa/ticket/4713.
Updated patches attached.
Attaching additional patch for an issue that popped up recently.
--
Jan Cholasta
From fef20b5966b4a49cc8c230437cf8f06899b51840 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 24 Nov 2014 13:57:10 +
Subject: [PATCH] Fix memory leak in GetKeytabControl asn1 code
https://fedorahosted.org/freeipa/ticket/4713
---
asn1/ipa_asn1.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/asn1/ipa_asn1.c b/asn1/ipa_asn1.c
index 50851a8..9efca96 100644
--- a/asn1/ipa_asn1.c
+++ b/asn1/ipa_asn1.c
@@ -77,12 +77,12 @@ bool ipaasn1_enc_getktreply(int kvno, struct keys_container *keys,
{
GetKeytabControl_t gkctrl = { 0 };
bool ret = false;
+KrbKey_t *KK;
gkctrl.present = GetKeytabControl_PR_reply;
gkctrl.choice.reply.newkvno = kvno;
for (int i = 0; i keys-nkeys; i++) {
-KrbKey_t *KK;
KK = calloc(1, sizeof(KrbKey_t));
if (!KK) goto done;
KK-key.type = keys-ksdata[i].key.enctype;
@@ -109,9 +109,18 @@ bool ipaasn1_enc_getktreply(int kvno, struct keys_container *keys,
}
ret = encode_GetKeytabControl(gkctrl, buf, len);
+KK = NULL;
done:
ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_GetKeytabControl, gkctrl);
+if (KK) {
+free(KK-key.value.buf);
+if (KK-salt) {
+free(KK-salt-value.buf);
+free(KK-salt);
+}
+free(KK);
+}
return ret;
}
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 366-372 Additional Coverity fixes
Dne 25.11.2014 v 09:19 Alexander Bokovoy napsal(a): On Tue, 11 Nov 2014, Jan Cholasta wrote: Dne 10.11.2014 v 19:25 Jan Cholasta napsal(a): Hi, the attached patches provide additional fixes for https://fedorahosted.org/freeipa/ticket/4651. I'm not 100% sure if the fixes for ipa-sam and ipa-kdb are correct, please check them carefully. Honza Changed the ticket to https://fedorahosted.org/freeipa/ticket/4713. Updated patches attached. I think I've ACKed all individual patches. Thanks for the review. Pushed to: master: d55936756d9593bb1c713dc4c1edd251a112c619 ipa-4-1: 94bc7a9431c3e44c4e0dcf84fbb0f0613ec86600 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1111 Use NSS protocol range setter
Dne 24.11.2014 v 15:59 Rob Crittenden napsal(a):
Jan Cholasta wrote:
Dne 21.11.2014 v 16:09 Rob Crittenden napsal(a):
Jan Cholasta wrote:
Hi,
Dne 20.11.2014 v 23:26 Rob Crittenden napsal(a):
Use new capability in python-nss-0.16 to use the NSS protocol range
setter. This lets us enable TLSv1.1 and TLSv1.2 for client connections.
I made this configurable via tls_protocol_range in case somebody wants
to override it.
There isn't a whole ton of error handling on bad input but there is
enough, I think, to point the user in the the right direction.
Added a couple more lines of debug output to include the negotiated
protocol and cipher.
rob
1) The patch needs a rebase on top of ipa-4-1 (applies fine on master)
Attached.
2) Could you split the option into two options, say tls_version_min
and tls_version_max? IMO it would be easier to manage the version
range that way, when for example you have to lower just the minimal
version on a client to make it able to connect to a SSL3-only server.
Sure. I waffled back and forth before deciding on a single value.
Separate values are probably less error-prone.
3) Would it make sense to print a warning when the configured minimal
TLS version is not safe and the connection uses a safe TLS version? This
is for the case when you have to lower the minimal version on the client
because of an old server, then the server gets updated, then you
probably no longer want to have unsafe minimal version configured on the
client.
I see what you're saying but I think it could end up being just spam
that user's get used to. That and given that I'd probably want to set it
up to require tls1.1 as a minimum but we can't do that because dogtag
only supports through tls1.0 right now AFAICT. That'd be a lot of
warnings.
You are probably right about the spam. Nevermind then.
Functionally the patch is OK.
rob
Thanks for the patch, ACK.
Fixed option names in commit message and pushed to:
master: 5c0ad221e815e8c7b95c1d1095ebd6cf18e7e11c
ipa-4-1: 8ef191448f0511b9c1749f47615437d649db0777
BTW before we can close the ticket, we are going to need a couple more
fixes:
1) Bump required versions of 389-ds-base, pki-core and openldap, once
the necessary fixes are available.
Right, to be sure that POODLE is fully addressed.
I will post a patch once we have all of them.
2) Configure mod_nss to also support TLS 1.2. It should be done on both
server install and upgrade. This requires a new version of mod_nss.
mod_nss 1.0.10 in F-21 and rawhide should both support TLS 1.2 today.
mod_nss is also very tolerant of bad/unknown protocols. It won't blow up
on unknown protocols.
So if the given mod_nss doesn't support TLSv1.2 it will simply report an
error about an unknown protocol and configure the server for 1.0/1.1 if
configured as:
NSSProtocol TLSv1.0,TLSv1.1,TLSv1.2
The attached patch 379 fixes this.
rob
--
Jan Cholasta
From 815247c1aa4a923b2aa6fb12bd221ebd83083af2 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 25 Nov 2014 08:12:53 +
Subject: [PATCH] Add TLS 1.2 to the protocol list in mod_nss config
https://fedorahosted.org/freeipa/ticket/4653
---
install/tools/ipa-upgradeconfig | 13 +
ipaserver/install/httpinstance.py | 7 ---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index ffb51a9..815fe04 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -1287,6 +1287,18 @@ def fix_trust_flags():
sysupgrade.set_upgrade_state('http', 'fix_trust_flags', True)
+def update_mod_nss_protocol(http):
+root_logger.info('[Updating mod_nss protocol versions]')
+
+if sysupgrade.get_upgrade_state('nss.conf', 'protocol_updated_tls12'):
+root_logger.info(Protocol versions already updated)
+return
+
+http.set_mod_nss_protocol()
+
+sysupgrade.set_upgrade_state('nss.conf', 'protocol_updated_tls12', True)
+
+
def main():
Get some basics about the system. If getting those basics fail then
@@ -1388,6 +1400,7 @@ def main():
http.change_mod_nss_port_from_http()
http.stop()
+update_mod_nss_protocol(http)
fix_trust_flags()
http.start()
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 14efa5b..f9e0200 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -115,7 +115,8 @@ class HTTPInstance(service.Service):
self.step(setting mod_nss port to 443, self.__set_mod_nss_port)
-self.step(setting mod_nss protocol list to TLSv1.0 and TLSv1.1, self.__set_mod_nss_protocol)
+self.step(setting mod_nss protocol list to TLSv1.0 - TLSv1.2,
+ self.set_mod_nss_protocol)
self.step(setting mod_nss password file, self.__set_mod_nss_passwordfile)
self.step(enabling mod_nss renegotiate, self.enable_mod_nss_renegotiate
Re: [Freeipa-devel] [PATCH 0172] Fix zonemgr option encoding detection
Hi, Dne 25.11.2014 v 14:07 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4766 Patch attached. Thanks, ACK. Hopefully it's correct this time. Pushed to: master: c13862104ab64cda81c86c51b849c8d01c3c9187 ipa-4-1: e457a3e615b695cfd98e7d54594e5a3663562b06 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0172] Fix zonemgr option encoding detection
Hi, Dne 25.11.2014 v 14:07 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4766 Patch attached. Thanks, ACK. Hopefully it's correct this time. Pushed to: master: c13862104ab64cda81c86c51b849c8d01c3c9187 ipa-4-1: e457a3e615b695cfd98e7d54594e5a3663562b06 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0168] Better workaround to get status of CA during upgrade
Hi, Dne 27.11.2014 v 14:24 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4676 Replaces current workaround. Should go to 4.1.3. Patch attached. When constructing URLs with host:port, please use ipautil.format_netloc(). wget should be added as a dependency of freeipa-python in the spec file. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0173] Throw zonemgr error message before installation proceeds
Hi, Dne 27.11.2014 v 14:20 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4771 Patch attached. I would prefer if you did something like this instead: 1) Add validate_idn_normalized function with the normalized IDN check to ipapython.dnsutil, have it raise ValueError if the check fails. (Also please get rid of the map() call for better readability.) 2) Use validate_idn_normalized in DNSNameParam. 3) Do the following in validate_zonemgr_str: validate_idn_normalized(zonemgr) try: zonemgr = DNSName(zonemgr) except dns.exception.DNSException as e: raise ValueError(e) Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 380 Improve validation of --instance and --backend options in ipa-restore
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4744.
Honza
--
Jan Cholasta
From 204a065d67a65d9ed43e5eaa22958cd8daf7b15f Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 1 Dec 2014 12:12:15 +
Subject: [PATCH] Improve validation of --instance and --backend options in
ipa-restore
https://fedorahosted.org/freeipa/ticket/4744
---
ipaplatform/base/paths.py| 2 +-
ipaserver/install/ipa_backup.py | 2 +-
ipaserver/install/ipa_restore.py | 73
3 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index c4cdc58..3389042 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -262,7 +262,7 @@ class BasePathNamespace(object):
VAR_LIB_DIRSRV_INSTANCE_SCRIPTS_TEMPLATE = /var/lib/dirsrv/scripts-%s
VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE = /var/lib/dirsrv/slapd-%s
SLAPD_INSTANCE_BACKUP_DIR_TEMPLATE = /var/lib/dirsrv/slapd-%s/bak/%s
-IPACA_DIRSRV_INSTANCE_DB_TEMPLATE = /var/lib/dirsrv/slapd-%s/db/ipaca
+SLAPD_INSTANCE_DB_DIR_TEMPLATE = /var/lib/dirsrv/slapd-%s/db/%s
SLAPD_INSTANCE_LDIF_DIR_TEMPLATE = /var/lib/dirsrv/slapd-%s/ldif
VAR_LIB_SLAPD_PKI_IPA_DIR_TEMPLATE = /var/lib/dirsrv/slapd-PKI-IPA
VAR_LIB_IPA = /var/lib/ipa
diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py
index 5d583f7..72d1475 100644
--- a/ipaserver/install/ipa_backup.py
+++ b/ipaserver/install/ipa_backup.py
@@ -292,7 +292,7 @@ class Backup(admintool.AdminTool):
for instance in [realm_to_serverid(api.env.realm), 'PKI-IPA']:
if os.path.exists(paths.VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE % instance):
-if os.path.exists(paths.IPACA_DIRSRV_INSTANCE_DB_TEMPLATE % instance):
+if os.path.exists(paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE % (instance, 'ipaca')):
self.db2ldif(instance, 'ipaca', online=options.online)
self.db2ldif(instance, 'userRoot', online=options.online)
self.db2bak(instance, online=options.online)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 9cb978a..0977039 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -188,15 +188,35 @@ class Restore(admintool.AdminTool):
self.log.info(Preparing restore from %s on %s,
self.backup_dir, api.env.host)
-if not options.instance:
-instances = []
-for instance in [realm_to_serverid(api.env.realm), 'PKI-IPA']:
-if os.path.exists(paths.VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE % instance):
-instances.append(instance)
+if options.instance and options.backend:
+database = (options.instance, options.backend)
+if not os.path.exists(paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
+ database):
+raise admintool.ScriptError(
+Instance %s with backend %s does not exist % database)
+
+databases = [database]
else:
-instances = [options.instance]
-if options.data_only and not instances:
-raise admintool.ScriptError('No instances to restore to')
+if options.instance:
+instances = [options.instance]
+else:
+instances = [realm_to_serverid(api.env.realm), 'PKI-IPA']
+
+if options.backend:
+backends = [options.backend]
+else:
+backends = ['userRoot', 'ipaca']
+
+databases = []
+for instance in instances:
+for backend in backends:
+database = (instance, backend)
+if os.path.exists(paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
+ database):
+databases.append(database)
+
+if options.data_only and not databases:
+raise admintool.ScriptError('No instances to restore to')
create_ds_user()
pent = pwd.getpwnam(DS_USER)
@@ -223,7 +243,7 @@ class Restore(admintool.AdminTool):
# These two checks would normally be in the validate method but
# we need to know the type of backup we're dealing with.
if (self.backup_type != 'FULL' and not options.data_only and
-not instances):
+not databases):
raise admintool.ScriptError('Cannot restore a data backup into an empty system')
if (self.backup_type == 'FULL' and not options.data_only and
(options.instance or options.backend)):
@@ -244,6 +264,15 @@ class Restore(admintool.AdminTool):
not user_input(Continue to restore?, False)):
raise admintool.ScriptError(Aborted
Re: [Freeipa-devel] [PATCH 0173] Throw zonemgr error message before installation proceeds
Dne 1.12.2014 v 09:10 Jan Cholasta napsal(a): Hi, Dne 27.11.2014 v 14:20 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4771 Patch attached. I would prefer if you did something like this instead: 1) Add validate_idn_normalized function with the normalized IDN check to ipapython.dnsutil, have it raise ValueError if the check fails. (Also please get rid of the map() call for better readability.) 2) Use validate_idn_normalized in DNSNameParam. 3) Do the following in validate_zonemgr_str: validate_idn_normalized(zonemgr) try: zonemgr = DNSName(zonemgr) except dns.exception.DNSException as e: raise ValueError(e) Honza Actually, sratch that, exceptions thrown by python-dns do not have messages. ACK. Pushed to: master: ca25c92ea89661755d7204ac703e8c419c8929fa ipa-4-1: 07e29d250550f238e5706b348d69632fdbb67bda Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 792 add --hosts option to allow/retrieve keytab methods
Hi, Dne 1.12.2014 v 14:17 Petr Vobornik napsal(a): `--hosts` option added to: * service-allow-create-keytab * service-allow-retrieve-keytab * service-disallow-create-keytab * service-disallow-retrieve-keytab * host-allow-create-keytab * host-allow-retrieve-keytab * host-disallow-create-keytab * host-disallow-retrieve-keytab in order to allow hosts to retrieve keytab of their services or related hosts as described on http://www.freeipa.org/page/V4/Keytab_Retrieval design page https://fedorahosted.org/freeipa/ticket/4777 Since groups of users are supported with group members, we should probably also support groups of hosts with hostgroup members, for consistency. I'm pondering how to handle Web UI. I'm not font of adding a third pair of tables to host and service details pages because the amount of space on the page required for the keytab management is much bigger than its importance compared to other fields. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert
Hi, Dne 2.12.2014 v 13:16 Tomas Babej napsal(a): Hi, For CA certificates that are not certificates of IPA CA, we incorrectly set the trust flags to ,,, regardless what the actual trust_flags parameter was passed. Make the load_cacert method respect trust_flags and make C,, default set of trust flags. For unknown CA certificates, you must keep the default ,, and explicitly override it where necessary. We don't want to trust *any* CA certificate to issue server certs. https://fedorahosted.org/freeipa/ticket/4779 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert
Dne 2.12.2014 v 13:55 Tomas Babej napsal(a): On 12/02/2014 01:45 PM, Jan Cholasta wrote: Hi, Dne 2.12.2014 v 13:16 Tomas Babej napsal(a): Hi, For CA certificates that are not certificates of IPA CA, we incorrectly set the trust flags to ,,, regardless what the actual trust_flags parameter was passed. Make the load_cacert method respect trust_flags and make C,, default set of trust flags. For unknown CA certificates, you must keep the default ,, and explicitly override it where necessary. We don't want to trust *any* CA certificate to issue server certs. https://fedorahosted.org/freeipa/ticket/4779 Honza Updated patch attached. However, this boils down to the same, so there is really no functional difference between the two versions of the patches in the current code base. All places where load_cacert is called, the trust flags are explicitly overriden. OK, then we don't need a default value at all. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert
Dne 2.12.2014 v 14:09 Tomas Babej napsal(a): On 12/02/2014 02:02 PM, Jan Cholasta wrote: Dne 2.12.2014 v 13:55 Tomas Babej napsal(a): On 12/02/2014 01:45 PM, Jan Cholasta wrote: Hi, Dne 2.12.2014 v 13:16 Tomas Babej napsal(a): Hi, For CA certificates that are not certificates of IPA CA, we incorrectly set the trust flags to ,,, regardless what the actual trust_flags parameter was passed. Make the load_cacert method respect trust_flags and make C,, default set of trust flags. For unknown CA certificates, you must keep the default ,, and explicitly override it where necessary. We don't want to trust *any* CA certificate to issue server certs. https://fedorahosted.org/freeipa/ticket/4779 Honza Updated patch attached. However, this boils down to the same, so there is really no functional difference between the two versions of the patches in the current code base. All places where load_cacert is called, the trust flags are explicitly overriden. OK, then we don't need a default value at all. Updated patch makes trust_flags a required argument of load_cacert. Thanks, ACK! Pushed to: master: faec4ef9de431a1b72423be8ce6cea28a7221531 ipa-4-1: db4ac4774523c1d41a606b1c0297e9eeae13ebd6 Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0289] hosts: Display assigned ID view by default in host-find and show
Hi, Dne 2.12.2014 v 15:43 Tomas Babej napsal(a): Hi, Makes ipaassignedidview a default attribute and takes care about the conversion from the DN to the proper ID view name. https://fedorahosted.org/freeipa/ticket/4774 Since you are converting the value from DN to primary key string, the type of the ipassignedview param should be changed to Str, for consistency with existing code. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0289] hosts: Display assigned ID view by default in host-find and show
Dne 2.12.2014 v 17:01 Tomas Babej napsal(a): On 12/02/2014 04:14 PM, Jan Cholasta wrote: Hi, Dne 2.12.2014 v 15:43 Tomas Babej napsal(a): Hi, Makes ipaassignedidview a default attribute and takes care about the conversion from the DN to the proper ID view name. https://fedorahosted.org/freeipa/ticket/4774 Since you are converting the value from DN to primary key string, the type of the ipassignedview param should be changed to Str, for consistency with existing code. Honza I see. Actually during the development, I craved for simple output_normalizer option in the Param itself, which would apply the output normalization funtion after the post_callback, instead of having to mangle the entry_attrs in the post callback of each command (and could potentionally apply on the client). Would there a be not-so-hard way to do this in the framework? My understanding is that Output classes are quite decoupled from the Params they resulted from, and at the point we're printing the information via the textui, we're no longer aware what Param instance it originated from. This wouldn't solve the real problem in this case, which is that we do not support one-to-many relationships between objects in the framework (and our support for many-to-many relationships is clunky). I plan to fix this with https://fedorahosted.org/freeipa/ticket/3932. Updated patch attached. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 792 add --hosts option to allow/retrieve keytab methods
Dne 1.12.2014 v 19:25 Petr Vobornik napsal(a):
On 12/01/2014 02:33 PM, Jan Cholasta wrote:
Hi,
Dne 1.12.2014 v 14:17 Petr Vobornik napsal(a):
`--hosts` option added to:
* service-allow-create-keytab
* service-allow-retrieve-keytab
* service-disallow-create-keytab
* service-disallow-retrieve-keytab
* host-allow-create-keytab
* host-allow-retrieve-keytab
* host-disallow-create-keytab
* host-disallow-retrieve-keytab
in order to allow hosts to retrieve keytab of their services or related
hosts as described on http://www.freeipa.org/page/V4/Keytab_Retrieval
design page
https://fedorahosted.org/freeipa/ticket/4777
Since groups of users are supported with group members, we should
probably also support groups of hosts with hostgroup members, for
consistency.
--hostgroup options added.
Thanks, ACK.
Fixed a typo in host.py:
+label=_('Hosts Groups allowed to create keytab'),
^
and pushed to:
master: 026c9eca0920e92e56148b808c851e9bde00ece8
ipa-4-1: 1108e7145538f84da2e0dfdf4fb0e76583575dd2
I'm pondering how to handle Web UI. I'm not font of adding a third pair
of tables to host and service details pages because the amount of space
on the page required for the keytab management is much bigger than its
importance compared to other fields.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 382 Fix automatic CA cert renewal endless loop in dogtag-ipa-ca-renew-agent
Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4765. Honza -- Jan Cholasta From 5e541c915c3165328bca199f295164a2a9b509e2 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Wed, 3 Dec 2014 07:43:15 + Subject: [PATCH] Fix automatic CA cert renewal endless loop in dogtag-ipa-ca-renew-agent Reset profile name after requesting the CA cert from Dogtag to prevent the automatic renewal request from being restarted in subsequent calls. https://fedorahosted.org/freeipa/ticket/4765 --- install/certmonger/dogtag-ipa-ca-renew-agent-submit | 2 ++ 1 file changed, 2 insertions(+) diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit index 0a2cff1..e0dd33f 100755 --- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit +++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit @@ -408,8 +408,10 @@ def renew_ca_cert(): IPA CA certificate is about to expire, use ipa-cacert-manage to renew it) elif state == 'request': +profile = os.environ['CERTMONGER_CA_PROFILE'] os.environ['CERTMONGER_CA_PROFILE'] = 'caCACert' result = call_handler(request_and_store_cert) +os.environ['CERTMONGER_CA_PROFILE'] = profile if result[0] == WAIT: return (result[0], '%s:%s' % (state, result[1])) -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew
Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4781. Honza -- Jan Cholasta From d1d323fa046a9aabed08571c2be2d91a02e866e0 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Thu, 4 Dec 2014 08:15:46 + Subject: [PATCH] Check subject name encoding in ipa-cacert-manage renew https://fedorahosted.org/freeipa/ticket/4781 --- ipaserver/install/ipa_cacert_manage.py | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py index 2a8d95f..8fda6a2 100644 --- a/ipaserver/install/ipa_cacert_manage.py +++ b/ipaserver/install/ipa_cacert_manage.py @@ -213,18 +213,21 @@ class CACertManage(admintool.AdminTool): try: nss_cert = x509.load_certificate(old_cert, x509.DER) subject = nss_cert.subject +der_subject = x509.get_der_subject(old_cert, x509.DER) #pylint: disable=E1101 pkinfo = nss_cert.subject_public_key_info.format() #pylint: enable=E1101 nss_cert = x509.load_certificate_from_file(cert_file.name) +cert = nss_cert.der_data if nss_cert.subject != subject: raise admintool.ScriptError(Subject name mismatch) +if x509.get_der_subject(cert, x509.DER) != der_subject: +raise admintool.ScriptError(Subject name encoding mismatch) #pylint: disable=E1101 if nss_cert.subject_public_key_info.format() != pkinfo: raise admintool.ScriptError(Subject public key info mismatch) #pylint: enable=E1101 -cert = nss_cert.der_data finally: del nss_cert nss.nss_shutdown() @@ -238,7 +241,7 @@ class CACertManage(admintool.AdminTool): tmpdb.add_cert(cert, 'IPA CA', 'C,,') except ipautil.CalledProcessError, e: raise admintool.ScriptError( -Not compatible with the current CA certificate: %s, e) +Not compatible with the current CA certificate: %s % e) ca_certs = x509.load_certificate_list_from_file(ca_file.name) for ca_cert in ca_certs: -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 384 Do not renew the IPA CA cert by serial number in dogtag-ipa-ca-renew-agent
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4784.
Honza
--
Jan Cholasta
From 1e268143669621c01973859590af0a22d80255bf Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 4 Dec 2014 15:34:55 +
Subject: [PATCH] Do not renew the IPA CA cert by serial number in
dogtag-ipa-ca-renew-agent
Always use the full CSR when renewing the IPA CA certificate with Dogtag. The
IPA CA certificate may be issued by an external CA, in which case renewal by
serial number does not make sense and will fail if the IPA CA was initially
installed as a subordinate of an external CA.
https://fedorahosted.org/freeipa/ticket/4784
---
install/certmonger/dogtag-ipa-ca-renew-agent-submit | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index 0a2cff1..2500313 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -147,7 +147,7 @@ def request_cert():
path = paths.DOGTAG_IPA_RENEW_AGENT_SUBMIT
args = [path] + sys.argv[1:]
if os.environ.get('CERTMONGER_CA_PROFILE') == 'caCACert':
-args += ['-O', 'bypassCAnotafter=true']
+args += ['-N', '-O', 'bypassCAnotafter=true']
stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ)
sys.stderr.write(stderr)
sys.stderr.flush()
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew
Dne 5.12.2014 v 09:03 Martin Kosek napsal(a): On 12/04/2014 09:36 AM, Jan Cholasta wrote: +if x509.get_der_subject(cert, x509.DER) != der_subject: +raise admintool.ScriptError(Subject name encoding mismatch) I think we can expect this to be a pretty common error, given this is the default behavior of Microsoft Certificate Services. I would thus like to make the error message more juicy. We need to make sure we offer some pointers for these users or they will just blame IPA for screwing up. So, the information I wrote https://bugzilla.redhat.com/show_bug.cgi?id=1129558#c11 need to somehow get to the error message as a potential/likely root cause of the problem. Whether you write it in the error message itself or update the design page and just insert a link is up to you. Martin I would rather document this and have users read the documentation, which they should do anyway when something goes wrong. There are many errors in IPA which are common and users may blame IPA for them and I don't see what makes this one so special that it should require a special treatment. Anyway, I have created http://www.freeipa.org/page/Troubleshooting#External_CA_renewal_with_ipa-cacert-manage_fails. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew
Dne 5.12.2014 v 11:43 Martin Kosek napsal(a): On 12/05/2014 11:34 AM, Jan Cholasta wrote: Dne 5.12.2014 v 09:03 Martin Kosek napsal(a): On 12/04/2014 09:36 AM, Jan Cholasta wrote: +if x509.get_der_subject(cert, x509.DER) != der_subject: +raise admintool.ScriptError(Subject name encoding mismatch) I think we can expect this to be a pretty common error, given this is the default behavior of Microsoft Certificate Services. I would thus like to make the error message more juicy. We need to make sure we offer some pointers for these users or they will just blame IPA for screwing up. So, the information I wrote https://bugzilla.redhat.com/show_bug.cgi?id=1129558#c11 need to somehow get to the error message as a potential/likely root cause of the problem. Whether you write it in the error message itself or update the design page and just insert a link is up to you. Martin I would rather document this and have users read the documentation, which they should do anyway when something goes wrong. There are many errors in IPA which are common and users may blame IPA for them and I don't see what makes this one so special that it should require a special treatment. I saw several reasons: - Certificateinstallation error are more common than the others and users are usually quite lost in what to do with them. - In this case, we know by 90% probability what is the root cause - It will block one of the main use cases for the new CA renewal tool and people will likely hit it as MS CAs is one of the most common CAs and this is it's default behavior. Giving more details in this case will not hurt us, but benefit users. So I still do not see the harm. I do not see a harm either, my point is that we should probably point the user to documentation when *anything* in *any* script goes wrong, not just when some arbitrarily cherry-picked error occurs. Anyway, I have created http://www.freeipa.org/page/Troubleshooting#External_CA_renewal_with_ipa-cacert-manage_fails. Good. Do you plan to reference the section or enhance the error message? I plan to reference http://www.freeipa.org/page/Troubleshooting. Martin -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew
Dne 5.12.2014 v 12:01 Jan Cholasta napsal(a): Dne 5.12.2014 v 11:43 Martin Kosek napsal(a): On 12/05/2014 11:34 AM, Jan Cholasta wrote: Dne 5.12.2014 v 09:03 Martin Kosek napsal(a): On 12/04/2014 09:36 AM, Jan Cholasta wrote: +if x509.get_der_subject(cert, x509.DER) != der_subject: +raise admintool.ScriptError(Subject name encoding mismatch) I think we can expect this to be a pretty common error, given this is the default behavior of Microsoft Certificate Services. I would thus like to make the error message more juicy. We need to make sure we offer some pointers for these users or they will just blame IPA for screwing up. So, the information I wrote https://bugzilla.redhat.com/show_bug.cgi?id=1129558#c11 need to somehow get to the error message as a potential/likely root cause of the problem. Whether you write it in the error message itself or update the design page and just insert a link is up to you. Martin I would rather document this and have users read the documentation, which they should do anyway when something goes wrong. There are many errors in IPA which are common and users may blame IPA for them and I don't see what makes this one so special that it should require a special treatment. I saw several reasons: - Certificateinstallation error are more common than the others and users are usually quite lost in what to do with them. - In this case, we know by 90% probability what is the root cause - It will block one of the main use cases for the new CA renewal tool and people will likely hit it as MS CAs is one of the most common CAs and this is it's default behavior. Giving more details in this case will not hurt us, but benefit users. So I still do not see the harm. I do not see a harm either, my point is that we should probably point the user to documentation when *anything* in *any* script goes wrong, not just when some arbitrarily cherry-picked error occurs. Anyway, I have created http://www.freeipa.org/page/Troubleshooting#External_CA_renewal_with_ipa-cacert-manage_fails. Good. Do you plan to reference the section or enhance the error message? I plan to reference http://www.freeipa.org/page/Troubleshooting. See the attached patch (385). Martin -- Jan Cholasta From b5a4da2119eb6a57750fdb55bec5a5ad1c6db669 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Tue, 9 Dec 2014 12:47:58 + Subject: [PATCH] Refer the user to freeipa.org when something goes wrong in ipa-cacert-manage https://fedorahosted.org/freeipa/ticket/4781 --- ipaserver/install/ipa_cacert_manage.py | 8 1 file changed, 8 insertions(+) diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py index 8fda6a2..f73c7fe 100644 --- a/ipaserver/install/ipa_cacert_manage.py +++ b/ipaserver/install/ipa_cacert_manage.py @@ -120,6 +120,14 @@ class CACertManage(admintool.AdminTool): return rc +def log_failure(self, error_message, return_value, exception, backtrace): +super(CACertManage, self).log_failure( +error_message, return_value, exception, backtrace) + +if isinstance(exception, admintool.ScriptError): +print(\nVisit http://www.freeipa.org/page/Troubleshooting for + troubleshooting guide) + def ldap_connect(self): conn = ldap2() -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 380 Improve validation of --instance and --backend options in ipa-restore
Dne 9.12.2014 v 14:27 David Kupka napsal(a): On 12/01/2014 01:16 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4744. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Works for me, ACK. Thanks for the review. Pushed to: master: 7b0149f32b95b42598dd30acde4d2c589ffcfce1 ipa-4-1: f92d0efca693ddcf4de0bc3413ab26c76bad6963 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 793-794 Fix schema related replication issues between IPA-3-0 and IPA-4-1
Hi, Dne 5.12.2014 v 14:33 Petr Vobornik napsal(a): Hello, I've transformed Thierry's and Ludwig's findings of bz 1167964 [1] and ticket 4794 [2] into patches. I wonder if the mgrpRFC822MailMember and nsViewFilter issue(patch 794) should be solved on 389's side rather than on FreeIPA's? Also is the increase of nsslapd-sasl-max-buffer-size necessary? With these two patches, replication appears to work fine for me. Tested with F21 FreeIPA 4.1.x-GIT-something and ipa-server-3.0.0-42.el6.x86_64 [1] https://bugzilla.redhat.com/show_bug.cgi?id=1167964 [2] https://fedorahosted.org/freeipa/ticket/4794 Patch 793: Works for me, ACK. Pushed to: master: 489dfe64689f86f7ddc4ad0784de0636f8e6c1f8 ipa-4-1: 2fa07b1d24f61f9bcff5adb804a18c9eae72932d Patch 794: As Thierry pointed out, this patch is not necessary, as the bug is fixed by patch 793 alone. Not pushed. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0168] Better workaround to get status of CA during upgrade
Dne 1.12.2014 v 16:48 Martin Basti napsal(a): On 01/12/14 08:46, Jan Cholasta wrote: Hi, Dne 27.11.2014 v 14:24 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4676 Replaces current workaround. Should go to 4.1.3. Patch attached. When constructing URLs with host:port, please use ipautil.format_netloc(). wget should be added as a dependency of freeipa-python in the spec file. Honza Updated patch attached. Thanks, ACK. Pushed to: master: 337faf506462a01c6dbcd00f2039ed5627691864 ipa-4-1: 5052af773f652bc19e91fe49e15351e5c5c7d976 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew
Dne 9.12.2014 v 13:03 David Kupka napsal(a): On 12/04/2014 09:36 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4781. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Works for me, ACK. Thanks for the review. Pushed to: master: f7f3c83748b3b5d5d968cc3c72145f3c5f23cd8b ipa-4-1: 731035e526441b93b69fb20c6a6c990cdcdc4899 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew
Dne 10.12.2014 v 17:53 Martin Basti napsal(a): On 10/12/14 16:02, Martin Kosek wrote: On 12/10/2014 02:35 PM, Jan Cholasta wrote: Dne 10.12.2014 v 11:53 Martin Kosek napsal(a): On 12/09/2014 01:56 PM, Jan Cholasta wrote: Dne 5.12.2014 v 12:01 Jan Cholasta napsal(a): Dne 5.12.2014 v 11:43 Martin Kosek napsal(a): On 12/05/2014 11:34 AM, Jan Cholasta wrote: Dne 5.12.2014 v 09:03 Martin Kosek napsal(a): On 12/04/2014 09:36 AM, Jan Cholasta wrote: +if x509.get_der_subject(cert, x509.DER) != der_subject: +raise admintool.ScriptError(Subject name encoding mismatch) I think we can expect this to be a pretty common error, given this is the default behavior of Microsoft Certificate Services. I would thus like to make the error message more juicy. We need to make sure we offer some pointers for these users or they will just blame IPA for screwing up. So, the information I wrote https://bugzilla.redhat.com/show_bug.cgi?id=1129558#c11 need to somehow get to the error message as a potential/likely root cause of the problem. Whether you write it in the error message itself or update the design page and just insert a link is up to you. Martin I would rather document this and have users read the documentation, which they should do anyway when something goes wrong. There are many errors in IPA which are common and users may blame IPA for them and I don't see what makes this one so special that it should require a special treatment. I saw several reasons: - Certificateinstallation error are more common than the others and users are usually quite lost in what to do with them. - In this case, we know by 90% probability what is the root cause - It will block one of the main use cases for the new CA renewal tool and people will likely hit it as MS CAs is one of the most common CAs and this is it's default behavior. Giving more details in this case will not hurt us, but benefit users. So I still do not see the harm. I do not see a harm either, my point is that we should probably point the user to documentation when *anything* in *any* script goes wrong, not just when some arbitrarily cherry-picked error occurs. Anyway, I have created http://www.freeipa.org/page/Troubleshooting#External_CA_renewal_with_ipa-cacert-manage_fails. Good. Do you plan to reference the section or enhance the error message? I plan to reference http://www.freeipa.org/page/Troubleshooting. See the attached patch (385). I think the reference for the Troubleshooting page should be more narrow so that people only see the URL only for the cases we give specific advise for. Otherwise I assume they will just ignore the page if they do not find the advise for other errors. Right, makes sense. Other idea would be to give reference to the article when the actual CSR is generated - as a heads up. I think referring to troubleshooting before there actually is some trouble is not very good for publicity. Ah, that's a good point - in this purpose it would be better to have it somewhere else or only refer to the MS article. Anyway, updated patch attached, it implements what you suggested originally - link to the troubleshooting guide is added to relevant error messages. Let's think about this in more broad terms when the time comes for the installer refactoring. Ok. I am fine with the patch conceptually. So now just someone (David?) needs to make sure it did not break anything :-) Martin ACK, seems it doesnt break anything. Thanks for the review. Pushed to: master: 8f9c5988e2f370cef66a4cd7cf3d363f061a439c ipa-4-1: 3cb2f5e841f5bac6a8cc02bc9467846b35f7aab8 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0168] Better workaround to get status of CA during upgrade
Dne 10.12.2014 v 18:01 Jan Cholasta napsal(a): Dne 1.12.2014 v 16:48 Martin Basti napsal(a): On 01/12/14 08:46, Jan Cholasta wrote: Hi, Dne 27.11.2014 v 14:24 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4676 Replaces current workaround. Should go to 4.1.3. Patch attached. When constructing URLs with host:port, please use ipautil.format_netloc(). wget should be added as a dependency of freeipa-python in the spec file. Honza Updated patch attached. Thanks, ACK. Pushed to: master: 337faf506462a01c6dbcd00f2039ed5627691864 ipa-4-1: 5052af773f652bc19e91fe49e15351e5c5c7d976 It turns out I messed up the review (sorry). This fixes the upgrade, but it also breaks ipa-server-install: 2014-12-10T06:06:44Z DEBUG [8/27]: starting certificate server instance 2014-12-10T06:06:44Z DEBUG Starting external process 2014-12-10T06:06:44Z DEBUG args='/bin/systemctl' 'start' 'pki-tomcatd.target' 2014-12-10T06:06:45Z DEBUG Process finished, return code=0 2014-12-10T06:06:45Z DEBUG stdout= 2014-12-10T06:06:45Z DEBUG stderr= 2014-12-10T06:06:45Z DEBUG Starting external process 2014-12-10T06:06:45Z DEBUG args='/bin/systemctl' 'is-active' 'pki-tomcatd.target' 2014-12-10T06:06:45Z DEBUG Process finished, return code=0 2014-12-10T06:06:45Z DEBUG stdout=active 2014-12-10T06:06:45Z DEBUG stderr= 2014-12-10T06:06:45Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 300 2014-12-10T06:06:49Z DEBUG The httpd proxy is not installed, wait on local port 2014-12-10T06:06:49Z DEBUG Waiting until the CA is running 2014-12-10T06:06:49Z DEBUG Starting external process 2014-12-10T06:06:49Z DEBUG args='/usr/bin/wget' '-S' '-O' '-' '--timeout=30' 'https://vm-088.idm.lab.bos.redhat.com:8443/ca/admin/ca/getStatus' 2014-12-10T06:07:09Z DEBUG Process finished, return code=5 2014-12-10T06:07:09Z DEBUG stdout= 2014-12-10T06:07:09Z DEBUG stderr=--2014-12-10 01:06:49-- https://vm-088.idm.lab.bos.redhat.com:8443/ca/admin/ca/getStatus Resolving vm-088.idm.lab.bos.redhat.com (vm-088.idm.lab.bos.redhat.com)... 10.16.78.88 Connecting to vm-088.idm.lab.bos.redhat.com (vm-088.idm.lab.bos.redhat.com)|10.16.78.88|:8443... connected. ERROR: cannot verify vm-088.idm.lab.bos.redhat.com's certificate, issued by ‘/O=IDM.LAB.BOS.REDHAT.COM/CN=Certificate Authority’: Self-signed certificate encountered. To connect to vm-088.idm.lab.bos.redhat.com insecurely, use `--no-check-certificate'. 2014-12-10T06:07:09Z DEBUG The CA status is: check interrupted I have reopened the ticket. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0174] Show SSHFP record in CLI if contains space in fingerprint part
Hi, Dne 4.12.2014 v 15:22 Martin Basti napsal(a): SSHFP records added by nsupdate contains space in fingerprint part, this space prevent framework to show record. Fixes https://fedorahosted.org/freeipa/ticket/4789 https://fedorahosted.org/freeipa/ticket/4790 Patch attached. Thanks, ACK. Pushed to: master: b5ff0b941efad5170ff5fdda4ab05b9f1c7a2113 ipa-4-1: d229c4a1cc397cfe6adf661b6bcc8360a758248c Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0177] Fix adding (warning) messages on client side
Hi,
Dne 9.12.2014 v 16:07 Martin Basti napsal(a):
Ticket: https://fedorahosted.org/freeipa/ticket/4793
I'm able to reproduce it only in one nose test.
Which test?
Patch attached.
What about:
result['messages'] = result.get('messages', ()) + (message.to_dict(),)
(My point is, don't support both lists and tuples, pick just one.)
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0168] Better workaround to get status of CA during upgrade
Dne 11.12.2014 v 10:01 Martin Basti napsal(a): On 10/12/14 19:21, Jan Cholasta wrote: Dne 10.12.2014 v 18:01 Jan Cholasta napsal(a): Dne 1.12.2014 v 16:48 Martin Basti napsal(a): On 01/12/14 08:46, Jan Cholasta wrote: Hi, Dne 27.11.2014 v 14:24 Martin Basti napsal(a): Ticket: https://fedorahosted.org/freeipa/ticket/4676 Replaces current workaround. Should go to 4.1.3. Patch attached. When constructing URLs with host:port, please use ipautil.format_netloc(). wget should be added as a dependency of freeipa-python in the spec file. Honza Updated patch attached. Thanks, ACK. Pushed to: master: 337faf506462a01c6dbcd00f2039ed5627691864 ipa-4-1: 5052af773f652bc19e91fe49e15351e5c5c7d976 It turns out I messed up the review (sorry). This fixes the upgrade, but it also breaks ipa-server-install: 2014-12-10T06:06:44Z DEBUG [8/27]: starting certificate server instance 2014-12-10T06:06:44Z DEBUG Starting external process 2014-12-10T06:06:44Z DEBUG args='/bin/systemctl' 'start' 'pki-tomcatd.target' 2014-12-10T06:06:45Z DEBUG Process finished, return code=0 2014-12-10T06:06:45Z DEBUG stdout= 2014-12-10T06:06:45Z DEBUG stderr= 2014-12-10T06:06:45Z DEBUG Starting external process 2014-12-10T06:06:45Z DEBUG args='/bin/systemctl' 'is-active' 'pki-tomcatd.target' 2014-12-10T06:06:45Z DEBUG Process finished, return code=0 2014-12-10T06:06:45Z DEBUG stdout=active 2014-12-10T06:06:45Z DEBUG stderr= 2014-12-10T06:06:45Z DEBUG wait_for_open_ports: localhost [8080, 8443] timeout 300 2014-12-10T06:06:49Z DEBUG The httpd proxy is not installed, wait on local port 2014-12-10T06:06:49Z DEBUG Waiting until the CA is running 2014-12-10T06:06:49Z DEBUG Starting external process 2014-12-10T06:06:49Z DEBUG args='/usr/bin/wget' '-S' '-O' '-' '--timeout=30' 'https://vm-088.idm.lab.bos.redhat.com:8443/ca/admin/ca/getStatus' 2014-12-10T06:07:09Z DEBUG Process finished, return code=5 2014-12-10T06:07:09Z DEBUG stdout= 2014-12-10T06:07:09Z DEBUG stderr=--2014-12-10 01:06:49-- https://vm-088.idm.lab.bos.redhat.com:8443/ca/admin/ca/getStatus Resolving vm-088.idm.lab.bos.redhat.com (vm-088.idm.lab.bos.redhat.com)... 10.16.78.88 Connecting to vm-088.idm.lab.bos.redhat.com (vm-088.idm.lab.bos.redhat.com)|10.16.78.88|:8443... connected. ERROR: cannot verify vm-088.idm.lab.bos.redhat.com's certificate, issued by ‘/O=IDM.LAB.BOS.REDHAT.COM/CN=Certificate Authority’: Self-signed certificate encountered. To connect to vm-088.idm.lab.bos.redhat.com insecurely, use `--no-check-certificate'. 2014-12-10T06:07:09Z DEBUG The CA status is: check interrupted I have reopened the ticket. Patch with '--no-check-certificate' option attached. Before workaround there was no certificate check, so it should not be problem if we ignore the certificate. Martin^2 Thanks, ACK. Pushed to: master: 95becc1d542c78721088398eddbfd0d0ffe9b27f ipa-4-1: 8440c2ee97e1c7e29e20629a2579af28a6d654be -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA integration with external DNS services
Hi, Dne 1.12.2014 v 17:12 Simo Sorce napsal(a): On Mon, 01 Dec 2014 16:17:54 +0100 Petr Spacek pspa...@redhat.com wrote: On 14.11.2014 17:31, Petr Spacek wrote: On 14.11.2014 02:22, Simo Sorce wrote: I think what I'd like to see is to be able to configure a DNS zone in LDAP and mark it external. The zone would held the TSIG keys necessary to perform DNS updates. When the regular ipa dnsrecord-add etc... commands are called, the framework detects that the zone is external, fewttches the TSIG key (if the user has access to it) and spawn an nsupdate command that will perform the update against whatever DNS server is authoritative for the zone. Would it be feasible to use FreeIPA server as XML-RPC-DNS proxy instead of nsupdate command (to hide TSIG key behind FreeIPA)? I do not like the XML-RPC-DNS approach as it requires a special client, leaving out the majority of clients. I'm confused. Above you have suggested hiding the nsupdate machinery behind the framework and now you are saying that you do not like the XML-RPC-DNS approach. But the framework talks XML-RPC (and JSON-RPC) and using *anything else* would require a special client. Or did you mean some other XML-RPC-DNS? Also, I am thinking that we only _need_ to set infrastructure relevant names (like IPA servers SRV records), but if someone decides not to use IPA for the DNS we may decide that it is not our responsibility to provide a full end-to-end client dns update solution. So I would concentrate on making it possible for IPA *Servers* to use a private TSIG key to update infrastructure relevant names, and possibly defer the clients side of the problem. We could use an internal bus on the server to allow the ipa framework to use nsupdate w/o gaining direct access to the TSIG key, this way admins can use ipa dnsrecod-add and friends w/o exposing the key. +1, we had a short discussion about external DNS with Petr yesterday and reached the same conclusion. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0003-2 User life cycle: new stageuser plugin with add verb
Hi, Dne 4.2.2015 v 15:25 David Kupka napsal(a): On 02/03/2015 11:50 AM, thierry bordaz wrote: On 09/17/2014 12:32 PM, thierry bordaz wrote: On 09/01/2014 01:08 PM, Petr Viktorin wrote: On 08/08/2014 03:54 PM, thierry bordaz wrote: Hi, The attached patch is related to 'User Life Cycle' (https://fedorahosted.org/freeipa/ticket/3813) It creates a stageuser plugin with a first function stageuser-add. Stage user entries are provisioned under 'cn=staged users,cn=accounts,cn=provisioning,SUFFIX'. Thanks thierry Avoid `from ipalib.plugins.baseldap import *` in new code; instead import the module itself and use e.g. `baseldap.LDAPObject`. The stageuser help (docstring) is copied from the user plugin, and discusses things like account lockout and disabling users. It should rather explain what stageuser itself does. (And I don't very much like the Note about the interface being badly designed...) Also decide if the docs should call it staged user or stage user or stageuser. A lot of the code is copied and pasted over from the users plugin. Don't do that. Either import things (e.g. validate_nsaccountlock) from the users plugin, or move the reused code into a shared module. For the `user` object, since so much is the same, it might be best to create a common base class for user and stageuser; and similarly for the Command plugins. The default permissions need different names, and you don't need another copy of the 'non_object' ones. Also, run the makeaci script. Hello, This modified patch is mainly moving common base class into a new plugin: accounts.py. user/stageuser plugin inherits from accounts. It also creates a better description of what are stage user, how to add a new stage user, updates ACI.txt and separate active/stage user managed permissions. thanks thierry ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Thanks David for the reviews. Here the last patches ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel The freeipa-tbordaz-0002 patch had trailing whitespaces on few lines so I'm attaching fixed version (and unchanged patch freeipa-tbordaz-0003-3 to keep them together). The ULC feature is still WIP but these patches look good to me and don't break anything as far as I tested. We should push them now to avoid further rebases. Thierry can then prepare other patches delivering the rest of ULC functionality. Few comments from just reading the patches: 1) I would name the base class baseuser, account does not necessarily mean user account. 2) This is very wrong: -class user_add(LDAPCreate): +class user_add(user, LDAPCreate): You are creating a plugin which is both an object and an command. 3) This is purely subjective, but I don't like the name deleteuser, as it has a verb in it. We usually don't do that and IMHO we shouldn't do that. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
Dne 15.1.2015 v 15:39 Martin Basti napsal(a): On 15/01/15 15:07, Jan Cholasta wrote: Dne 15.1.2015 v 14:58 Martin Basti napsal(a): On 15/01/15 14:25, Jan Cholasta wrote: Hi, Dne 15.1.2015 v 13:27 Martin Basti napsal(a): On 15/01/15 13:17, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4722 Patch attached. Fast fix. Updated patch attached. 1) Forward zone commands are not fixed. FWzones are new and always normalized to absolute name in ldap Would you bet your money on that? Better be safe than sorry, especially when it's just a matter of moving the code around (right?) 2) It seems that the primary key returned by -mod, -del and -show (.result.value) is made absolute somewhere else in the code. Would it be possible to do it in one place? IMO it is not possible. Value is generated from key, and key is normalized to absolute zone before calling execute. LDAPUpdate: ... if self.obj.primary_key: pkey = keys[-1] else: pkey = None return dict(result=entry_attrs, value=pkey_to_value(pkey, options)) The idnsname attribute is just taken from LDAP without any normalization Right. 3) Attribute values returned from LDAP are never None, so the if should be if 'idnsname' in entry_attrs:. Ok I will revert the change I made. 4) If idnsname always has only single value, use entry_attrs.single_value['idnsname'] = entry_attrs.single_value['idnsname'].make_absolute() Thanks Honza Updated patch attached. Updated patch attached. Thanks. Is there a reason why you put the _make_zone_absolute calls in dnszone_* and dnsforwardzone_* instead of DNSZoneBase_*? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 388 Remove RUV from LDIF files before using them in ipa-restore
Dne 13.1.2015 v 17:58 Jan Cholasta napsal(a):
Dne 13.1.2015 v 17:44 Petr Vobornik napsal(a):
On 01/12/2015 05:46 PM, Jan Cholasta wrote:
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4822.
Honza
works for me, ACK
Thanks, pushed to:
master: 05e6adecb51b93e9b9d2326df4eabee90c3dfe72
ipa-4-1: eb7917026d418a6d6a1e7a24a19097065df10497
Posting additional patch 394 which fixes a SELinux issue.
--
Jan Cholasta
From 6f88548e2302c1f99d0756afa351ce79b7e4ed67 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 20 Jan 2015 11:22:29 +
Subject: [PATCH] Put LDIF files to their original location in ipa-restore
This prevents SELinux failures during online data restore.
https://fedorahosted.org/freeipa/ticket/4822
---
ipaserver/install/ipa_restore.py | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index be48716..562a793 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -504,10 +504,17 @@ class Restore(admintool.AdminTool):
cn = time.strftime('import_%Y_%m_%d_%H_%M_%S')
dn = DN(('cn', cn), ('cn', 'import'), ('cn', 'tasks'), ('cn', 'config'))
+ldifdir = paths.SLAPD_INSTANCE_LDIF_DIR_TEMPLATE % instance
ldifname = '%s-%s.ldif' % (instance, backend)
+ldiffile = os.path.join(ldifdir, ldifname)
srcldiffile = os.path.join(self.dir, ldifname)
-ldiffile = '%s.noruv' % srcldiffile
+if not os.path.exists(ldifdir):
+pent = pwd.getpwnam(DS_USER)
+os.mkdir(ldifdir, 0770)
+os.chown(ldifdir, pent.pw_uid, pent.pw_gid)
+
+ipautil.backup_file(ldiffile)
with open(ldiffile, 'wb') as out_file:
ldif_writer = ldif.LDIFWriter(out_file)
with open(srcldiffile, 'rb') as in_file:
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 390 Do not crash on unknown services in installutils.stopped_service
Dne 13.1.2015 v 18:55 Jan Cholasta napsal(a):
Dne 13.1.2015 v 18:46 David Kupka napsal(a):
On 01/13/2015 05:55 PM, Jan Cholasta wrote:
Dne 13.1.2015 v 12:12 Jan Cholasta napsal(a):
Hi,
the attached patch fixes
https://fedorahosted.org/freeipa/ticket/4835.
Honza
Modified the fix to create only one service object in stopped_service.
Updated patch attached.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Hi!
Thanks for the patch. Works for me, ACK.
Thanks, pushed to:
master: 5bf1c9a6f7d734c296c8eb987cfc4f7e2a345130
ipa-4-1: 065e2bbc9f2260d8c60c55f92a386513727576da
Posting additional patch 393 which is necessary to properly fix this.
David, could you take a look please?
--
Jan Cholasta
From 92b37953087d5e396b1c3bf0a3b776558d6b894c Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 20 Jan 2015 09:38:43 +
Subject: [PATCH] Do not assume certmonger is running in httpinstance
https://fedorahosted.org/freeipa/ticket/4835
---
ipaserver/install/httpinstance.py | 48 +++
1 file changed, 29 insertions(+), 19 deletions(-)
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 2fb315b..cda85ab 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -39,6 +39,7 @@ from ipaserver.install import sysupgrade
from ipalib import api
from ipaplatform.tasks import tasks
from ipaplatform.paths import paths
+from ipaplatform import services
SELINUX_BOOLEAN_SETTINGS = dict(
@@ -228,25 +229,34 @@ class HTTPInstance(service.Service):
print Adding Include conf.d/ipa-rewrite to %s failed. % paths.HTTPD_NSS_CONF
def configure_certmonger_renewal_guard(self):
-bus = dbus.SystemBus()
-obj = bus.get_object('org.fedorahosted.certmonger',
- '/org/fedorahosted/certmonger')
-iface = dbus.Interface(obj, 'org.fedorahosted.certmonger')
-path = iface.find_ca_by_nickname('IPA')
-if path:
-ca_obj = bus.get_object('org.fedorahosted.certmonger', path)
-ca_iface = dbus.Interface(ca_obj,
- 'org.freedesktop.DBus.Properties')
-helper = ca_iface.Get('org.fedorahosted.certmonger.ca',
- 'external-helper')
-if helper:
-args = shlex.split(helper)
-if args[0] != paths.IPA_SERVER_GUARD:
-self.backup_state('certmonger_ipa_helper', helper)
-args = [paths.IPA_SERVER_GUARD] + args
-helper = ' '.join(pipes.quote(a) for a in args)
-ca_iface.Set('org.fedorahosted.certmonger.ca',
- 'external-helper', helper)
+certmonger = services.knownservices.certmonger
+certmonger_stopped = not certmonger.is_running()
+
+if certmonger_stopped:
+certmonger.start()
+try:
+bus = dbus.SystemBus()
+obj = bus.get_object('org.fedorahosted.certmonger',
+ '/org/fedorahosted/certmonger')
+iface = dbus.Interface(obj, 'org.fedorahosted.certmonger')
+path = iface.find_ca_by_nickname('IPA')
+if path:
+ca_obj = bus.get_object('org.fedorahosted.certmonger', path)
+ca_iface = dbus.Interface(ca_obj,
+ 'org.freedesktop.DBus.Properties')
+helper = ca_iface.Get('org.fedorahosted.certmonger.ca',
+ 'external-helper')
+if helper:
+args = shlex.split(helper)
+if args[0] != paths.IPA_SERVER_GUARD:
+self.backup_state('certmonger_ipa_helper', helper)
+args = [paths.IPA_SERVER_GUARD] + args
+helper = ' '.join(pipes.quote(a) for a in args)
+ca_iface.Set('org.fedorahosted.certmonger.ca',
+ 'external-helper', helper)
+finally:
+if certmonger_stopped:
+certmonger.stop()
def __setup_ssl(self):
fqdn = self.fqdn
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
Hi, Dne 15.1.2015 v 13:27 Martin Basti napsal(a): On 15/01/15 13:17, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4722 Patch attached. Fast fix. Updated patch attached. 1) Forward zone commands are not fixed. 2) It seems that the primary key returned by -mod, -del and -show (.result.value) is made absolute somewhere else in the code. Would it be possible to do it in one place? 3) Attribute values returned from LDAP are never None, so the if should be if 'idnsname' in entry_attrs:. 4) If idnsname always has only single value, use entry_attrs.single_value['idnsname'] = entry_attrs.single_value['idnsname'].make_absolute() Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
Dne 15.1.2015 v 14:58 Martin Basti napsal(a): On 15/01/15 14:25, Jan Cholasta wrote: Hi, Dne 15.1.2015 v 13:27 Martin Basti napsal(a): On 15/01/15 13:17, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4722 Patch attached. Fast fix. Updated patch attached. 1) Forward zone commands are not fixed. FWzones are new and always normalized to absolute name in ldap Would you bet your money on that? Better be safe than sorry, especially when it's just a matter of moving the code around (right?) 2) It seems that the primary key returned by -mod, -del and -show (.result.value) is made absolute somewhere else in the code. Would it be possible to do it in one place? IMO it is not possible. Value is generated from key, and key is normalized to absolute zone before calling execute. LDAPUpdate: ... if self.obj.primary_key: pkey = keys[-1] else: pkey = None return dict(result=entry_attrs, value=pkey_to_value(pkey, options)) The idnsname attribute is just taken from LDAP without any normalization Right. 3) Attribute values returned from LDAP are never None, so the if should be if 'idnsname' in entry_attrs:. Ok I will revert the change I made. 4) If idnsname always has only single value, use entry_attrs.single_value['idnsname'] = entry_attrs.single_value['idnsname'].make_absolute() Thanks Honza Updated patch attached. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 395 Revert Make all ipatokenTOTP attributes mandatory
Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4833. Honza -- Jan Cholasta From f5e6e45977b699bada1990f8231d0f142ab6fc61 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Wed, 21 Jan 2015 07:57:03 + Subject: [PATCH] Revert Make all ipatokenTOTP attributes mandatory This reverts commit adcd373931c50d91550f6b74b191d08ecce5b137. https://fedorahosted.org/freeipa/ticket/4833 --- install/share/70ipaotp.ldif | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/share/70ipaotp.ldif b/install/share/70ipaotp.ldif index b35ab62..0c48d9a 100644 --- a/install/share/70ipaotp.ldif +++ b/install/share/70ipaotp.ldif @@ -29,7 +29,7 @@ attributeTypes: (2.16.840.1.113730.3.8.16.1.24 NAME 'ipatokenTOTPsyncWindow' DES attributeTypes: (2.16.840.1.113730.3.8.16.1.25 NAME 'ipatokenHOTPauthWindow' DESC 'HOTP Auth Window (maximum authentication skip-ahead)' EQUALITY integerMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA OTP') attributeTypes: (2.16.840.1.113730.3.8.16.1.26 NAME 'ipatokenHOTPsyncWindow' DESC 'HOTP Sync Window (maximum synchronization skip-ahead)' EQUALITY integerMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.1 NAME 'ipaToken' SUP top ABSTRACT DESC 'Abstract token class for tokens' MUST (ipatokenUniqueID) MAY (description $ managedBy $ ipatokenOwner $ ipatokenDisabled $ ipatokenNotBefore $ ipatokenNotAfter $ ipatokenVendor $ ipatokenModel $ ipatokenSerial) X-ORIGIN 'IPA OTP') -objectClasses: (2.16.840.1.113730.3.8.16.2.2 NAME 'ipatokenTOTP' SUP ipaToken STRUCTURAL DESC 'TOTP Token Type' MUST (ipatokenOTPkey $ ipatokenOTPalgorithm $ ipatokenOTPdigits $ ipatokenTOTPclockOffset $ ipatokenTOTPtimeStep) MAY (ipatokenTOTPwatermark) X-ORIGIN 'IPA OTP') +objectClasses: (2.16.840.1.113730.3.8.16.2.2 NAME 'ipatokenTOTP' SUP ipaToken STRUCTURAL DESC 'TOTP Token Type' MAY (ipatokenOTPkey $ ipatokenOTPalgorithm $ ipatokenOTPdigits $ ipatokenTOTPclockOffset $ ipatokenTOTPtimeStep $ ipatokenTOTPwatermark) X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.3 NAME 'ipatokenRadiusProxyUser' SUP top AUXILIARY DESC 'Radius Proxy User' MAY (ipatokenRadiusConfigLink $ ipatokenRadiusUserName) X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.4 NAME 'ipatokenRadiusConfiguration' SUP top STRUCTURAL DESC 'Proxy Radius Configuration' MUST (cn $ ipatokenRadiusServer $ ipatokenRadiusSecret) MAY (description $ ipatokenRadiusTimeout $ ipatokenRadiusRetries $ ipatokenUserMapAttribute) X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.5 NAME 'ipatokenHOTP' SUP ipaToken STRUCTURAL DESC 'HOTP Token Type' MUST (ipatokenOTPkey $ ipatokenOTPalgorithm $ ipatokenOTPdigits $ ipatokenHOTPcounter) X-ORIGIN 'IPA OTP') -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 395 Revert Make all ipatokenTOTP attributes mandatory
Dne 21.1.2015 v 09:09 Martin Kosek napsal(a): On 01/21/2015 09:02 AM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4833. Honza Please also add the reason why we are reverting the change (see details https://bugzilla.redhat.com/show_bug.cgi?id=1176995#c7) directly to commit description. When done, I will ACK. Updated patch attached. (Feel free to amend the explanation.) -- Jan Cholasta From 625f01ec59b3304cffd7db3ae134026f3e5bc93c Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Wed, 21 Jan 2015 07:57:03 + Subject: [PATCH] Revert Make all ipatokenTOTP attributes mandatory This prevents schema replication conflicts which cause replication failures with older versions of IPA. This reverts commit adcd373931c50d91550f6b74b191d08ecce5b137. https://fedorahosted.org/freeipa/ticket/4833 --- install/share/70ipaotp.ldif | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/install/share/70ipaotp.ldif b/install/share/70ipaotp.ldif index b35ab62..0c48d9a 100644 --- a/install/share/70ipaotp.ldif +++ b/install/share/70ipaotp.ldif @@ -29,7 +29,7 @@ attributeTypes: (2.16.840.1.113730.3.8.16.1.24 NAME 'ipatokenTOTPsyncWindow' DES attributeTypes: (2.16.840.1.113730.3.8.16.1.25 NAME 'ipatokenHOTPauthWindow' DESC 'HOTP Auth Window (maximum authentication skip-ahead)' EQUALITY integerMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA OTP') attributeTypes: (2.16.840.1.113730.3.8.16.1.26 NAME 'ipatokenHOTPsyncWindow' DESC 'HOTP Sync Window (maximum synchronization skip-ahead)' EQUALITY integerMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.1 NAME 'ipaToken' SUP top ABSTRACT DESC 'Abstract token class for tokens' MUST (ipatokenUniqueID) MAY (description $ managedBy $ ipatokenOwner $ ipatokenDisabled $ ipatokenNotBefore $ ipatokenNotAfter $ ipatokenVendor $ ipatokenModel $ ipatokenSerial) X-ORIGIN 'IPA OTP') -objectClasses: (2.16.840.1.113730.3.8.16.2.2 NAME 'ipatokenTOTP' SUP ipaToken STRUCTURAL DESC 'TOTP Token Type' MUST (ipatokenOTPkey $ ipatokenOTPalgorithm $ ipatokenOTPdigits $ ipatokenTOTPclockOffset $ ipatokenTOTPtimeStep) MAY (ipatokenTOTPwatermark) X-ORIGIN 'IPA OTP') +objectClasses: (2.16.840.1.113730.3.8.16.2.2 NAME 'ipatokenTOTP' SUP ipaToken STRUCTURAL DESC 'TOTP Token Type' MAY (ipatokenOTPkey $ ipatokenOTPalgorithm $ ipatokenOTPdigits $ ipatokenTOTPclockOffset $ ipatokenTOTPtimeStep $ ipatokenTOTPwatermark) X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.3 NAME 'ipatokenRadiusProxyUser' SUP top AUXILIARY DESC 'Radius Proxy User' MAY (ipatokenRadiusConfigLink $ ipatokenRadiusUserName) X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.4 NAME 'ipatokenRadiusConfiguration' SUP top STRUCTURAL DESC 'Proxy Radius Configuration' MUST (cn $ ipatokenRadiusServer $ ipatokenRadiusSecret) MAY (description $ ipatokenRadiusTimeout $ ipatokenRadiusRetries $ ipatokenUserMapAttribute) X-ORIGIN 'IPA OTP') objectClasses: (2.16.840.1.113730.3.8.16.2.5 NAME 'ipatokenHOTP' SUP ipaToken STRUCTURAL DESC 'HOTP Token Type' MUST (ipatokenOTPkey $ ipatokenOTPalgorithm $ ipatokenOTPdigits $ ipatokenHOTPcounter) X-ORIGIN 'IPA OTP') -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 492 Add anonymous read ACI for DUA profile
Hi, Dne 20.1.2015 v 18:05 Martin Kosek napsal(a): On 01/20/2015 05:58 PM, Martin Kosek wrote: DUA profile(s) are consumed by Solaris clients. https://fedorahosted.org/freeipa/ticket/4850 I forgot to add CN to the list (I only coppied all the MAY attributes). Fix attached. Martin Works for me, ACK. Pushed to: master: 0a7a8d66040f7a5f0e55da4b01e614dd9b569a00 ipa-4-1: b54b740f7903a0722930cc281ccb5a2bece45aef Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 397 Do not crash when replica is unreachable in ipa-restore
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4857.
Honza
--
Jan Cholasta
From 6270155705249b6b6bcb4665156d73f2f14edb86 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 27 Jan 2015 07:38:06 +
Subject: [PATCH] Do not crash when replica is unreachable in ipa-restore
https://fedorahosted.org/freeipa/ticket/4857
---
ipaserver/install/ipa_restore.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 562a793..b4ef808 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -458,6 +458,7 @@ class Restore(admintool.AdminTool):
self.dirman_password)
except Exception, e:
self.log.critical(Unable to disable agreement on %s: %s % (master, e))
+continue
master_dn = DN(('cn', master), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn)
try:
@@ -482,6 +483,7 @@ class Restore(admintool.AdminTool):
self.dirman_password)
except Exception, e:
self.log.critical(Unable to disable agreement on %s: %s % (master, e))
+continue
host_entries = repl.find_ipa_replication_agreements()
hosts = [rep.single_value.get('nsds5replicahost')
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 396 Create correct log directories during full restore in ipa-restore
Dne 26.1.2015 v 17:22 Martin Kosek napsal(a): On 01/26/2015 12:12 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4865. Honza I tested the use case and log directories were properly created. So ACK, works for me. Martin Thanks. Pushed to: master: c90286cbbc1ab21e185c4d60d3a86142172c47ca ipa-4-1: 275fb2dcec64d7de48bec9faf16c4551d18c6c42 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
Dne 23.1.2015 v 15:51 Martin Basti napsal(a): On 23/01/15 08:22, Jan Cholasta wrote: Dne 20.1.2015 v 12:49 Martin Basti napsal(a): On 15/01/15 16:07, Jan Cholasta wrote: Dne 15.1.2015 v 15:39 Martin Basti napsal(a): On 15/01/15 15:07, Jan Cholasta wrote: Dne 15.1.2015 v 14:58 Martin Basti napsal(a): On 15/01/15 14:25, Jan Cholasta wrote: Hi, Dne 15.1.2015 v 13:27 Martin Basti napsal(a): On 15/01/15 13:17, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4722 Patch attached. Fast fix. Updated patch attached. 1) Forward zone commands are not fixed. FWzones are new and always normalized to absolute name in ldap Would you bet your money on that? Better be safe than sorry, especially when it's just a matter of moving the code around (right?) 2) It seems that the primary key returned by -mod, -del and -show (.result.value) is made absolute somewhere else in the code. Would it be possible to do it in one place? IMO it is not possible. Value is generated from key, and key is normalized to absolute zone before calling execute. LDAPUpdate: ... if self.obj.primary_key: pkey = keys[-1] else: pkey = None return dict(result=entry_attrs, value=pkey_to_value(pkey, options)) The idnsname attribute is just taken from LDAP without any normalization Right. 3) Attribute values returned from LDAP are never None, so the if should be if 'idnsname' in entry_attrs:. Ok I will revert the change I made. 4) If idnsname always has only single value, use entry_attrs.single_value['idnsname'] = entry_attrs.single_value['idnsname'].make_absolute() Thanks Honza Updated patch attached. Updated patch attached. Thanks. Is there a reason why you put the _make_zone_absolute calls in dnszone_* and dnsforwardzone_* instead of DNSZoneBase_*? I moved callback into Base classes. Patch attached. 1) Multi-line statements should generally use parentheses for implicit line continuation rather than backslashes: +entry_attrs.single_value['idnsname'] = ( + entry_attrs.single_value['idnsname'].make_absolute()) 2) You can remove the DN asserts from dnszone_*, they are already asserted in DNSZoneBase_*. 3) Do not just ignore return values of super().post_callback(): def post_callback(self, something, ...): something = super(...).post_callback(something, ...) ... return something Updated patch attached. Thanks, ACK. Pushed to: master: af0a2409f92e2ef8b322628c0d0569f5e0dac902 ipa-4-1: 270253a999d6a59616634b307f9428e89d1fccb9 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 396 Create correct log directories during full restore in ipa-restore
Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4865. Honza -- Jan Cholasta From 2cdb9f96c94c146805f43f38b5b93d48c95eecdb Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Mon, 26 Jan 2015 10:39:48 + Subject: [PATCH] Create correct log directories during full restore in ipa-restore https://fedorahosted.org/freeipa/ticket/4865 --- ipaserver/install/ipa_restore.py | 29 ++--- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py index 562a793..6de73e6 100644 --- a/ipaserver/install/ipa_restore.py +++ b/ipaserver/install/ipa_restore.py @@ -713,22 +713,21 @@ class Restore(admintool.AdminTool): not exist then tomcat will fail to start. The directory is different depending on whether we have a d9-based -or a d10-based installation. We can tell based on whether there is -a PKI-IPA 389-ds instance. +or a d10-based installation. -if os.path.exists(paths.ETC_SLAPD_PKI_IPA_DIR): # dogtag 9 -topdir = paths.PKI_CA_LOG_DIR -dirs = [topdir, -'/var/log/pki-ca/signedAudit,'] -else: # dogtag 10 -topdir = paths.TOMCAT_TOPLEVEL_DIR -dirs = [topdir, -paths.TOMCAT_CA_DIR, -paths.TOMCAT_CA_ARCHIVE_DIR, -paths.TOMCAT_SIGNEDAUDIT_DIR,] - -if os.path.exists(topdir): -return +dirs = [] +# dogtag 9 +if (os.path.exists(paths.VAR_LIB_PKI_CA_DIR) and +not os.path.exists(paths.PKI_CA_LOG_DIR)): +dirs += [paths.PKI_CA_LOG_DIR, + os.path.join(paths.PKI_CA_LOG_DIR, 'signedAudit')] +# dogtag 10 +if (os.path.exists(paths.VAR_LIB_PKI_TOMCAT_DIR) and +not os.path.exists(paths.TOMCAT_TOPLEVEL_DIR)): +dirs += [paths.TOMCAT_TOPLEVEL_DIR, + paths.TOMCAT_CA_DIR, + paths.TOMCAT_CA_ARCHIVE_DIR, + paths.TOMCAT_SIGNEDAUDIT_DIR] try: pent = pwd.getpwnam(PKI_USER) -- 2.1.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
Dne 23.1.2015 v 10:13 Martin Basti napsal(a): On 23/01/15 08:04, Jan Cholasta wrote: Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? These are expected during IPA restart/start etc. Other ldap exceptions should not happen, so we can get abrt reports from users, if something is wrong. Makes sense. Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) This will not work. There is relativization for some zones before this step. I will try to clean the mess I found now in a new patch. Please do. It's an ACK then. Is this supposed to go in ipa-4-1 too or is it master only? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
Dne 23.1.2015 v 10:25 Martin Basti napsal(a): On 23/01/15 10:23, Jan Cholasta wrote: Dne 23.1.2015 v 10:13 Martin Basti napsal(a): On 23/01/15 08:04, Jan Cholasta wrote: Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? These are expected during IPA restart/start etc. Other ldap exceptions should not happen, so we can get abrt reports from users, if something is wrong. Makes sense. Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) This will not work. There is relativization for some zones before this step. I will try to clean the mess I found now in a new patch. Please do. It's an ACK then. Is this supposed to go in ipa-4-1 too or is it master only? Both please, thank you. Pushed to: master: 46c12159e6c27082e7bc46e96d3738eea68dba91 ipa-4-1: 64cf3071ca908b22e5ad402585d9690c1a7fc518 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
Dne 20.1.2015 v 12:49 Martin Basti napsal(a): On 15/01/15 16:07, Jan Cholasta wrote: Dne 15.1.2015 v 15:39 Martin Basti napsal(a): On 15/01/15 15:07, Jan Cholasta wrote: Dne 15.1.2015 v 14:58 Martin Basti napsal(a): On 15/01/15 14:25, Jan Cholasta wrote: Hi, Dne 15.1.2015 v 13:27 Martin Basti napsal(a): On 15/01/15 13:17, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4722 Patch attached. Fast fix. Updated patch attached. 1) Forward zone commands are not fixed. FWzones are new and always normalized to absolute name in ldap Would you bet your money on that? Better be safe than sorry, especially when it's just a matter of moving the code around (right?) 2) It seems that the primary key returned by -mod, -del and -show (.result.value) is made absolute somewhere else in the code. Would it be possible to do it in one place? IMO it is not possible. Value is generated from key, and key is normalized to absolute zone before calling execute. LDAPUpdate: ... if self.obj.primary_key: pkey = keys[-1] else: pkey = None return dict(result=entry_attrs, value=pkey_to_value(pkey, options)) The idnsname attribute is just taken from LDAP without any normalization Right. 3) Attribute values returned from LDAP are never None, so the if should be if 'idnsname' in entry_attrs:. Ok I will revert the change I made. 4) If idnsname always has only single value, use entry_attrs.single_value['idnsname'] = entry_attrs.single_value['idnsname'].make_absolute() Thanks Honza Updated patch attached. Updated patch attached. Thanks. Is there a reason why you put the _make_zone_absolute calls in dnszone_* and dnsforwardzone_* instead of DNSZoneBase_*? I moved callback into Base classes. Patch attached. 1) Multi-line statements should generally use parentheses for implicit line continuation rather than backslashes: +entry_attrs.single_value['idnsname'] = ( +entry_attrs.single_value['idnsname'].make_absolute()) 2) You can remove the DN asserts from dnszone_*, they are already asserted in DNSZoneBase_*. 3) Do not just ignore return values of super().post_callback(): def post_callback(self, something, ...): something = super(...).post_callback(something, ...) ... return something -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 301-302] ID override sshpubkey handling
Dne 27.1.2015 v 16:22 David Kupka napsal(a): On 01/26/2015 04:33 PM, Tomas Babej wrote: Hi, the following two patches make sure that sshpubkeys work both with -mod and -add commands of ipaoverrideuser objects. Also covers the use cases with unit tests. https://fedorahosted.org/freeipa/ticket/4868 Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi, thanks for the patches but right now we need just a small fix for ipa-4-1 (attached). Thanks, ACK. Pushed to: master: 3b87302f5a280c044a8e6a8b4aa08a29e3b4b0d5 ipa-4-1: 0dc7448b3634be443806db45ffead57107213ad6 Your patches will latter go into ipa-4-2. +1 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 387 Fix validation of ipa-restore options
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4797.
Note that --data with data-only backup and --logs-only with data-only
restore are deliberately ignored and considered no-op.
Honza
--
Jan Cholasta
From 6b14a609d726f5b6dc8e94b1d3d21123637599c1 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 12 Jan 2015 12:44:21 +
Subject: [PATCH] Fix validation of ipa-restore options
Fix restore mode checks. Do some of the existing checks earlier to make them
effective. Check if --instance and --backend exist both in the filesystem and
in the backup.
Log backup type and restore mode before performing restore.
Update ipa-restore man page.
https://fedorahosted.org/freeipa/ticket/4797
---
install/tools/man/ipa-restore.1 | 6 +-
ipaserver/install/ipa_restore.py | 159 +++
2 files changed, 97 insertions(+), 68 deletions(-)
diff --git a/install/tools/man/ipa-restore.1 b/install/tools/man/ipa-restore.1
index d758490..e94d92f 100644
--- a/install/tools/man/ipa-restore.1
+++ b/install/tools/man/ipa-restore.1
@@ -65,16 +65,16 @@ Restore the data only. The default is to restore everything in the backup.
The full path to a GPG keyring. The keyring consists of two files, a public and a private key (.sec and .pub respectively). Specify the path without an extension.
.TP
\fB\-\-no\-logs\fR
-Exclude the IPA service log files in the backup (if they were backed up). Applicable only with a full backup.
+Exclude the IPA service log files in the backup (if they were backed up).
.TP
\fB\-\-online\fR
Perform the restore on\-line. Requires the \-\-data option.
.TP
\fB\-\-instance\fR=\fIINSTANCE\fR
-Restore only the databases in this 389\-ds instance. The default is to restore all found (at most this is the IPA REALM instance and the PKI\-IPA instance).
+Restore only the databases in this 389\-ds instance. The default is to restore all found (at most this is the IPA REALM instance and the PKI\-IPA instance). Requires the \-\-data option.
.TP
\fB\-\-backend\fR=\fIBACKEND\fR
-The backend to restore within an instance or instances.
+The backend to restore within an instance or instances. Requires the \-\-data option.
.TP
\fB\-\-v\fR, \fB\-\-verbose\fR
Print debugging information
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 0977039..ce7aa62 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -134,32 +134,21 @@ class Restore(admintool.AdminTool):
def validate_options(self):
+parser = self.option_parser
options = self.options
super(Restore, self).validate_options(needs_root=True)
-if options.data_only:
-installutils.check_server_configuration()
if len(self.args) 1:
-self.option_parser.error(
-must provide the backup to restore)
+parser.error(must provide the backup to restore)
elif len(self.args) 1:
-self.option_parser.error(
-must provide exactly one name for the backup)
-
-dirname = self.args[0]
-if not os.path.isabs(dirname):
-self.backup_dir = os.path.join(BACKUP_DIR, dirname)
-else:
-self.backup_dir = dirname
-
-if not os.path.isdir(dirname):
-raise self.option_parser.error(must provide path to backup directory)
+parser.error(must provide exactly one name for the backup)
+if not os.path.isdir(self.args[0]):
+parser.error(must provide path to backup directory)
if options.gpg_keyring:
if (not os.path.exists(options.gpg_keyring + '.pub') or
- not os.path.exists(options.gpg_keyring + '.sec')):
-raise admintool.ScriptError('No such key %s' %
-options.gpg_keyring)
+not os.path.exists(options.gpg_keyring + '.sec')):
+parser.error(no such key %s % options.gpg_keyring)
def ask_for_options(self):
@@ -185,38 +174,14 @@ class Restore(admintool.AdminTool):
api.bootstrap(in_server=False, context='restore')
api.finalize()
-self.log.info(Preparing restore from %s on %s,
-self.backup_dir, api.env.host)
-
-if options.instance and options.backend:
-database = (options.instance, options.backend)
-if not os.path.exists(paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE %
- database):
-raise admintool.ScriptError(
-Instance %s with backend %s does not exist % database)
-
-databases = [database]
+dirname = self.args[0]
+if not os.path.isabs(dirname):
+self.backup_dir = os.path.join(BACKUP_DIR, dirname)
else:
-if options.instance:
-instances = [options.instance]
-else:
-instances
Re: [Freeipa-devel] [PATCH] 0036 Abort full backup restoration on not matching host.
Dne 12.1.2015 v 13:08 Martin Kosek napsal(a): On 01/12/2015 12:53 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4823 Looking at this patch, are data-only backups supposed to work properly then? Wouldn't for example Directory Server fail to start when cn=config contain some hostname-bound values? Just checking... IMO the error should be raised in both data-only and full restore, if in unattended mode or the user wishes not to continue. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0036 Abort full backup restoration on not matching host.
Dne 12.1.2015 v 13:37 David Kupka napsal(a): On 01/12/2015 01:14 PM, Jan Cholasta wrote: Dne 12.1.2015 v 13:08 Martin Kosek napsal(a): On 01/12/2015 12:53 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4823 Looking at this patch, are data-only backups supposed to work properly then? Wouldn't for example Directory Server fail to start when cn=config contain some hostname-bound values? Just checking... IMO the error should be raised in both data-only and full restore, if in unattended mode or the user wishes not to continue. Description of the problem in ticket states: I tried to run ipa-restore (full kind) on replica from full backup taken on master and was expecting an error message that restore can not proceed and only data restore possible. I created the patch based on this request. Is it wrong and should ipa-restore fail every time when hostnames does not match? Yes, as Martin pointed out, the data may contain references to the hostname. Does it make sense to allow user to force the restoration in this case? Yes, if the users wish, they should be allowed to continue. Thanks for clarification. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 388 Remove RUV from LDIF files before using them in ipa-restore
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4822.
Honza
--
Jan Cholasta
From 38223d7d7df123af672a303aa989fb8259e84384 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 12 Jan 2015 15:37:33 +
Subject: [PATCH] Remove RUV from LDIF files before using them in ipa-restore
https://fedorahosted.org/freeipa/ticket/4822
---
ipaserver/install/ipa_restore.py | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 0977039..a6dbeec 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -24,6 +24,7 @@ import tempfile
import time
import pwd
from ConfigParser import SafeConfigParser
+import ldif
from ipalib import api, errors
from ipapython import version, ipautil, certdb, dogtag
@@ -94,6 +95,32 @@ def decrypt_file(tmpdir, filename, keyring):
return dest
+class RemoveRUVParser(ldif.LDIFParser):
+def __init__(self, input_file, writer, logger):
+ldif.LDIFParser.__init__(self, input_file)
+self.writer = writer
+self.log = logger
+
+def handle(self, dn, entry):
+objectclass = None
+nsuniqueid = None
+
+for name, value in entry.iteritems():
+name = name.lower()
+if name == 'objectclass':
+objectclass = [x.lower() for x in value]
+elif name == 'nsuniqueid':
+nsuniqueid = [x.lower() for x in value]
+
+if (objectclass and nsuniqueid and
+'nstombstone' in objectclass and
+'---' in nsuniqueid):
+self.log.debug(Removing RUV entry %s, dn)
+return
+
+self.writer.unparse(dn, entry)
+
+
class Restore(admintool.AdminTool):
command_name = 'ipa-restore'
log_file_name = paths.IPARESTORE_LOG
@@ -449,7 +476,14 @@ class Restore(admintool.AdminTool):
dn = DN(('cn', cn), ('cn', 'import'), ('cn', 'tasks'), ('cn', 'config'))
ldifname = '%s-%s.ldif' % (instance, backend)
-ldiffile = os.path.join(self.dir, ldifname)
+srcldiffile = os.path.join(self.dir, ldifname)
+ldiffile = '%s.noruv' % srcldiffile
+
+with open(ldiffile, 'wb') as out_file:
+ldif_writer = ldif.LDIFWriter(out_file)
+with open(srcldiffile, 'rb') as in_file:
+ldif_parser = RemoveRUVParser(in_file, ldif_writer, self.log)
+ldif_parser.parse()
if online:
conn = self.get_connection()
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0179] Fix traceback if zonemgr error message contains unicode characters
Dne 9.1.2015 v 14:07 David Kupka napsal(a): On 12/12/2014 02:26 PM, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4805 Patch attached. Does not work properly. $ locale LANG=cs_CZ.iso88592 LC_CTYPE=cs_CZ.iso88592 LC_NUMERIC=cs_CZ.iso88592 LC_TIME=cs_CZ.iso88592 LC_COLLATE=cs_CZ.iso88592 LC_MONETARY=cs_CZ.iso88592 LC_MESSAGES=cs_CZ.iso88592 LC_PAPER=cs_CZ.iso88592 LC_NAME=cs_CZ.iso88592 LC_ADDRESS=cs_CZ.iso88592 LC_TELEPHONE=cs_CZ.iso88592 LC_MEASUREMENT=cs_CZ.iso88592 LC_IDENTIFICATION=cs_CZ.iso88592 LC_ALL=cs_CZ.iso88592 $ sudo ipa-server-install --setup-dns --forwarder=10.65.201.89 -r TESTRELM.TEST -p -P -a --zonemg=Ťažk...@redhat.com -U /tmp/ipaserverinstall_invalidzonemgr.out 21 $ cat /tmp/ipaserverinstall_invalidzonemgr.out Usage: ipa-server-install [options] ipa-server-install: error: invalid zonemgr: domain name 'ŤaĹžko.redhat.com' should be normalized to: ĹĽaĹžko.redhat.com The provided value isn't right and must be normalized but the one displayed in error message doesn't match. That's because your terminal encoded it in utf-8 instead of iso-8859-2. It may be because you set the LC_* environment variables from within the terminal? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 386 Fix CA certificate renewal syslog alert
Dne 13.1.2015 v 18:46 David Kupka napsal(a): On 01/08/2015 05:04 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4820. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi! Thanks for the patch. Works for me, ACK. Thanks, pushed to: master: a63df8f3091992e227fe4654977bb91386ce0491 ipa-4-1: 818136bab14d5b137943349564a8cc3b31af5afa -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 387 Fix validation of ipa-restore options
Dne 13.1.2015 v 17:45 Jan Cholasta napsal(a):
Dne 13.1.2015 v 16:37 Petr Vobornik napsal(a):
On 01/13/2015 02:26 PM, Jan Cholasta wrote:
Dne 13.1.2015 v 13:01 Petr Vobornik napsal(a):
On 01/12/2015 02:28 PM, Jan Cholasta wrote:
Hi,
the attached patch fixes
https://fedorahosted.org/freeipa/ticket/4797.
Note that --data with data-only backup and --logs-only with data-only
restore are deliberately ignored and considered no-op.
Honza
3. When #2 fixed, data backup with --no-logs doesn't raise warning as
requested in ticket.
IMO such a warning does not make sense. You request no logs, you get no
logs, I don't see anything worth warning about here.
ok, makes sense
5. Nitpick: imho option validation should be done before temp dir
creation.
Fixed.
You've also moved
self.header = os.path.join(self.backup_dir, 'header')
below
self.read_header()
-- restore fails everytime
Silly me. Sorry. Fixed.
Rebased updated patch attached.
Rebased again, patch attached.
--
Jan Cholasta
From 3f4e54a89b8d7a1cec5576e32e74d6f71664e0b5 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 12 Jan 2015 12:44:21 +
Subject: [PATCH] Fix validation of ipa-restore options
Fix restore mode checks. Do some of the existing checks earlier to make them
effective. Check if --instance and --backend exist both in the filesystem and
in the backup.
Log backup type and restore mode before performing restore.
Update ipa-restore man page.
https://fedorahosted.org/freeipa/ticket/4797
---
install/tools/man/ipa-restore.1 | 8 +-
ipaserver/install/ipa_restore.py | 175 +++
2 files changed, 107 insertions(+), 76 deletions(-)
diff --git a/install/tools/man/ipa-restore.1 b/install/tools/man/ipa-restore.1
index d758490..5f40181 100644
--- a/install/tools/man/ipa-restore.1
+++ b/install/tools/man/ipa-restore.1
@@ -65,16 +65,16 @@ Restore the data only. The default is to restore everything in the backup.
The full path to a GPG keyring. The keyring consists of two files, a public and a private key (.sec and .pub respectively). Specify the path without an extension.
.TP
\fB\-\-no\-logs\fR
-Exclude the IPA service log files in the backup (if they were backed up). Applicable only with a full backup.
+Exclude the IPA service log files in the backup (if they were backed up).
.TP
\fB\-\-online\fR
-Perform the restore on\-line. Requires the \-\-data option.
+Perform the restore on\-line. Requires data\-only backup or the \-\-data option.
.TP
\fB\-\-instance\fR=\fIINSTANCE\fR
-Restore only the databases in this 389\-ds instance. The default is to restore all found (at most this is the IPA REALM instance and the PKI\-IPA instance).
+Restore only the databases in this 389\-ds instance. The default is to restore all found (at most this is the IPA REALM instance and the PKI\-IPA instance). Requires data\-only backup or the \-\-data option.
.TP
\fB\-\-backend\fR=\fIBACKEND\fR
-The backend to restore within an instance or instances.
+The backend to restore within an instance or instances. Requires data\-only backup or the \-\-data option.
.TP
\fB\-\-v\fR, \fB\-\-verbose\fR
Print debugging information
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index cd98d07..be48716 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -25,6 +25,7 @@ import time
import pwd
from ConfigParser import SafeConfigParser
import ldif
+import itertools
from ipalib import api, errors
from ipapython import version, ipautil, certdb, dogtag
@@ -161,32 +162,25 @@ class Restore(admintool.AdminTool):
def validate_options(self):
+parser = self.option_parser
options = self.options
super(Restore, self).validate_options(needs_root=True)
-if options.data_only:
-installutils.check_server_configuration()
if len(self.args) 1:
-self.option_parser.error(
-must provide the backup to restore)
+parser.error(must provide the backup to restore)
elif len(self.args) 1:
-self.option_parser.error(
-must provide exactly one name for the backup)
+parser.error(must provide exactly one name for the backup)
dirname = self.args[0]
if not os.path.isabs(dirname):
-self.backup_dir = os.path.join(BACKUP_DIR, dirname)
-else:
-self.backup_dir = dirname
-
+dirname = os.path.join(BACKUP_DIR, dirname)
if not os.path.isdir(dirname):
-raise self.option_parser.error(must provide path to backup directory)
+parser.error(must provide path to backup directory)
if options.gpg_keyring:
if (not os.path.exists(options.gpg_keyring + '.pub') or
- not os.path.exists(options.gpg_keyring + '.sec')):
-raise admintool.ScriptError('No such key %s
Re: [Freeipa-devel] [PATCH] 390 Do not crash on unknown services in installutils.stopped_service
Dne 13.1.2015 v 18:46 David Kupka napsal(a): On 01/13/2015 05:55 PM, Jan Cholasta wrote: Dne 13.1.2015 v 12:12 Jan Cholasta napsal(a): Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4835. Honza Modified the fix to create only one service object in stopped_service. Updated patch attached. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi! Thanks for the patch. Works for me, ACK. Thanks, pushed to: master: 5bf1c9a6f7d734c296c8eb987cfc4f7e2a345130 ipa-4-1: 065e2bbc9f2260d8c60c55f92a386513727576da -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 391-392 Make certificate renewal process synchronized
Dne 13.1.2015 v 18:47 David Kupka napsal(a): On 01/13/2015 12:17 PM, Jan Cholasta wrote: Hi, the attached patches fix https://fedorahosted.org/freeipa/ticket/4803. Note that if you want to test upgrades on CA-less, you need to apply my patch 390 as well: https://www.redhat.com/archives/freeipa-devel/2015-January/msg00103.html. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Hi! Thanks for the patch. It works as expected. I would prefer refactoring the certmonger-interacting code but since we're out time and needs this fix ASAP I'll cope with it. ACK. Thanks for the review. I would prefer that too, but I wanted to keep the changes minimal. It can be refactored in 4.2. Pushed to: master: b9ae7690489368ead9f4983d386fa210dc265dfa ipa-4-1: 760ebaa6852b12f1d58032b33ef538d9894dc3ef -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0001] ipa-client-install: attempt to get host TGT several times before aborting client installation
Dne 13.1.2015 v 09:22 Martin Kosek napsal(a): On 01/12/2015 05:45 PM, Martin Babinsky wrote: related to ticket https://fedorahosted.org/freeipa/ticket/4808 Patch attached. Martin^3 I think the --tgt-kinit-attempts approach is good one. Couple comments I have when reading the patch: 1) Function +def get_host_tgt(options, keytab, host, realm, env): should be made more general purpose and instead of whole options, it should rather accept just kinit_attemps. It will then enable future generations to reuse the function for something else. Just a generally good practice, nothing critical. 2) I think +if returncode == 0: +root_logger.info(Attempt %d succeeded. % n_attempts) +break can be just DEBUG level. People do not need to know we will try multiple attempts. 3) It may be even better to print Attempt %d/%d failed. instead of just number. But this is up to you. 4) I see several C-isms in the code, as a programming practice, let us remove them :-) In Python, the OK/notOK status is generally passed via exceptions, not return codes unless you really need them for anything meaningful. So, you can omit raiseonerr=False and have the handling code in an Except clause. When max number of attempts is breached, you then just raise the exception further (use bare raise, to re-raise to keep the original stack). +1 Additionally: 5) I would prefer if the option was named --kinit-attempts instead of --tgt-kinit-attempts (the tgt seems redundant). 6) Please do not use backslashes for line wrapping, unless it is absolutely necessary. Instead, enclose the expression in parens for implicit continuation: + help=(number of attempts to obtain host TGT + if the first one fails (defaults to %default).)) 7) Please follow PEP8 in new code: ipa-client/ipa-install/ipa-client-install:151:80: E501 line too long (93 79 characters) ipa-client/ipa-install/ipa-client-install:1100:1: E302 expected 2 blank lines, found 1 ipa-client/ipa-install/ipa-client-install:1107:29: E126 continuation line over-indented for hanging indent ipa-client/ipa-install/ipa-client-install:1107:41: E231 missing whitespace after ',' ipa-client/ipa-install/ipa-client-install:1108:29: E128 continuation line under-indented for visual indent ipa-client/ipa-install/ipa-client-install:1116:51: E225 missing whitespace around operator ipa-client/ipa-install/ipa-client-install:2453:80: E501 line too long (88 79 characters) ipa-client/ipa-install/ipa-client-install:2454:80: E501 line too long (89 79 characters) ipa-client/ipa-install/ipa-client-install:2531:80: E501 line too long (83 79 characters) ipa-client/ipa-install/ipa-client-install:2532:80: E501 line too long (81 79 characters) Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 390 Do not crash on unknown services in installutils.stopped_service
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4835.
Honza
--
Jan Cholasta
From 09155e3546adbeededbd025dea631c2a5ac39cc6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 13 Jan 2015 10:59:08 +
Subject: [PATCH] Do not crash on unknown services in
installutils.stopped_service
https://fedorahosted.org/freeipa/ticket/4835
---
ipaserver/install/installutils.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 64578f7..6955edf 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -867,19 +867,19 @@ def stopped_service(service, instance_name=):
log_instance_name)
# Figure out if the service is running, if not, yield
-if not services.knownservices[service].is_running(instance_name):
+if not services.service(service).is_running(instance_name):
root_logger.debug('Service %s%s is not running, continue.', service,
log_instance_name)
yield
else:
# Stop the service, do the required stuff and start it again
root_logger.debug('Stopping %s%s.', service, log_instance_name)
-services.knownservices[service].stop(instance_name)
+services.service(service).stop(instance_name)
try:
yield
finally:
root_logger.debug('Starting %s%s.', service, log_instance_name)
-services.knownservices[service].start(instance_name)
+services.service(service).start(instance_name)
def check_entropy():
'''
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 389 Fix ipa-restore on systems without IPA installed
Dne 13.1.2015 v 10:46 Petr Vobornik napsal(a):
On 01/12/2015 06:07 PM, Jan Cholasta wrote:
Hi,
the attached patch fixes https://fedorahosted.org/freeipa/ticket/4824.
Honza
Is there a reason why `installutils.check_server_configuration()
` is called in `cert_restore_prepare`, ie., method which is not really
connected with it, and not in `run` as for DATA backup?
Full restore may be done when IPA is not installed, but
cert_restore_prepare crashes when IPA is not installed, the check
prevents that.
Anyway, see the attached patch for an alternative, possibly better approach.
--
Jan Cholasta
From 529831fd7d48660774c6c7fa6f3a0770b45d602a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 12 Jan 2015 17:03:22 +
Subject: [PATCH] Fix ipa-restore on systems without IPA installed
https://fedorahosted.org/freeipa/ticket/4824
---
ipaserver/install/ipa_restore.py | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 0977039..5e88f2b 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -694,8 +694,12 @@ class Restore(admintool.AdminTool):
cainstance.stop_tracking_certificates(
dogtag.configured_constants())
httpinstance.HTTPInstance().stop_tracking_certificates()
-dsinstance.DsInstance().stop_tracking_certificates(
-realm_to_serverid(api.env.realm))
+try:
+dsinstance.DsInstance().stop_tracking_certificates(
+realm_to_serverid(api.env.realm))
+except OSError:
+# When IPA is not installed, DS NSS DB does not exist
+pass
for basename in ('cert8.db', 'key3.db', 'secmod.db', 'pwdfile.txt'):
filename = os.path.join(paths.IPA_NSSDB_DIR, basename)
--
2.1.0
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 391-392 Make certificate renewal process synchronized
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/4803.
Note that if you want to test upgrades on CA-less, you need to apply my
patch 390 as well:
https://www.redhat.com/archives/freeipa-devel/2015-January/msg00103.html.
Honza
--
Jan Cholasta
From 9b6f5f227996fd4b5fbc714c44a766311294a06a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 6 Jan 2015 13:08:54 +
Subject: [PATCH 1/2] Restart dogtag when its server certificate is renewed
https://fedorahosted.org/freeipa/ticket/4803
---
install/tools/ipa-upgradeconfig | 6 +++---
ipaserver/install/cainstance.py | 7 ---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index c155e95..f4a6e0d 100755
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -779,7 +779,7 @@ def certificate_renewal_update(ca):
dogtag_constants = dogtag.configured_constants()
# bump version when requests is changed
-version = 2
+version = 3
requests = (
(
dogtag_constants.ALIAS_DIR,
@@ -825,8 +825,8 @@ def certificate_renewal_update(ca):
dogtag_constants.ALIAS_DIR,
'Server-Cert cert-pki-ca',
'dogtag-ipa-renew-agent',
-None,
-None,
+'stop_pkicad',
+'renew_ca_cert',
None,
),
)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 6b4317f..951a384 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1534,16 +1534,17 @@ class CAInstance(service.Service):
done by the renewal script, renew_ca_cert once all the subsystem
certificates are renewed.
+nickname = 'Server-Cert cert-pki-ca'
pin = self.__get_ca_pin()
try:
certmonger.dogtag_start_tracking(
ca='dogtag-ipa-renew-agent',
-nickname='Server-Cert cert-pki-ca',
+nickname=nickname,
pin=pin,
pinfile=None,
secdir=self.dogtag_constants.ALIAS_DIR,
-pre_command=None,
-post_command=None)
+pre_command='stop_pkicad',
+post_command='renew_ca_cert %s' % nickname)
except RuntimeError, e:
root_logger.error(
certmonger failed to start tracking certificate: %s % e)
--
2.1.0
From 2423f45bacf43c789f6eb3f392b15fbd1d5dd2c9 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 8 Jan 2015 09:06:46 +
Subject: [PATCH 2/2] Make certificate renewal process synchronized
Synchronization is achieved using a global renewal lock.
https://fedorahosted.org/freeipa/ticket/4803
---
freeipa.spec.in| 1 +
install/certmonger/Makefile.am | 1 +
.../certmonger/dogtag-ipa-ca-renew-agent-submit| 4 +-
install/certmonger/ipa-server-guard| 55 +++
install/restart_scripts/renew_ca_cert | 11 ++-
install/restart_scripts/renew_ra_cert | 11 ++-
install/restart_scripts/restart_dirsrv | 10 +-
install/restart_scripts/restart_httpd | 10 +-
install/restart_scripts/stop_pkicad| 4 +
install/tools/ipa-upgradeconfig| 3 +
ipaplatform/base/paths.py | 2 +
ipaserver/install/cainstance.py| 38
ipaserver/install/certs.py | 104 +
ipaserver/install/httpinstance.py | 42 +
14 files changed, 290 insertions(+), 6 deletions(-)
create mode 100755 install/certmonger/ipa-server-guard
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 40bad04..3175512 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -660,6 +660,7 @@ fi
%{_sbindir}/ipa-advise
%{_sbindir}/ipa-cacert-manage
%{_libexecdir}/certmonger/dogtag-ipa-ca-renew-agent-submit
+%{_libexecdir}/certmonger/ipa-server-guard
%{_libexecdir}/ipa-otpd
%dir %{_libexecdir}/ipa
%{_libexecdir}/ipa/ipa-dnskeysyncd
diff --git a/install/certmonger/Makefile.am b/install/certmonger/Makefile.am
index ef6a0a6..2dc476f 100644
--- a/install/certmonger/Makefile.am
+++ b/install/certmonger/Makefile.am
@@ -3,6 +3,7 @@ NULL =
appdir = $(libexecdir)/certmonger/
app_SCRIPTS = \
dogtag-ipa-ca-renew-agent-submit \
+ ipa-server-guard \
$(NULL)
EXTRA_DIST =\
diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index c63c0c2..0bebb49 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -38,7 +38,7 @@ from ipapython.dn import DN
from ipalib import api, errors, pkcs10, x509
from ipaplatform.paths import paths
from
Re: [Freeipa-devel] [PATCH] 387 Fix validation of ipa-restore options
Dne 13.1.2015 v 13:01 Petr Vobornik napsal(a): On 01/12/2015 02:28 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4797. Note that --data with data-only backup and --logs-only with data-only restore are deliberately ignored and considered no-op. Honza 1. I'm not sure how relative path to backup dir should work: I have a backup: /var/lib/ipa/backup/ipa-data-2015-01-13-10-53-06/ Absolute path works great. But: - ipa-data-2015-01-13-10-53-06 fails with `ipa-restore: error: must provide path to backup directory` Ugly hybrid (I was in home dir): ../../var/lib/ipa/backup/ipa-data-2015-01-13-10-53-06/ fails with: /var/lib/ipa/backup/../../var/lib/ipa/backup/ipa-data-2015-01-13-10-53-06/header I.e., dir name won't pass if not os.path.isdir(self.args[0]): and the second concatenation is weird. man ipa-restore says: Only the name of the backup needs to be passed in, not the full path. Backups are stored in a subdirectory in /var/lib/ipa/backup. If a backup is in another location then the full path must be provided. This was validated wrong, fixed. 2. Data backup cannot be done because the first 'for' never breaks +for instance in instances: +for backend in backends: +db_dir = (paths.SLAPD_INSTANCE_DB_DIR_TEMPLATE % + (instance, backend)) +if os.path.exists(db_dir): +break +else: +raise admintool.ScriptError( +Cannot restore a data backup into an empty system) Oops, fixed. 3. When #2 fixed, data backup with --no-logs doesn't raise warning as requested in ticket. IMO such a warning does not make sense. You request no logs, you get no logs, I don't see anything worth warning about here. 4. Since backup type is detected automatically(--data doesn't have to be specified for data restore), I guess that the ticket requirement: `Here expecting an error message that --online option can only provided along with --data option. ` is invalid, right? Man page states that --online requires --data option, which is not true. Fixed in man page. 5. Nitpick: imho option validation should be done before temp dir creation. Fixed. 6. pep8, fixing them is up to you: ./ipaserver/install/ipa_restore.py:150:5: E129 visually indented line with same indent as next logical line ./ipaserver/install/ipa_restore.py:184:13: E128 continuation line under-indented for visual indent Fixed. Updated patch attached. -- Jan Cholasta From 44801a195e293b790fc7ec08bff04e8d0cf29787 Mon Sep 17 00:00:00 2001 From: Jan Cholasta jchol...@redhat.com Date: Mon, 12 Jan 2015 12:44:21 + Subject: [PATCH] Fix validation of ipa-restore options Fix restore mode checks. Do some of the existing checks earlier to make them effective. Check if --instance and --backend exist both in the filesystem and in the backup. Log backup type and restore mode before performing restore. Update ipa-restore man page. https://fedorahosted.org/freeipa/ticket/4797 --- install/tools/man/ipa-restore.1 | 8 +- ipaserver/install/ipa_restore.py | 176 +++ 2 files changed, 108 insertions(+), 76 deletions(-) diff --git a/install/tools/man/ipa-restore.1 b/install/tools/man/ipa-restore.1 index d758490..5f40181 100644 --- a/install/tools/man/ipa-restore.1 +++ b/install/tools/man/ipa-restore.1 @@ -65,16 +65,16 @@ Restore the data only. The default is to restore everything in the backup. The full path to a GPG keyring. The keyring consists of two files, a public and a private key (.sec and .pub respectively). Specify the path without an extension. .TP \fB\-\-no\-logs\fR -Exclude the IPA service log files in the backup (if they were backed up). Applicable only with a full backup. +Exclude the IPA service log files in the backup (if they were backed up). .TP \fB\-\-online\fR -Perform the restore on\-line. Requires the \-\-data option. +Perform the restore on\-line. Requires data\-only backup or the \-\-data option. .TP \fB\-\-instance\fR=\fIINSTANCE\fR -Restore only the databases in this 389\-ds instance. The default is to restore all found (at most this is the IPA REALM instance and the PKI\-IPA instance). +Restore only the databases in this 389\-ds instance. The default is to restore all found (at most this is the IPA REALM instance and the PKI\-IPA instance). Requires data\-only backup or the \-\-data option. .TP \fB\-\-backend\fR=\fIBACKEND\fR -The backend to restore within an instance or instances. +The backend to restore within an instance or instances. Requires data\-only backup or the \-\-data option. .TP \fB\-\-v\fR, \fB\-\-verbose\fR Print debugging information diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py index 0977039..dbebd83 100644 --- a/ipaserver/install/ipa_restore.py +++ b/ipaserver
Re: [Freeipa-devel] [PATCH] 0036 Abort full backup restoration on not matching host.
Dne 13.1.2015 v 15:54 David Kupka napsal(a): On 01/13/2015 03:07 PM, David Kupka wrote: On 01/13/2015 02:57 PM, Jan Cholasta wrote: Dne 13.1.2015 v 14:44 David Kupka napsal(a): On 01/12/2015 04:50 PM, Rob Crittenden wrote: Jan Cholasta wrote: Dne 12.1.2015 v 16:30 Rob Crittenden napsal(a): Jan Cholasta wrote: Dne 12.1.2015 v 13:37 David Kupka napsal(a): On 01/12/2015 01:14 PM, Jan Cholasta wrote: Dne 12.1.2015 v 13:08 Martin Kosek napsal(a): On 01/12/2015 12:53 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/4823 Looking at this patch, are data-only backups supposed to work properly then? Wouldn't for example Directory Server fail to start when cn=config contain some hostname-bound values? Just checking... IMO the error should be raised in both data-only and full restore, if in unattended mode or the user wishes not to continue. Description of the problem in ticket states: I tried to run ipa-restore (full kind) on replica from full backup taken on master and was expecting an error message that restore can not proceed and only data restore possible. I created the patch based on this request. Is it wrong and should ipa-restore fail every time when hostnames does not match? Yes, as Martin pointed out, the data may contain references to the hostname. Does it make sense to allow user to force the restoration in this case? Yes, if the users wish, they should be allowed to continue. IIRC a data restore is just the data from the replicated tree so there is nothing hostname-specific. It is probably worth investigating so we don't go too far one way or the other. There's at least cn=fqdn,cn=masters,cn=etc,suffix. That's part of the replicated tree, but it does raise a question: What would it mean if you did a data restore to a server that doesn't exist as a master in the realm? Geez, I don't know, but it likely wouldn't go well. Checking for that would be quite an issue and it would surely exercise the python-ldap ldif module. Is it illegal though? I don't know. Any keytabs would be bad b/c the Kerberos master key is different. In all likelihood things would just go south. I imagine someone might try this in an attempt to setup a test/integration environment. It just wouldn't work. In a replicated environment though, with hosts A and B, restoring the data from B on A is probably not a big deal, though it does raise the question of why the heck would you do this? It could be that you only did backups on B and don't want to do a full re-init on A due to size/time/moon phase. A full restore definitely shouldn't be done on the wrong host as it will restore certificates and keytabs that are definitely host-specific. Should the continue prompt be removed then? Well, you've just about got me convinced we shouldn't allow it, at least not without several do you really want to do this? prompts. This seems to fall in the range of yeah, it will work if you know what you're doing, but why would you ever want to? I think until that question is answered it is safer to disallow it. I'd be ok with a ticket into the deferred to investigate this later to see if it can be relaxed. rob Ok, changed to remove the prompt and raise error. We can bring it back once some user comes with convincing reason. The error doesn't need to be logged, raising a ScriptError is perfectly sufficient (please use the message that includes both of the hostnames). Updated patch attached. Fixed indentation. Thanks, ACK. Pushed to: master: b6c58ff238eb335dcb2a80fc98ecfe8bce5e2422 ipa-4-1: 640a4b30c2475d7b62cc2407af358a8951c34121 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 389 Fix ipa-restore on systems without IPA installed
Dne 13.1.2015 v 16:47 Petr Vobornik napsal(a): On 01/13/2015 11:54 AM, Jan Cholasta wrote: Dne 13.1.2015 v 10:46 Petr Vobornik napsal(a): On 01/12/2015 06:07 PM, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/4824. Honza Is there a reason why `installutils.check_server_configuration() ` is called in `cert_restore_prepare`, ie., method which is not really connected with it, and not in `run` as for DATA backup? Full restore may be done when IPA is not installed, but cert_restore_prepare crashes when IPA is not installed, the check prevents that. Anyway, see the attached patch for an alternative, possibly better approach. Works, but with full restore I got: ... Disabling all replication. Unable to get connection, skipping disabling agreements: Unable to bind to LDAP server: [Errno 2] No such file or directory Stopping IPA services Restoring files ... I wonder if it needs a better error message, it may be confusing for users. Can you open a ticket for this? I would rather not deal with this right now in this patch... Btw what is the use case for fullrestore without IPA? Is it somewhere documented or mentioned? http://www.freeipa.org/page/V3/Backup_and_Restore#Catastrophic_hardware_failure_on_a_machine. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
