Merge pull request #6 from henrygab/rand_clarity

Remove Bias and clarify use of TRNG

Author: anakod

.github/workflows/build.yml

@@ -0,0 +1,36 @@
+name: "FAP: Build and lint"
+on: [push, pull_request]
+jobs:
+  ufbt-build-action:
+    runs-on: ubuntu-latest
+    name: 'ufbt: Build for Release branch'
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Setup ufbt
+        uses: flipperdevices/flipperzero-ufbt-action@v0.1.2
+        with:
+          task: setup
+          sdk-channel: release
+
+      - name: Build with ufbt
+        uses: flipperdevices/flipperzero-ufbt-action@v0.1.2
+        id: build-app
+        with:
+          skip-setup: true
+          sdk-channel: release
+
+      - name: Upload app artifacts
+        uses: actions/upload-artifact@v3
+        with:
+          name: ${{ github.event.repository.name }}-${{ steps.build-app.outputs.suffix }}
+          path: ${{ steps.build-app.outputs.fap-artifacts }}
+
+      # # You can remove this step if you don't want to check source code formatting
+      # - name: Lint sources
+      #   uses: flipperdevices/flipperzero-ufbt-action@v0.1.2
+      #   with:
+      #     # skip SDK setup, we already did it in previous step
+      #     skip-setup: true
+      #     task: lint

application.fam

@@ -1,12 +1,12 @@
 App(
     appid="passgen",
     name="Password Generator",
-    apptype=FlipperAppType.PLUGIN,
+    apptype=FlipperAppType.EXTERNAL,
     entry_point="passgenapp",
     requires=[
         "gui",
     ],
     fap_category="Tools",
     fap_icon="icons/passgen_icon.png",
     fap_icon_assets="icons",
-)
+)

passgen.c

@@ -1,4 +1,5 @@
 #include <furi.h>
+#include <furi_hal_random.h>
 #include <gui/gui.h>
 #include <gui/elements.h>
 #include <input/input.h>
@@ -27,6 +28,25 @@ typedef enum PassGen_Alphabet
 	Mixed = DigitsAllLetters | Special
 } PassGen_Alphabet;
 
+const char * const PassGen_AlphabetChars [16] = {
+	"0", // invalid value
+	/*    PASSGEN_SPECIAL    PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW */ PASSGEN_DIGITS   ,
+	/*    PASSGEN_SPECIAL    PASSGEN_LETTERS_UP */ PASSGEN_LETTERS_LOW /* PASSGEN_DIGITS */,
+	/*    PASSGEN_SPECIAL    PASSGEN_LETTERS_UP */ PASSGEN_LETTERS_LOW    PASSGEN_DIGITS   ,
+	/*    PASSGEN_SPECIAL */ PASSGEN_LETTERS_UP /* PASSGEN_LETTERS_LOW    PASSGEN_DIGITS */,
+	/*    PASSGEN_SPECIAL */ PASSGEN_LETTERS_UP /* PASSGEN_LETTERS_LOW */ PASSGEN_DIGITS   ,
+	/*    PASSGEN_SPECIAL */ PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW /* PASSGEN_DIGITS */,
+	/*    PASSGEN_SPECIAL */ PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW    PASSGEN_DIGITS   ,
+	      PASSGEN_SPECIAL /* PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW    PASSGEN_DIGITS */,
+	      PASSGEN_SPECIAL /* PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW */ PASSGEN_DIGITS   ,
+	      PASSGEN_SPECIAL /* PASSGEN_LETTERS_UP */ PASSGEN_LETTERS_LOW /* PASSGEN_DIGITS */,
+	      PASSGEN_SPECIAL /* PASSGEN_LETTERS_UP */ PASSGEN_LETTERS_LOW    PASSGEN_DIGITS   ,
+	      PASSGEN_SPECIAL    PASSGEN_LETTERS_UP /* PASSGEN_LETTERS_LOW    PASSGEN_DIGITS */,
+	      PASSGEN_SPECIAL    PASSGEN_LETTERS_UP /* PASSGEN_LETTERS_LOW */ PASSGEN_DIGITS   ,
+	      PASSGEN_SPECIAL    PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW /* PASSGEN_DIGITS */,
+	      PASSGEN_SPECIAL    PASSGEN_LETTERS_UP    PASSGEN_LETTERS_LOW    PASSGEN_DIGITS   ,
+};
+
 const int AlphabetLevels[] = { Digits, Lowercase, DigitsLower, DigitsAllLetters, Mixed };
 const char* AlphabetLevelNames[] = { "1234", "abcd", "ab12", "Ab12", "Ab1#" };
 const int AlphabetLevelsCount = sizeof(AlphabetLevels) / sizeof(int);
