Add support for extended ASCII in password generator by TheZ3ro · Pull Request #538 · keepassxreboot/keepassxc

TheZ3ro · 2017-04-28T20:08:04Z

Resolve #343

Description

This adds support for extended ASCII character in the password generator, like KeePass.

How has this been tested?

Manually and with Travis

Screenshots (if appropriate):

Types of changes

✅ New feature (non-breaking change which adds functionality)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

louib · 2017-04-28T20:26:42Z

src/gui/PasswordGeneratorWidget.cpp

    m_ui->checkBoxUpper->setChecked(config()->get("generator/UpperCase", true).toBool());
    m_ui->checkBoxNumbers->setChecked(config()->get("generator/Numbers", true).toBool());
    m_ui->checkBoxSpecialChars->setChecked(config()->get("generator/SpecialChars", false).toBool());
+    m_ui->checkBoxExtASCII->setChecked(config()->get("generator/EASCII", false).toBool());


@TheZ3ro I think we could call this one checkBoxExtendedASCII or checkBoxEASCII

@louib isn't Ext an abbreviation for Extended? I will change this to checkBoxExtendedASCII if Ext don't fit.

@TheZ3ro I don't mind the extra characters to make it explicit, but maybe that's just me.

I vote for the name as is. It's descriptive to me.

I agree with @louib too. Less confusion that way.

louib · 2017-04-28T20:30:02Z

@TheZ3ro I guess we could also blacklist those, yeah.
Is there any unit tests to add? I don't see a test file for password generators.

TheZ3ro · 2017-04-28T21:20:30Z

@louib I was also wondering about what to do. I think we can add a new test file for them

louib · 2017-04-30T15:57:58Z

@TheZ3ro also, I searched for look-alike inside of the EASCII group, but I did not pay attention to possible look-alike across groups with EASCII enabled, which the other classes seem to be handling. So we should see if there's any more look-alikes to exclude.

TheZ3ro · 2017-04-30T16:38:22Z

@louib this is the code for EASCII and SpecialChar from KeePass source:

if(m_strHighAnsi == null)
{
	StringBuilder sbHighAnsi = new StringBuilder();
	// [U+0080, U+009F] are C1 control characters,
	// U+00A0 is non-breaking space
	for(char ch = '\u00A1'; ch <= '\u00AC'; ++ch)
		sbHighAnsi.Append(ch);
	// U+00AD is soft hyphen (format character)
	for(char ch = '\u00AE'; ch < '\u00FF'; ++ch)
		sbHighAnsi.Append(ch);
	sbHighAnsi.Append('\u00FF');

	m_strHighAnsi = sbHighAnsi.ToString();
}

if(m_strSpecial == null)
{
	PwCharSet pcs = new PwCharSet(false);
	pcs.AddRange('!', '/');
	pcs.AddRange(':', '@');
	pcs.AddRange('[', '`');
	pcs.Add(@"|~");
	pcs.Remove(@"-_ ");
	pcs.Remove(PwCharSet.Brackets);

	m_strSpecial = pcs.ToString();
}

This is the only look alike

public const string LookAlike = @"O0l1I|";

louib · 2017-04-30T17:03:28Z

src/core/PasswordGenerator.h

        Numbers           = 0x4,
        SpecialCharacters = 0x8,
-        EASCII            = 0x16
+        EASCII            = 0x10


@TheZ3ro 😮 my bad, nice catch!

TheZ3ro · 2017-04-30T23:01:33Z

@louib how do you think we should test the password generator? Generating random password and checking if they respect the char restriction from their groups?

Generating one password doesn't really tell you if the generator is working properly (maybe the output password is valid only because it didn't selected an invalid char for this specific generation)
Should I generate N password?

And also, the restriction checking mechanism is based on the generator itself so it will pass the tests also in case where it should fail.

Do we need tests for the generator itself?

louib · 2017-05-01T14:27:11Z

Generating random password and checking if they respect the char restriction from their groups?

Pretty much what I was thinking, but I guess we can go ahead without testing this.

