PR#50890: Ticket 50889 - Extract pem files into a private namespace - 389-ds-base

389-ds-base

#50890 Ticket 50889 - Extract pem files into a private namespace

Closed 3 years ago by spichugi. Opened 4 years ago by tbordaz.

tbordaz/389-ds-base Ticket-50889 into master

Ticket 50889 - Extract pem files into a private namespace

Thierry Bordaz • 4 years ago

60ae321

ldap/admin/src/scripts/dscreate.map.in

file modified

		`@@ -37,3 +37,4 @@`
		`config_dir = config_dir`
		`db_dir = db_dir`
		`run_dir = run_dir`
		`+ instance_name = ServerIdentifier`

ldap/servers/slapd/back-ldbm/misc.c

file modified

-65

		`@@ -318,71 +318,6 @@`
		`return rval;`
		`}`

		`- char`
		`- get_sep(char *path)`
		`- {`
		`- if (NULL == path)`
		`- return '/'; /* default */`
		`- if (NULL != strchr(path, '/'))`
		`- return '/';`
		`- if (NULL != strchr(path, '\\'))`
		`- return '\\';`
		`- return '/'; /* default */`
		`- }`
		`-`
		`- /* mkdir -p */`
		`- int`
		`- mkdir_p(char *dir, unsigned int mode)`
		`- {`
		`- PRFileInfo64 info;`
		`- int rval;`
		`- char sep = get_sep(dir);`
		`-`
		`- rval = PR_GetFileInfo64(dir, &info);`
		`- if (PR_SUCCESS == rval) {`
		`- if (PR_FILE_DIRECTORY != info.type) /* not a directory */`
		`- {`
		`- PR_Delete(dir);`
		`- if (PR_SUCCESS != PR_MkDir(dir, mode)) {`
		`- slapi_log_err(SLAPI_LOG_ERR, "mkdir_p", "%s: error %d (%s)\n",`
		`- dir, PR_GetError(), slapd_pr_strerror(PR_GetError()));`
		`- return -1;`
		`- }`
		`- }`
		`- return 0;`
		`- } else {`
		`- /* does not exist */`
		`- char p, e;`
		`- char c[2] = {0, 0};`
		`- int len = strlen(dir);`
		`- rval = 0;`
		`-`
		`- e = dir + len - 1;`
		`- if (*e == sep) {`
		`- c[1] = *e;`
		`- *e = '\0';`
		`- }`
		`-`
		`- c[0] = '/';`
		`- p = strrchr(dir, sep);`
		`- if (NULL != p) {`
		`- *p = '\0';`
		`- rval = mkdir_p(dir, mode);`
		`- *p = c[0];`
		`- }`
		`- if (c[1])`
		`- *e = c[1];`
		`- if (0 != rval)`
		`- return rval;`
		`- if (PR_SUCCESS != PR_MkDir(dir, mode)) {`
		`- slapi_log_err(SLAPI_LOG_ERR, "mkdir_p", "%s: error %d (%s)\n",`
		`- dir, PR_GetError(), slapd_pr_strerror(PR_GetError()));`
		`- return -1;`
		`- }`
		`- return 0;`
		`- }`
		`- }`
		`-`
		`/* This routine checks to see if there is a callback registered for retrieving`
		`* RUV updates to add to the datastore transaction. If so, it allocates a`
		`* modify_context for consumption by the caller. */`

ldap/servers/slapd/ldaputil.c

file modified

+11 -1

		`@@ -508,12 +508,22 @@`
		`static int`
		`setup_ol_tls_conn(LDAP *ld, int clientauth)`
		`{`
		`- char *certdir = config_get_certdir();`
		`+ char *certdir;`
		`int optval = 0;`
		`int ssl_strength = 0;`
		`int rc = 0;`
		`const char *cacert = NULL;`

		`+ /* certdir is used to setup outgoing secure connection (openldap)`
		`+ * It refers to the place where PEM files have been extracted`
		`+ *`
		`+ * If a private tmp namespace exists`
		`+ * it is the place where PEM files have been extracted`
		`+ */`
		`+ if ((certdir = check_private_certdir()) == NULL) {`
		`+ certdir = config_get_certdir();`
		`+ }`
		`+`
		`if (config_get_ssl_check_hostname()) {`
		`ssl_strength = LDAP_OPT_X_TLS_HARD;`
		`} else {`

ldap/servers/slapd/slapi-private.h

