On Tue, 9 Jul 2024 at 14:57, Artyom Beilis via Boost wrote:

While this can be a useful class, it is useful mostly in the context of cryptography. So I rarely can see a case when you use it independently of a crypto library.

Additionally passwords are almost never stored as clear text so the only location I can see password is handled is in the forms you receive and usually the UI toolkits themselves handle it as string - so you don't solve it there unless you rewrite the 3rd party toolkits to use "safe" string.

Oh, you're referring to actual user passwords being stored in MySQL. We're speaking about different things. Sure, this kind of passwords must be first hashed, e.g. by using scrypt or argon2. I'm talking about the credentials require to actually log into the MySQL or Redis database server - this line, concretely: https://github.com/boostorg/mysql/blob/3faf2947f9951bb10239cb1c34cae9c571133... Best practices say that password should be erased once the connect_params object gets destroyed. While I think that's useful, there's also a similar problem when doing connection pooling - except that these credentials are usually required for the lifetime of the application, so the security gain would be much lower. See https://github.com/boostorg/mysql/blob/3faf2947f9951bb10239cb1c34cae9c571133... My use case for this secure_string class would be these two places. It's true that Boost.MySQL does require OpenSSL, indeed, so I could just use OPENSSL_Cleanse and code a Boost.MySQL specific secure_string class. But better hear people's opinion first, as it's a non-trivial task. Regards, Ruben.

On 7/9/24 15:57, Vinnie Falco via Boost wrote:

In my opinion a secure erase function which works most of the time but not all of the time is worse than not having it at all, as it may imply false claims about security.

As far as secure erase functions go, there's no variance about whether it works or not. It either works as specified in the contract or it has a bug. And it's fairly easy to make it work as intended anyway. The question is rather is the secure erase enough to consider your data safe from leaks. It definitely is not. But not allowing it to leak into heap and remain there for extended periods of time is a necessary step towards better security. Even having just that protection alone is better than not having anything at all.

On 7/9/24 18:41, Josh Juran wrote:

On Jul 9, 2024, at 10:24 AM, Andrey Semashev via Boost wrote:

...

As far as secure erase functions go, there's no variance about whether it works or not. It either works as specified in the contract or it has a bug. And it's fairly easy to make it work as intended anyway.

secret_string::~secret_string() { #if HAVE_SECURE_MEMSET

secure_memset( ptr, ‘\0’, len );

#else

memset( ptr, ‘\0’, len );

#endif

free( ptr ); }

The above will work most but not all of the time, depending on toolchain, OS, and even build settings. You might have a compiler that elides the memset() call (perhaps only at certain optimization levels) with an OS or libc that lacks secure_memset(), or you might have neglected to include the header that defines the feature-test macro.

That would be a bug in secret_string. The correct implementation would unconditionally call secure_memset and secure_memset would be implemented in a way that prevents the compiler from eliding it. The typical implementations either use volatile or a dummy asm statement that potentially consume the data to be cleared.

...

The question is rather is the secure erase enough to consider your data safe from leaks. It definitely is not. But not allowing it to leak into heap and remain there for extended periods of time is a necessary step towards better security. Even having just that protection alone is better than not having anything at all.

You might also consider having the sensitive string XORed with a one-time pad (possibly using a different allocator), so it’s never in the clear in its entirety. But I’m not a security expert and can’t speak to the efficacy of that scheme. At least it probably won’t be inadvertently undone by a sufficiently “smart” compiler.

Having XORed data and the XOR seed both in memory at the same time is pretty much the same as having the data in clear form.

A more important mitigation IMO is a timing-safe memcmp(), as the information otherwise leaked traverses not just process boundaries, but networks.

memset does not depend on the data, so normally it is not timing sensitive. memcmp and similar functions that may return early are timing-sensitive, but this is a problem separate from secure data cleanup.

On Tue, 9 Jul 2024 at 18:08, Andrey Semashev via Boost < boost@lists.boost.org> wrote:

On 7/9/24 18:41, Josh Juran wrote:

...

On Jul 9, 2024, at 10:24 AM, Andrey Semashev via Boost < boost@lists.boost.org> wrote:

...

As far as secure erase functions go, there's no variance about whether it works or not. It either works as specified in the contract or it has a bug. And it's fairly easy to make it work as intended anyway.

secret_string::~secret_string() { #if HAVE_SECURE_MEMSET

secure_memset( ptr, ‘\0’, len );

#else

memset( ptr, ‘\0’, len );

#endif

free( ptr ); }

