From 8a027b98d1b9341c1b636fbda0c1b2f6bf62b531 Mon Sep 17 00:00:00 2001 From: Martin Corino Date: Mon, 26 Feb 2024 09:39:43 +0100 Subject: [PATCH] Fix handling of binary secrets in wxSecretStore with wxGTK Use correct content type when creating binary secrets to prevent libsecret from munging them. Also update the sample to show more functionality. Closes #24351. Closes #24352. --- include/wx/secretstore.h | 7 +- samples/secretstore/secretstore.cpp | 153 ++++++++++++++++++++++++---- src/msw/secretstore.cpp | 5 +- src/osx/core/secretstore.cpp | 5 +- src/unix/secretstore.cpp | 18 ++-- 5 files changed, 154 insertions(+), 34 deletions(-) diff --git a/include/wx/secretstore.h b/include/wx/secretstore.h index 3b9cdec6ed..265161d45c 100644 --- a/include/wx/secretstore.h +++ b/include/wx/secretstore.h @@ -40,7 +40,7 @@ public: // Creates a secret value from the given data. wxSecretValue(size_t size, const void *data) - : m_impl(NewImpl(size, data)) + : m_impl(NewImpl(size, data, "application/octet-stream")) { } @@ -48,7 +48,7 @@ public: explicit wxSecretValue(const wxString& secret) { const wxScopedCharBuffer buf(secret.utf8_str()); - m_impl = NewImpl(buf.length(), buf.data()); + m_impl = NewImpl(buf.length(), buf.data(), "text/plain"); } wxSecretValue(const wxSecretValue& other); @@ -90,7 +90,8 @@ public: private: // This method is implemented in platform-specific code and must return a // new heap-allocated object initialized with the given data. - static wxSecretValueImpl* NewImpl(size_t size, const void *data); + static wxSecretValueImpl* + NewImpl(size_t size, const void *data, const char* contentType); // This ctor is only used by wxSecretStore and takes ownership of the // provided existing impl pointer. diff --git a/samples/secretstore/secretstore.cpp b/samples/secretstore/secretstore.cpp index 7b591eaf3c..b39daa37c1 100644 --- a/samples/secretstore/secretstore.cpp +++ b/samples/secretstore/secretstore.cpp @@ -25,6 +25,11 @@ #include "wx/crt.h" #include "wx/log.h" +#include + +bool g_verbose_ {false}; +bool g_binary_ {false}; + bool Save(wxSecretStore& store, const wxString& service, const wxString& user) { char password[4096]; @@ -107,12 +112,50 @@ static bool PrintResult(bool ok) return ok; } +static void PrintSecrets(const wxSecretValue& expected, const wxSecretValue& loaded) +{ + wxPrintf("Expected: size=%ld data=", expected.GetSize()); + if (g_binary_) + { + wxPrintf("["); + size_t n = expected.GetSize(); + const char* p = static_cast(expected.GetData()); + for (size_t i=0; i(loaded.GetData()); + for (size_t i=0; i \n" + " or %s {load|delete} \n" + " or %s [options] selftest \n" + "\n" + "Sample showing wxSecretStore class functionality.\n" + "Specify one of the commands to perform the corresponding\n" + "function call. The \"service\" argument is mandatory for\n" + "all commands, \"save\" also requires \"user\" and will\n" + "prompt for password.\n\n" + "options:\n" + "\t-v\trun verbose (possibly shows secrets on errors)\n" + "\t-b\trun selftest using binary secrets (otherwise uses text strings)\n\n", + argv[0], argv[0], argv[0]); +} + int main(int argc, char **argv) { // To complement the standard EXIT_{SUCCESS,FAILURE}. @@ -176,20 +247,59 @@ int main(int argc, char **argv) return EXIT_FAILURE; } - if ( argc < 2 || - argc != (argv[1] == wxString("save") ? 4 : 3) ) + wxString operation; + wxString service; + wxString user; + + for (int arg=1; arg \n" - " or %s {load|delete|selftest} \n" - "\n" - "Sample showing wxSecretStore class functionality.\n" - "Specify one of the commands to perform the corresponding\n" - "function call. The \"service\" argument is mandatory for\n" - "all commands, \"save\" also requires \"user\" and will\n" - "prompt for password.\n", - argv[0], argv[0]); + "ERROR: Unknown argument : %s\n\n", + argv[arg]); + usage(argv); return EXIT_SYNTAX; + } + } + + if (operation.IsEmpty() || service.IsEmpty()) + { + if (operation.IsEmpty()) + wxFprintf(stderr, + "ERROR: Missing required operation argument\n\n"); + else + wxFprintf(stderr, + "ERROR: Missing required service argument\n\n"); + usage(argv); + return EXIT_SYNTAX; } wxSecretStore store = wxSecretStore::GetDefault(); @@ -201,13 +311,17 @@ int main(int argc, char **argv) return EXIT_FAILURE; } - const wxString operation = argv[1]; - const wxString service = argv[2]; - bool ok; if ( operation == "save" ) { - ok = Save(store, service, argv[3]); + if (user.IsEmpty()) + { + wxFprintf(stderr, + "ERROR: Missing required user argument\n\n"); + usage(argv); + return EXIT_SYNTAX; + } + ok = Save(store, service, user); } else if ( operation == "load" ) { @@ -225,8 +339,9 @@ int main(int argc, char **argv) { wxFprintf(stderr, "Unknown operation \"%s\", expected \"save\", \"load\" or " - "\"delete\".\n", + "\"delete\".\n\n", operation); + usage(argv); return EXIT_SYNTAX; } diff --git a/src/msw/secretstore.cpp b/src/msw/secretstore.cpp index 5ff0371882..f619ab0d99 100644 --- a/src/msw/secretstore.cpp +++ b/src/msw/secretstore.cpp @@ -134,7 +134,10 @@ public: // ============================================================================ /* static */ -wxSecretValueImpl* wxSecretValue::NewImpl(size_t size, const void *data) +wxSecretValueImpl* +wxSecretValue::NewImpl(size_t size, + const void *data, + const char* WXUNUSED(contentType)) { return new wxSecretValueGenericImpl(size, data); } diff --git a/src/osx/core/secretstore.cpp b/src/osx/core/secretstore.cpp index 50d2eef5f7..d61f861db0 100644 --- a/src/osx/core/secretstore.cpp +++ b/src/osx/core/secretstore.cpp @@ -240,7 +240,10 @@ private: // ============================================================================ /* static */ -wxSecretValueImpl* wxSecretValue::NewImpl(size_t size, const void *data) +wxSecretValueImpl* +wxSecretValue::NewImpl(size_t size, + const void *data, + const char* WXUNUSED(contentType)) { return new wxSecretValueGenericImpl(size, data); } diff --git a/src/unix/secretstore.cpp b/src/unix/secretstore.cpp index 6f76b55ff9..d8f6c575d9 100644 --- a/src/unix/secretstore.cpp +++ b/src/unix/secretstore.cpp @@ -68,15 +68,10 @@ private: class wxSecretValueLibSecretImpl : public wxSecretValueImpl { public: - // Create a new secret value. - // - // Notice that we have to use text/plain as content type and not - // application/octet-stream which would have been more logical because - // libsecret accepts only valid UTF-8 strings for the latter, while our - // data is not necessarily UTF-8 (nor even text at all...). - wxSecretValueLibSecretImpl(size_t size, const void* data) + // Create a new secret value for the given content type. + wxSecretValueLibSecretImpl(size_t size, const void* data, const char* contentType) : m_value(secret_value_new(static_cast(data), size, - "text/plain")) + contentType)) { } @@ -362,9 +357,12 @@ const char* wxSecretStoreLibSecretImpl::FIELD_USER = "user"; // ============================================================================ /* static */ -wxSecretValueImpl* wxSecretValue::NewImpl(size_t size, const void *data) +wxSecretValueImpl* +wxSecretValue::NewImpl(size_t size, + const void *data, + const char* contentType) { - return new wxSecretValueLibSecretImpl(size, data); + return new wxSecretValueLibSecretImpl(size, data, contentType); } /* static */