file modified

		`@@ -1223,6 +1223,8 @@`
		`void strcpy_unescape_value(char d, const char s);`
		`void get_internal_conn_op (uint64_t connid, int32_t op_id, int32_t op_internal_id, int32_t op_nested_count);`
		`char slapi_berval_get_string_copy(const struct berval bval);`
		`+ char get_sep(char *path);`
		`+ int mkdir_p(char *dir, unsigned int mode);`

		`/* lenstr stuff */`

ldap/servers/slapd/ssl.c

file modified

+130 -5

		`@@ -913,6 +913,85 @@`

		`#define SSLVGreater(x, y) (((x) > (y)) ? (x) : (y))`

		`+ /* This routine returns the absolute path where to extract the pem files`
		`+ * (certificate/key) in case nsslapd-private-certdir is private namespace.`
		`+ * If absolute patch does not exist, it creates it with right 0777`
		`+ *`
		`+ * If the configuration parameter is not defined it returns NULL`
		`+ *`
		`+ * If the configuration parameter (nsslapd-private-certdir) is defined and`
		`+ * valid (under /tmp namespace) it returns it.`
		`+ *`
		`+ * If /tmp is not a private namespace or configaration parameter is invalid`
		`+ * it returns NULL`
		`+ *`
		`+ * If returned value is not NULL, caller must free it (slapi_ch_free_string)`
		`+ */`
		`+ char *`
		`+ check_private_certdir()`
		`+ {`
		`+ FILE *f;`
		`+ char sline[1024];`
		`+ const char *private_namespace_root = "/systemd-private";`
		`+ const char private_mountpoint = "/tmp"; / Using systemd PrivateTmp=Yes, "/tmp" is expected to be private */`
		`+ int mountid, parentid, major, minor;`
		`+ char root[256] = {0}; /* path which forms the root of the mount */`
		`+ char mountpoint[256] = {0}; /* path of the mountpoint relative to process root directory */`
		`+ char rest[256] = {0};`
		`+ PRBool tmp_private = PR_FALSE;`
		`+ char *path_certdir;`
		`+ char *certdir;`
		`+ char *bname = NULL;`
		`+`
		`+ /* Check if /tmp is a private namespace */`
		`+ f = fopen("/proc/self/mountinfo", "r");`
		`+ if (f == NULL) {`
		`+ return NULL;`
		`+ }`
		`+ while (fgets(sline, sizeof(sline), f)) {`
		`+ sscanf(sline,"%d %d %d:%d %s %s %s\n",`
		`+ &mountid, &parentid, &major, &minor, &root, &mountpoint, &rest);`
		`+ if ((strncmp(mountpoint, private_mountpoint, strlen(private_mountpoint)) == 0) && /* mountpoint=/tmp */`
		`+ strstr(root, private_namespace_root)) { /* root=...systemd-private... */`
		`+ tmp_private = PR_TRUE;`
		`+ break;`
		`+ }`
		`+ }`
		`+ fclose(f);`
		`+`
		`+ if (!tmp_private) {`
		`+ /* tmp is not a private name space */`
		`+ slapi_log_err(SLAPI_LOG_WARNING, "Security Initialization",`
		`+ "%s is not a private namespace. pem files not exported there\n",`
		`+ private_mountpoint);`
		`+ return NULL;`
		`+ }`
		`+`
		`+ /* Create the subdirectory under the private tmp namespace`
		`+ * e.g.: /tmp/slapd-standalone1`
		`+ */`
		`+ certdir = config_get_certdir();`
		`+ if (certdir == NULL) {`
		`+ /* config does not define a certdir path, extract pem`
		`+ * under private_mountpoint`
		`+ */`
		`+ bname = "";`
		`+ } else {`
		`+ bname = basename(certdir);`
		`+ }`
		`+ path_certdir = slapi_ch_smprintf("%s/%s", private_mountpoint, bname);`
		`+ slapi_ch_free_string(&certdir);`
		`+`
		`+ if (mkdir_p(path_certdir, 0777)) {`
		`+ slapi_log_err(SLAPI_LOG_WARNING, "Security Initialization",`
		`+ "check_private_certdir - Fail to create %s\n",`
		`+ path_certdir);`
		`+ slapi_ch_free_string(&path_certdir);`
		`+ return NULL;`
		`+ }`
		`+ return path_certdir;`
		`+ }`
		`+`
		`/*`
		`* slapd_nss_init() is always called from main(), even if we do not`
		`* plan to listen on a secure port. If config_available is 0, the`
		`@@ -945,7 +1024,9 @@`
		`emin, emax);`

		`/* set in slapd_bootstrap_config,`
		`- thus certdir is available even if config_available is false */`
		`+ thus certdir is available even if config_available is false`
		`+ certdir is the path where the NSS database is located`
		`+ */`
		`certdir = config_get_certdir();`

		`/* make sure path does not end in the path separator character */`
		`@@ -2105,9 +2186,15 @@`
		`errorCode, slapd_pr_strerror(errorCode));`
		`} else {`
		`if (slapi_client_uses_non_nss(ld) && config_get_extract_pem()) {`
		`- char *certdir = config_get_certdir();`
		`+ char *certdir;`
		`char *keyfile = NULL;`
		`char *certfile = NULL;`
		`+ /* If a private tmp namespace exists`
		`+ * it is the place where PEM files will be extracted`
		`+ */`
		`+ if ((certdir = check_private_certdir()) == NULL) {`
		`+ certdir = config_get_certdir();`
		`+ }`
		`if (KeyExtractFile) {`
		`if ('/' == *KeyExtractFile) {`
		`keyfile = KeyExtractFile;`
		`@@ -2402,6 +2489,13 @@`
		`return rv;`
		`}`

		`+ /* This function is called to get the path of PEM file where`
		`+ * to extract key/cert (called by slapd_extract_key/cert).`
		`+ * If filename is absolute path, it is the responsibility of the admin`
		`+ * to set it according CONFIG_CERTDIR_ATTRIBUTE`
		`+ * Else if it exists a private tmp namespace it locates the PEM file under it`
		`+ * Else it locates the PEM file under CONFIG_CERTDIR_ATTRIBUTE`
		`+ */`
		`static char *`
		`gen_pem_path(char *filename)`
		`{`
		`@@ -2410,11 +2504,17 @@`
		`char *dname = NULL;`
		`char *bname = NULL;`
		`char *certdir = NULL;`
		`+ char *default_certdir = config_get_certdir();`

		`if (!filename) {`
		`goto bail;`
		`}`
		`- certdir = config_get_certdir();`
		`+ /* If a private tmp namespace exists`
		`+ * it is the place where PEM file will be extracted`
		`+ */`
		`+ if ((certdir = check_private_certdir()) == NULL) {`
		`+ certdir = config_get_certdir();`
		`+ }`
		`pem = PL_strstr(filename, PEMEXT);`
		`if (pem) {`
		`*pem = '\0';`
		`@@ -2424,6 +2524,11 @@`
		`if (!PL_strcmp(dname, ".")) {`
		`/* just a file name */`
		`pempath = slapi_ch_smprintf("%s/%s%s", certdir, bname, PEMEXT);`
		`+ } else if (!PL_strcmp(dname, default_certdir)) {`
		`+ /* This is absolute path to certdir (e.g. /etc/dirsrv/slapd-standalone1)`
		`+ * PEM file will be in private namespace or certdir`
		`+ */`
		`+ pempath = slapi_ch_smprintf("%s/%s%s", certdir, bname, PEMEXT);`
		`} else if (*dname == '/') {`
		`/* full path */`
		`pempath = slapi_ch_smprintf("%s/%s%s", dname, bname, PEMEXT);`
		`@@ -2432,6 +2537,7 @@`
		`pempath = slapi_ch_smprintf("%s/%s/%s%s", certdir, dname, bname, PEMEXT);`
		`}`
		`bail:`
		`+ slapi_ch_free_string(&default_certdir);`
		`slapi_ch_free_string(&certdir);`
		`return pempath;`
		`}`
		`@@ -2462,13 +2568,13 @@`
		`CertExtractFile = slapi_entry_attr_get_charptr(entry, "ServerCertExtractFile");`
		`personality = slapi_entry_attr_get_charptr(entry, "nsSSLPersonalitySSL");`
		`}`
		`+`
		`certfile = gen_pem_path(CertExtractFile);`
		`if (isCA) {`
		`slapi_ch_free_string(&CACertPemFile);`
		`CACertPemFile = certfile;`
		`}`

		`- certdir = config_get_certdir();`
		`certHandle = CERT_GetDefaultCertDB();`
		`for (node = CERT_LIST_HEAD(list); !CERT_LIST_END(node, list);`
		`node = CERT_LIST_NEXT(node)) {`
		`@@ -2483,8 +2589,15 @@`
		`slapi_log_err(SLAPI_LOG_INFO, "slapd_extract_cert", "CA CERT NAME: %s\n", cert->nickname);`
		`if (!certfile) {`
		`char buf[BUFSIZ];`
		`+ /* If a private tmp namespace exists`
		`+ * it is the place where PEM file will be extracted`
		`+ */`
		`+ if ((certdir = check_private_certdir()) == NULL) {`
		`+ certdir = config_get_certdir();`
		`+ }`
		`certfile = slapi_ch_smprintf("%s/%s%s", certdir,`
		`escape_string_for_filename(cert->nickname, buf), PEMEXT);`
		`+ slapi_ch_free_string(&certdir);`
		`entrySetValue(slapi_entry_get_sdn(entry), "CACertExtractFile", certfile);`
		`slapi_set_cacertfile(certfile);`
		`}`
		`@@ -2510,8 +2623,15 @@`
		`slapi_log_err(SLAPI_LOG_INFO, "slapd_extract_cert", "SERVER CERT NAME: %s\n", cert->nickname);`
		`if (!certfile) {`
		`char buf[BUFSIZ];`
		`+ /* If a private tmp namespace exists`
		`+ * it is the place where PEM file will be extracted`
		`+ */`
		`+ if ((certdir = check_private_certdir()) == NULL) {`
		`+ certdir = config_get_certdir();`
		`+ }`
		`certfile = slapi_ch_smprintf("%s/%s%s", certdir,`
		`escape_string_for_filename(cert->nickname, buf), PEMEXT);`
		`+ slapi_ch_free_string(&certdir);`
		`}`
		`if (!outFile) {`
		`outFile = PR_Open(certfile, PR_CREATE_FILE \| PR_RDWR \| PR_TRUNCATE, 00660);`
		`@@ -2777,10 +2897,15 @@`
		`"nsSSLPersonalitySSL value not found.\n");`
		`goto bail;`
		`}`
		`- certdir = config_get_certdir();`
		`keyfile = gen_pem_path(KeyExtractFile);`
		`if (!keyfile) {`
		`char buf[BUFSIZ];`
		`+ /* If a private tmp namespace exists`
		`+ * it is the place where PEM file will be extracted`
		`+ */`
		`+ if ((certdir = check_private_certdir()) == NULL) {`
		`+ certdir = config_get_certdir();`
		`+ }`
		`keyfile = slapi_ch_smprintf("%s/%s-Key%s", certdir,`
		`escape_string_for_filename(personality, buf), PEMEXT);`
		`}`