The above will work most but not all of the time, depending on toolchain, OS, and even build settings. You might have a compiler that elides the memset() call (perhaps only at certain optimization levels) with an OS or libc that lacks secure_memset(), or you might have neglected to include the header that defines the feature-test macro.

just a thought : It would be nice if this worked: #include <string> #include <algorithm> struct secure_string : std::basic_string<volatile char> { ~secure_string() { std::fill(begin(), end(), '\0'); } }; Alas: /opt/compiler-explorer/gcc-14.1.0/include/c++/14.1.0/bits/basic_string.h:90:21: error: static assertion failed 90 | static_assert(is_same_v<_CharT, typename _Alloc::value_type>); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That would be a bug in secret_string. The correct implementation would unconditionally call secure_memset and secure_memset would be implemented in a way that prevents the compiler from eliding it. The typical implementations either use volatile or a dummy asm statement that potentially consume the data to be cleared.

...

...

The question is rather is the secure erase enough to consider your data safe from leaks. It definitely is not. But not allowing it to leak into heap and remain there for extended periods of time is a necessary step towards better security. Even having just that protection alone is better than not having anything at all.

You might also consider having the sensitive string XORed with a one-time pad (possibly using a different allocator), so it’s never in the clear in its entirety. But I’m not a security expert and can’t speak to the efficacy of that scheme. At least it probably won’t be inadvertently undone by a sufficiently “smart” compiler.

Having XORed data and the XOR seed both in memory at the same time is pretty much the same as having the data in clear form.

...

A more important mitigation IMO is a timing-safe memcmp(), as the information otherwise leaked traverses not just process boundaries, but networks.

memset does not depend on the data, so normally it is not timing sensitive.

memcmp and similar functions that may return early are timing-sensitive, but this is a problem separate from secure data cleanup.

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Such a library would be quite useful, but when I was working at Ripple our investigation concluded that it would be difficult to impossible to implement it portably in a way that could make security guarantees. I would suggest that you do not bother, as you will face many incredibly annoying obstacles at every step of the way which conspire to make your implementation fail in common cases.

P1315 is still making its way through WG21 I believe. Note that this proposal was adopted for C:

https://open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1315r3.html

Looks like most implementations are far from trivial. According to cppreference, C adopted it as part of C23 under the name memset_explicit, but stdlibs don't implement it yet. Nothing in the C++ side yet. https://en.cppreference.com/w/c/string/byte/memset

In my opinion a secure erase function which works most of the time but not all of the time is worse than not having it at all, as it may imply false claims about security. That said, I have asked an expert again for more details, which you can follow here:

https://x.com/FalcoVinnie/status/1810654344607633515

Thanks for taking the time.

Of course, I could be wrong and it is possible that newer operating systems offer robust facilities for ensuring that secrets are not leaked. This would require platform-specific implementation. If there is sufficient support for the popular platforms, it is worth exploring making into a Boost library.

There is OPENSSL_Cleanse, which does a similar job. I found it surprisingly simple - I wonder if this is just enough or there are any corner cases where this doesn't work: https://github.com/openssl/openssl/blob/b544047c99c4a7413f793afe82ab1c165f85... Other than that, there is - explicit_bzero, available on Linux and FreeBSD - ZeroSecureMemory on Windows - I can't find anything about OSX - seems they don't expose such a function So it really depends on whether we consider the volatile pointer approach enough. If that's the case, it's doable. Otherwise, it's not and we should rather wait.

Thanks

On 7/10/24 14:59, Peter Dimov via Boost wrote:

Ruben Perez wrote:

...

There is OPENSSL_Cleanse, which does a similar job. I found it surprisingly simple - I wonder if this is just enough or there are any corner cases where this doesn't work: https://github.com/openssl/openssl/blob/b544047c99c4a7413f793afe82ab1c165f85...

That's an interesting approach. I can't offhand think of a reason why it wouldn't work.

The compiler might convert OPENSSL_cleanse to something like this: void OPENSSL_cleanse(void *ptr, size_t len) { memset_t func = memset_func; if (func != memset) func(ptr, 0, len); else memset(ptr, 0, len); } The purpose here is that, if memset_func is actually memset most of the time, it can further optimize the call to memset, including to completely remove it, depending on the call context. The (well-predictable) branch is typically cheaper than an indirect call. I think I've seen compilers do something along those lines as a result of call devirtualization, especially with IPO and LTCG. I'm not saying that's what actually happens in OpenSSL, just that something like this is possible. I think, a dummy asm statement is more reliable and more efficient. void secure_cleanse(void *ptr, size_t len) { memset(ptr, 0, len); // a normal memset, optimizations are welcome __asm__ __volatile__ ("" : : "r" (ptr), "r" (len) : "memory"); } You can even make that function inline and it'll work, and in an optimal way, too.