- Added YubiKey 2FA integration for unlocking databases [#127] - Added TOTP support [#519] - Added CSV import tool [#146, #490] - Added KeePassXC CLI tool [#254] - Added diceware password generator [#373] - Added support for entry references [#370, #378] - Added support for Twofish encryption [#167] - Enabled DEP and ASLR for in-memory protection [#371] - Enabled single instance mode [#510] - Enabled portable mode [#645] - Enabled database lock on screensaver and session lock [#545] - Redesigned welcome screen with common features and recent databases [#292] - Multiple updates to search behavior [#168, #213, #374, #471, #603, #654] - Added auto-type fields {CLEARFIELD}, {SPACE}, {{}, {}} [#267, #427, #480] - Fixed auto-type errors on Linux [#550] - Prompt user prior to executing a cmd:// URL [#235] - Entry attributes can be protected (hidden) [#220] - Added extended ascii to password generator [#538] - Added new database icon to toolbar [#289] - Added context menu entry to empty recycle bin in databases [#520] - Added "apply" button to entry and group edit windows [#624] - Added macOS tray icon and enabled minimize on close [#583] - Fixed issues with unclean shutdowns [#170, #580] - Changed keyboard shortcut to create new database to CTRL+SHIFT+N [#515] - Compare window title to entry URLs [#556] - Implemented inline error messages [#162] - Ignore group expansion and other minor changes when making database "dirty" [#464] - Updated license and copyright information on souce files [#632] - Added contributors list to about dialog [#629]

Louis-Bertrand Varin and others added 2 commits April 28, 2017 19:01

Adding EASCII character class.

fa7c945

Add Extended ASCII to password generator

b474d34

louib reviewed Apr 28, 2017

View reviewed changes

louib reviewed Apr 30, 2017

View reviewed changes

droidmonkey added this to the v2.2.0 milestone Apr 30, 2017

droidmonkey approved these changes Apr 30, 2017

View reviewed changes

Only printable extended ASCII

40b4dc3

louib approved these changes May 1, 2017

View reviewed changes

TheZ3ro changed the title ~~WIP: Add support for extended ASCII in password generator~~ Add support for extended ASCII in password generator May 1, 2017

TheZ3ro added the new feature label May 1, 2017

TheZ3ro merged commit 7040bef into develop May 1, 2017

TheZ3ro deleted the feature/extendedAscii branch May 1, 2017 16:14

phoerious added pr: new feature Pull request adds a new feature and removed new feature labels Nov 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for extended ASCII in password generator#538

Add support for extended ASCII in password generator#538
TheZ3ro merged 3 commits intodevelopfrom
feature/extendedAscii

TheZ3ro commented Apr 28, 2017 •

edited

Loading

Uh oh!

louib Apr 28, 2017

Uh oh!

TheZ3ro Apr 28, 2017

Uh oh!

louib Apr 30, 2017

Uh oh!

droidmonkey Apr 30, 2017

Uh oh!

eternaltyro May 6, 2017

Uh oh!

louib commented Apr 28, 2017

Uh oh!

TheZ3ro commented Apr 28, 2017

Uh oh!

louib commented Apr 30, 2017

Uh oh!

TheZ3ro commented Apr 30, 2017

Uh oh!

louib Apr 30, 2017

Uh oh!

TheZ3ro commented Apr 30, 2017

Uh oh!

louib commented May 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

TheZ3ro commented Apr 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How has this been tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

louib Apr 28, 2017

Choose a reason for hiding this comment

Uh oh!

TheZ3ro Apr 28, 2017

Choose a reason for hiding this comment

Uh oh!

louib Apr 30, 2017

Choose a reason for hiding this comment

Uh oh!

droidmonkey Apr 30, 2017

Choose a reason for hiding this comment

Uh oh!

eternaltyro May 6, 2017

Choose a reason for hiding this comment

Uh oh!

louib commented Apr 28, 2017

Uh oh!

TheZ3ro commented Apr 28, 2017

Uh oh!

louib commented Apr 30, 2017

Uh oh!

TheZ3ro commented Apr 30, 2017

Uh oh!

louib Apr 30, 2017

Choose a reason for hiding this comment

Uh oh!

TheZ3ro commented Apr 30, 2017

Uh oh!

louib commented May 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

TheZ3ro commented Apr 28, 2017 •

edited

Loading