ldap/servers/slapd/util.c

file modified

+65

		`@@ -1571,3 +1571,68 @@`
		`slapi_attr_first_value(attr, &val);`
		`return slapi_value_get_string(val);`
		`}`
		`+`
		`+ char`
		`+ get_sep(char *path)`
		`+ {`
		`+ if (NULL == path)`
		`+ return '/'; /* default */`
		`+ if (NULL != strchr(path, '/'))`
		`+ return '/';`
		`+ if (NULL != strchr(path, '\\'))`
		`+ return '\\';`
		`+ return '/'; /* default */`
		`+ }`
		`+`
		`+ /* mkdir -p */`
		`+ int`
		`+ mkdir_p(char *dir, unsigned int mode)`
		`+ {`
		`+ PRFileInfo64 info;`
		`+ int rval;`
		`+ char sep = get_sep(dir);`
		`+`
		`+ rval = PR_GetFileInfo64(dir, &info);`
		`+ if (PR_SUCCESS == rval) {`
		`+ if (PR_FILE_DIRECTORY != info.type) /* not a directory */`
		`+ {`
		`+ PR_Delete(dir);`
		`+ if (PR_SUCCESS != PR_MkDir(dir, mode)) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "mkdir_p", "%s: error %d (%s)\n",`
		`+ dir, PR_GetError(), slapd_pr_strerror(PR_GetError()));`
		`+ return -1;`
		`+ }`
		`+ }`
		`+ return 0;`
		`+ } else {`
		`+ /* does not exist */`
		`+ char p, e;`
		`+ char c[2] = {0, 0};`
		`+ int len = strlen(dir);`
		`+ rval = 0;`
		`+`
		`+ e = dir + len - 1;`
		`+ if (*e == sep) {`
		`+ c[1] = *e;`
		`+ *e = '\0';`
		`+ }`
		`+`
		`+ c[0] = '/';`
		`+ p = strrchr(dir, sep);`
		`+ if (NULL != p) {`
		`+ *p = '\0';`
		`+ rval = mkdir_p(dir, mode);`
		`+ *p = c[0];`
		`+ }`
		`+ if (c[1])`
		`+ *e = c[1];`
		`+ if (0 != rval)`
		`+ return rval;`
		`+ if (PR_SUCCESS != PR_MkDir(dir, mode)) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "mkdir_p", "%s: error %d (%s)\n",`
		`+ dir, PR_GetError(), slapd_pr_strerror(PR_GetError()));`
		`+ return -1;`
		`+ }`
		`+ return 0;`
		`+ }`
		`+ }`
		`\ No newline at end of file`