Am 10.07.2024 um 17:37 schrieb Andrey Semashev via Boost:

On 7/10/24 18:26, Andrey Semashev wrote:

...

On 7/10/24 14:59, Peter Dimov via Boost wrote:

...

Ruben Perez wrote:

...

There is OPENSSL_Cleanse, which does a similar job. I found it surprisingly simple - I wonder if this is just enough or there are any corner cases where this doesn't work: https://github.com/openssl/openssl/blob/b544047c99c4a7413f793afe82ab1c165f85... That's an interesting approach. I can't offhand think of a reason why it wouldn't work. The compiler might convert OPENSSL_cleanse to something like this:

void OPENSSL_cleanse(void *ptr, size_t len) { memset_t func = memset_func; if (func != memset) func(ptr, 0, len); else memset(ptr, 0, len); }

The purpose here is that, if memset_func is actually memset most of the time, it can further optimize the call to memset, including to completely remove it, depending on the call context. The (well-predictable) branch is typically cheaper than an indirect call. I think I've seen compilers do something along those lines as a result of call devirtualization, especially with IPO and LTCG.

I'm not saying that's what actually happens in OpenSSL, just that something like this is possible. I think, a dummy asm statement is more reliable and more efficient.

void secure_cleanse(void *ptr, size_t len) { memset(ptr, 0, len); // a normal memset, optimizations are welcome __asm__ __volatile__ ("" : : "r" (ptr), "r" (len) : "memory"); }

You can even make that function inline and it'll work, and in an optimal way, too. And for compilers that don't support __asm__, I think you could replace it with:

std::atomic_signal_fence(std::memory_order::acq_rel);

I think Peter is actually right in his assessment: While 'memset_func' is TU-local, it is also 'volatile'. This implies every look at it might reveal a different content. Therefore the necessity for non-constness and dynamic initialization. We humans see that the variable can't change its value other than in the initialization. The compiler can't reason that because said properties are the only ones known during the compilation of 'OPENSSL_cleanse()'. It can't perform unbounded look-ahead until the end of the TU like we do. Dani -- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5

Andrey Semashev via Boost: With volatile, the compiler is not allowed to optimize away or reorder loads and stores of the variable. There's no restriction on what the compiler is allowed to do with the loaded value.

It could insert a branch against `memset` but that seems really only like a theoretical optimization. This is an example where the volatile preserves the secure_erase() call: https://godbolt.org/z/854jfarb9 Kind of neat trying to get the compiler to elide this stuff. You were right, Andrey, you really need LTO and LTCG and the like to get this kind of behavior. Super neat stuff. Idk, seems like whatever OpenSSL is doing is Sound Enough in practice. I'd be curious to know if it's actually sound for a compiler to make that kind of optimization or at least what are the limits on how far a compiler can actually go to ensure soundness. If you know those limits, you can probably make this pattern work reliably. - Christian

On 09.07.24 16:37, Andrey Semashev via Boost wrote:

On 7/9/24 17:29, Rainer Deyke via Boost wrote:

...

Passwords travel along a long chain from user input to system calls. The entire chain needs to be secure or none of it is.

Why does it have to be an "all or none" choice?

Security is always about making life *hard enough* for the attacker so that the attack is not worthwhile. It is never about making the protection impenetrable, as there is simply no such thing.

Security is about identifying weaknesses and reinforcing them, not about spraying obstacles around at random. No point in putting an extra strong lock on your front door while the back door is wide open and the east wall is missing. So: is there any real attack in the wild that can be prevented by using a secure string class? -- Rainer Deyke (rainerd@eldwood.com)

Le mardi 09 juillet 2024 à 22:14 +0200, Rainer Deyke via Boost a écrit :

So: is there any real attack in the wild that can be prevented by using a secure string class?

I think the key here is that an attack is not "prevented", but "mitigated". If the attacker has access to your memory, you already have a problem. But if it contains a lot of sensitive data, it's even worse. Iirc heartblead was that kind of failure that would have been mitigated if memory had been cleared correctly upon disposal. Regards, Julien