@@ -45,13 +65,18 @@ typedef struct {
     Gui* gui;
     FuriMutex** mutex;
 	NotificationApp* notify;
+	const char* alphabet;
 	char password[PASSGEN_MAX_LENGTH+1];
-	char alphabet[PASSGEN_CHARACTERS_LENGTH+1];
-	int length;
+	int length; // must be <= PASSGEN_MAX_LENGTH
 	int level;
 } PassGen;
 
 void state_free(PassGen* app) {
+	// NOTE: would have preferred if a "safe" memset() was available...
+	//       but, since cannot prevent optimization from removing
+	//       memset(), fill with random data instead.
+	furi_hal_random_fill_buf((void*)(app->password), PASSGEN_MAX_LENGTH);
+
     gui_remove_view_port(app->gui, app->view_port);
     furi_record_close(RECORD_GUI);
     view_port_free(app->view_port);
@@ -100,19 +125,16 @@ static void render_callback(Canvas* canvas, void* ctx) {
 void build_alphabet(PassGen* app)
 {
 	PassGen_Alphabet mode = AlphabetLevels[app->level];
-	app->alphabet[0] = '\0';
-	if ((mode & Digits) != 0)
-		strcat(app->alphabet, PASSGEN_DIGITS);
-	if ((mode & Lowercase) != 0)
-		strcat(app->alphabet, PASSGEN_LETTERS_LOW);
-	if ((mode & Uppercase) != 0)
-		strcat(app->alphabet, PASSGEN_LETTERS_UP);
-	if ((mode & Special) != 0)
-		strcat(app->alphabet, PASSGEN_SPECIAL);
+	if (mode > 0 && mode < 16) {
+		app->alphabet = PassGen_AlphabetChars[mode];
+	} else {
+		app->alphabet = PassGen_AlphabetChars[0]; // Invalid mode ... password will be all zero digits
+	}
 }
 
 PassGen* state_init() {
     PassGen* app = malloc(sizeof(PassGen));
+	_Static_assert(8 <= PASSGEN_MAX_LENGTH, "app->length must be set <= PASSGEN_MAX_LENGTH");
 	app->length = 8;
 	app->level = 2;
 	build_alphabet(app);
@@ -131,13 +153,46 @@ PassGen* state_init() {
 
 void generate(PassGen* app)
 {
-	int hi = strlen(app->alphabet);
-	for (int i=0; i<app->length; i++)
-	{
-		int x = rand() % hi;
-		app->password[i]=app->alphabet[x];
+	memset(app->password, 0, PASSGEN_MAX_LENGTH+1);
+
+	int char_option_count = strlen(app->alphabet);
+	if (char_option_count < 0) {
+		return;
+	}
+
+	// determine largest character value that avoids bias
+	char ceil = CHAR_MAX - (CHAR_MAX % char_option_count) - 1;
+
+	// iteratively fill the password buffer with random values
+	// then keep only values that are in-range (no bias)
+	void* remaining_buffer = app->password;
+	size_t remaining_length = (app->length * sizeof(char));
+
+	while (remaining_length != 0) {
+		// fewer calls to hardware TRNG is more efficient
+		furi_hal_random_fill_buf(remaining_buffer, remaining_length);
+
+		// keep only values that are in-range (no bias)
+		char* target = remaining_buffer;
+		char* source = remaining_buffer;
+		size_t valid_count = 0;
+
+		for (size_t i = 0; i < remaining_length; i++) {
+			int v = *source;
+			// if the generated random value is in range, keep it
+			if (v < ceil) {
+				v %= char_option_count;
+				*target = app->alphabet[v];
+				// increment target pointer and count of valid items found
+				target++;
+				valid_count++;
+			}
+			// always increment the source pointer
+			source++;
+		}
+		remaining_length -= valid_count;
+		remaining_buffer = target;
 	}
-	app->password[app->length] = '\0';
 }
 
 void update_password(PassGen* app, bool vibro)