src/lib389/lib389/instance/setup.py

file modified

		`@@ -728,6 +728,7 @@`
		`ds_port=slapd['port'],`
		`ds_user=slapd['user'],`
		`rootdn=slapd['root_dn'],`
		`+ instance_name=slapd['instance_name'],`
		`ds_passwd=self._secure_password, # We set our own password here, so we can connect and mod.`
		`# This is because we never know the users input root password as they can validily give`
		`# us a hashed input.`

wrappers/systemd.template.service.in

file modified

		`@@ -15,6 +15,7 @@`
		`PIDFile=/run/@package_name@/slapd-%i.pid`
		`ExecStartPre=@libexecdir@/ds_systemd_ask_password_acl @instconfigdir@/slapd-%i/dse.ldif`
		`ExecStart=@sbindir@/ns-slapd -D @instconfigdir@/slapd-%i -i /run/@package_name@/slapd-%i.pid`
		`+ PrivateTmp=on`

		`[Install]`
		`WantedBy=multi-user.target`

tbordaz commented 4 years ago

Bug Description:
since 1.3.5, certificates and keys are, by default, extracted under
nsslapd-certdir directory. They are exracted in pem files.

Some pem files (i.e. Serv-Cert-Key.pem) contain sensitive.
The ticket is to extract them into a private namespace specific
to the DS process.

