Hi James,
On 2/2/22 12:47, James Prestwood wrote:
This tool will decrypt an IWD network profile which was previously
encrypted using a systemd provided key. Either a text passphrase
can be provided (--pass) or a file containing the secret (--file).
You may want to provide some usage examples in the commit description. Maybe
even mention stdin/--infile and stdout/-outfile use.
This can be useful for debugging, or recovering an encrypted
profile after enabling SystemdEncrypt.
---
Makefile.am | 8 +-
tools/decrypt-profile.c | 232 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 239 insertions(+), 1 deletion(-)
create mode 100644 tools/decrypt-profile.c
diff --git a/Makefile.am b/Makefile.am
index 35938d22..a106b56f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -362,7 +362,7 @@ man_MANS += wired/ead.8
endif
endif
-noinst_PROGRAMS += tools/probe-req
+noinst_PROGRAMS += tools/probe-req tools/decrypt-profile
Should this be an installed binary? Something like iwd-decrypt-profile?
+#include <stdio.h>
+#include <unistd.h>
+#include <libgen.h> > +#include <fcntl.h>
Isn't sys/stat.h enough? In any case, you might want to group related #includes
together.
+#include <getopt.h>
+#include <sys/stat.h>
+#include <ell/ell.h>
+
+#include "ell/useful.h"
+
+#include "src/storage.h"
+
<snip>
+int main(int argc, char *argv[])
+{
+ const char *pass = NULL;
+ const char *file = NULL;
+ const char *infile = NULL;
+ const char *outfile = NULL;
+ _auto_(l_free) char *ssid = NULL;
+ _auto_(l_free) char *decrypted = NULL;
+ _auto_(l_settings_free) struct l_settings *settings = NULL;
+ _auto_(l_free) uint8_t *secret = NULL;
+ size_t secret_len = 0;
+ int ret = EXIT_FAILURE;
+ char profile[BUFSIZ];
+ ssize_t len = 0;
+ char *line;
+
+ if (isatty(0))
+ goto done_reading;
Err, why is the command line arguments not being parsed first? I mean you might
want to use 'decrypt-profile --help' ?
+
+ while ((line = fgets(profile + len, sizeof(profile) - len, stdin)))
+ len += strlen(profile + len);
You should check whether infile was specified prior to attempting this.
+
+done_reading:
+ for (;;) {
+ int opt;
+
+ opt = getopt_long(argc, argv, "pfhio",
+ main_options, NULL);
+ if (opt < 0)
+ break;
+
+ switch (opt) {
+ case 'p':
+ pass = optarg;
+ break;
+ case 'f':
+ file = optarg;
+ break;
+ case 'i':
+ infile = optarg;
+ break;
+ case 'o':
+ outfile = optarg;
+ break;
+ case 's':
+ ssid = l_strdup(optarg);
+ break;
+ case 'h':
+ usage();
+ return EXIT_SUCCESS;
+ default:
+ goto failed;
+ }
+ }
+
+ if (!file && !pass) {
+ usage();
+ goto failed;
+ }
+
So if infile specified, read that. Otherwise, if no --ssid, fail. Read
stdin...
+ if (!len && !infile) {
+ printf("No input file (use stdin or --infile)\n\n");
+ usage();
+ goto failed;
+ }
+
+ if (len && infile) {
+ printf("Use only stdin or --infile for input profile\n\n");
+ usage();
+ goto failed;
+ }
+
Check for ssid and override that.
+ if (infile) {
+ /* Try and detect SSID from input file */
+ if (!ssid) {
+ char *dot;
+
+ ssid = l_strdup(basename((char *)infile));
+ /* remove extension */
+ dot = strchr(ssid, '.');
+ if (!dot)
+ goto no_ssid;
+
+ *dot = '\0';
+ }
+
That isn't how ssids are stored in the profile. You're better off relying on
storage_network_ssid_from_path() from storage.c here.
+ len = read_file(profile, sizeof(profile), "%s",
infile);
+ if (len < 0) {
+ printf("Could not read from %s (%ld)\n", infile, len);
+ goto failed;
+ }
Why not rely on the new functions from patch 1? We could introduce a full-path
version of storage_network_open maybe.
+ }
+
+ if (!ssid) {
+no_ssid:
+ printf("Can't detect SSID from input source (use --ssid)\n");
+ goto failed;
+ }
+
+ if (pass) {
+ secret_len = strlen(pass);
+ secret = l_memdup(pass, secret_len);
Why do we need the memdup?
+ } else
+ secret = secret_from_file(file, &secret_len);
This could actually be an mmap instead.
+
+ __storage_set_encryption_key(secret, secret_len);
+
+ settings = l_settings_new();
+
+ if (!l_settings_load_from_data(settings, profile, len)) {
+ printf("Profile is not formatted correctly\n");
+ goto failed;
+ }
+
+ if (!__storage_decrypt(settings, ssid, NULL)) {
Note that you will also get errors from this function as well. So I'm not sure
if you want to re-design it to not print unnecessarily.
+ printf("Unable to decrypt profile\n");
+ goto failed;
+ }
+
+ decrypted = l_settings_to_data(settings, (size_t *)&len);
+
+ if (!outfile)
+ printf("%s\n", decrypted);
+ else {
+ int fd = open(outfile, O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR);
+
+ if (fd < 0) {
+ printf("Unable to open %s (%d)\n\n", outfile, fd);
+ close(fd);
+ goto failed;
+ }
+
+ len = write(fd, decrypted, len);
+
+ close(fd);
+
+ if (len < 0) {
+ printf("Unable to write to %s (%ld)\n\n", outfile, len);
+ goto failed;
+ }
+ }
+
+ ret = EXIT_SUCCESS;
+
+failed:
+ return ret;
+}
Regards,
-Denis