Issue 6349 - RFE - Use previously extracted key path #6363

Firstyear · 2024-10-16T07:39:25Z

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a
container.

Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration.

fixes: #6340

Author: William Brown william@blackhats.net.au

Review by: ???

Firstyear · 2024-10-16T07:40:19Z

Tested with suites/replication/tls_client_auth_repl_test.py and suites/basic

progier389 · 2024-10-16T16:12:18Z

Looks good except a minor point:
I wonder if we should not have some cleanup code to free key_extract_file and cert_extract_file called during shutdown (to avoid polluting libasan reports 😉)

tbordaz

The change looks good except a doubt of a small memory leak.
If the concern was about the verbose warning in check_private_certdir you can also log it once, making the change smaller.
So your fix is to avoid those annoying logs and cleanup the code. Correct ?

ldap/servers/slapd/ssl.c

Firstyear · 2024-10-17T00:31:41Z

Good catch @tbordaz I have fixed both.

Firstyear · 2024-10-17T00:32:03Z

Looks good except a minor point: I wonder if we should not have some cleanup code to free key_extract_file and cert_extract_file called during shutdown (to avoid polluting libasan reports 😉)

We don't seem to have a shutdown call that I can use (unless I'm silly and couldn't see it).

EDIT: We don't have one, so I added one.

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: 389ds#6349 Author: William Brown <william@blackhats.net.au> Review by: ???

droideck

LGTM!

progier389

LGTM

tbordaz

LGTM as well

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

@tbordaz

Bug Description: slapd_SSL_client_auth uses the values of KeyExtractFile and CertExtractFile from the configuration entry, and each time it's called attempts to determine if this is an absolute or relative path. If the path is relative, it prepends a /tmp location based on if the running system /tmp is a private namespace or not. This causes a replication client that uses TLS certificate authentication to repeatedly emit warnings that the key extraction occurred to non-private tmp in a container. Fix Description: During key extraction, we extract keys and files using the "last token" from nss as the item that we extract. Because of this, we know that there can only be a single extracted key and cert, allowing us to extract these and store the full abs path of the extracted files rather than deriving them during each client iteration. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @tbordaz @progier389 @droideck (Thanks!)

Firstyear · 2024-10-18T01:36:44Z

   6fa06a719..aaf38ee7e  389-ds-base-3.0 -> 389-ds-base-3.0
   74f2f25a4..8aa9e3423  389-ds-base-2.5 -> 389-ds-base-2.5
   7715ff141..8d82ebf06  389-ds-base-2.4 -> 389-ds-base-2.4
   545947493..b6b0db91d  389-ds-base-2.3 -> 389-ds-base-2.3
   62c77454f..20ce92890  389-ds-base-2.2 -> 389-ds-base-2.2
   f6563c36b..b8a017bf3  389-ds-base-2.1 -> 389-ds-base-2.1
   9f45be601..49d3c04b9  389-ds-base-2.0 -> 389-ds-base-2.0

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: 389ds#6349 Author: William Brown <william@blackhats.net.au> Review by: ???

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: After more testing, if the connection is dropped and restarted, the certpath is retrieved but re-extraction does not occur. This still triggers the warning however. To resolve this, we only warn about the tpm namespace during library initialisation. I really hope I got it right this time :( fixes: 389ds#6349 Author: William Brown <william@blackhats.net.au> Review by: ???

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

@progier389

Bug Description: Keys/Certs are extracted to PEM repeatedly causing many warnings during outbound TLS authenticated replication Fix Description: slapd_ssl_init() is called every time an outbound TLS connection is made, and will trigger a key extraction. If key extraction is already complete then the extraction is skipped. fixes: #6349 Author: William Brown <william@blackhats.net.au> Review by: @progier389 @tbordaz

Firstyear requested a review from tbordaz October 16, 2024 07:39

tbordaz reviewed Oct 16, 2024

View reviewed changes

ldap/servers/slapd/ssl.c Show resolved Hide resolved

ldap/servers/slapd/ssl.c Show resolved Hide resolved

Firstyear force-pushed the 6340-20241016-cert-warning branch from c12d295 to 972775c Compare October 17, 2024 00:26

Firstyear force-pushed the 6340-20241016-cert-warning branch from 972775c to 9713792 Compare October 17, 2024 00:46

Firstyear force-pushed the 6340-20241016-cert-warning branch from 9713792 to 9c77e0e Compare October 17, 2024 00:57

droideck approved these changes Oct 17, 2024

View reviewed changes

progier389 approved these changes Oct 17, 2024

View reviewed changes

tbordaz approved these changes Oct 17, 2024

View reviewed changes

Firstyear merged commit b1b4356 into 389ds:main Oct 18, 2024
197 checks passed

Firstyear deleted the 6340-20241016-cert-warning branch October 18, 2024 01:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Issue 6349 - RFE - Use previously extracted key path #6363

Issue 6349 - RFE - Use previously extracted key path #6363

Uh oh!

Firstyear commented Oct 16, 2024 •

edited

Loading

Uh oh!

Firstyear commented Oct 16, 2024

Uh oh!

progier389 commented Oct 16, 2024

Uh oh!

tbordaz left a comment

Uh oh!

Uh oh!

Uh oh!

Firstyear commented Oct 17, 2024

Uh oh!

Firstyear commented Oct 17, 2024 •

edited

Loading

Uh oh!

droideck left a comment

Uh oh!

progier389 left a comment

Uh oh!

tbordaz left a comment

Uh oh!

Uh oh!

Firstyear commented Oct 18, 2024

Uh oh!

Uh oh!

Issue 6349 - RFE - Use previously extracted key path #6363

Issue 6349 - RFE - Use previously extracted key path #6363

Uh oh!

Conversation

Firstyear commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Firstyear commented Oct 16, 2024

Uh oh!

progier389 commented Oct 16, 2024

Uh oh!

tbordaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Firstyear commented Oct 17, 2024

Uh oh!

Firstyear commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

droideck left a comment

Choose a reason for hiding this comment

Uh oh!

progier389 left a comment

Choose a reason for hiding this comment

Uh oh!

tbordaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Firstyear commented Oct 18, 2024

Uh oh!

Uh oh!

Firstyear commented Oct 16, 2024 •

edited

Loading

Firstyear commented Oct 17, 2024 •

edited

Loading