Fix Description:
If the process is started with systemd, it uses the PrivateTmp=on
directive to create a private namespace.
Then if such private namespace exists and nsslapd-private-certdir is
defined withing the private namespace, DS extracts the certificates/keys
under nsslapd-private-certdir

https://pagure.io/389-ds-base/issue/50889

Reviewed by: ?

Platforms tested: F30

Doc impact: yes

Metadata Update from @tbordaz:
- Pull-request tagged with: WIP

4 years ago

rebased onto 0d79265002c9072cf755e89589912df1e0d938a4

4 years ago

rebased onto 379a43f7de55092937a42b509fe76ee9c43f1a7d

4 years ago

rebased onto a2e1cface7b1bf9bb4d5e8fa84de5a4ca2c1de83

4 years ago

tbordaz commented 4 years ago

All tests completed, I am removing WIP flag to get feedbacks
Things I am not comfortable with are the safety of private namespace and the way it is tested

Metadata Update from @tbordaz:
- Pull-request untagged with: WIP

4 years ago

firstyear commented 4 years ago

Can I review this tomorrow? Sorry about the delay :(

rebased onto 3a248de3997232421a135c9ddfba34b14752aec1

4 years ago

firstyear commented on line 5 of ldap/ldif/template-dse.ldif.in 4 years ago

What about existing servers on upgrade? I think we sohuldn't have this in the template, but we should make this a cn=config migration.

firstyear commented 4 years ago

I don't know if this needs a cn=config attribute, I think your detection of the system private namespace is enough - if it exists we use that, if not we dont? I think the cn=config doesn't really serve a purpose here in that case .... is there some use case you had in mind?

firstyear commented 4 years ago

Besides these comments the code looks good, I like the systemd private namespace detection :)

rebased onto 8d978b8860d8d31fc11ab82eb7e6fe9c6f6c6420

4 years ago

rebased onto 73611bd21a339a0993d6939c3836097d5959a84d

4 years ago

rebased onto 60ae321

4 years ago

tbordaz commented 4 years ago

@firstyear after discussion with @lkrispen he convinced me that you were right :)

It is useless to introduce a new config parameter: Location of PEM file is internal thing and no reason to specify a path, If admin do need PEM it is always possible to extract key/cert (certutil), current location comes from historical reason and was not specified, adding a config param brings complexity without real added value.
Please have a look at the revisited patch.

firstyear commented 4 years ago

looks really good :) ack from me :)

Pull-Request has been merged by tbordaz

4 years ago