-
Notifications
You must be signed in to change notification settings - Fork 994
Enhance FLARE-VM Installer #694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
0378b25
to
5535835
Compare
As part of the default config there are chocolatey packages that were not displayed previously. Only display packages sorted by category that are relevant to RE, excluded hardcoded categories. Add a listbox that displays additional packages that are either chocolatey packages or belong to excluded categories typically used in Commando-VM. Add a new functionality to search across chocolatey packages including FLARE-VM packages.
5535835
to
5405ff9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sara-rn this is going in the right direction, it is a nice enhancement! 🚀
Please remember to add screenshots to the PR description when doing visual changes, like changes to the UI, and to add a comment with a screenshot every time you update the PR significantly. Ensure you update the installer screenshot in the README (after we are done discussing the UI) in this PR. We forgot this in the previous PR.
We need to do some improvements to make the new UI nicer and easier to read, using better the space (at the moment there is a lot space to the right of the textbox and some things are placed a bit weird):
- Increase the font. Font smaller than the currently used for the package description is difficult to read.
- Bold and increase the font of both the text on the top (
Select packages to install
) andAdditional packages to install
. They are titles and should be a bit more visible. - Move the
Add package
button next to theFind Package
. - Split the explanation in longer lines and use bullet points, using the space on the right:
FLARE-VM uses Chocolatey packages. You can add additional packages from:
- Community packages: https://community.chocolatey.org/packages
- VM-Packages: https://github.com/mandiant/VM-Packages/wiki/Packages
- Make the URLs clickable. If this is not possible, we need a way to short at least the VM-Packages URL, which is very long.
- Add a bit of space after the package list by category and before the
[Reset] [Select All][Clear] [Buttons]
. It looks a bit too compact. If needed you can make the window a bit taller to achieve this. - Correct typo in Windows title: Change
FLAREVM package selection
byFLARE-VM package section
.
Looking for packages in the community feed seems to take some time. What about adding a message Finding package...
when pressing the button after the package is found/not found to provide feedback to the user? We should add this also for the VM-Packages for consistency, although they seem to be found much faster.
If after finding the package, you modify the package name and then press Add Package
a package that hasn't been found is added. The Add Package
button should be disabled if the package name in the text box is edited. An example with microsoft-edge
:
I find the error when I add magika.vm
confusing. I think we should allow to also add packages in the list of package with category via the search textbox. Functionality wise it doesn't matter if they end up twice in the config file.
Loading the second UI window (the package selection) takes some time. The first window is closed and the user doesn't know what is going on. I think we should initialize $packagesByCategory
(doing the querying of packages per category) before opening the first installer window. This way the second window will open much faster. This should be easy to implement, just moving the call to Get-Packages-Categories
.
[nitpick] It is easy to copy trailing whitespaces when copying package names. What about removing trailing whitespaces at the beginning/end of the package name, to ensure it is found and added? If you don't implement this as part of this PR, please create an issue (before the PR is merged to avoid forgetting it).
[nitpick] It shouldn't be possible to add a package twice. If you don't implement this as part of this PR, please create an issue (before the PR is merged to avoid forgetting it). An example with dumpert.vm
:
install.ps1
Outdated
} | ||
|
||
Add-Type -AssemblyName System.Windows.Forms | ||
[System.Windows.Forms.Application]::EnableVisualStyles() | ||
$excludedCategories=@('Command and Control','Credential Access','Exploitation','Forensic','Lateral Movement', 'Payload Development','Privilege Scalation','Reconnaissance','Wordlists') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to exclude Web Application
as well. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[discussion] I am not sure if it is better to have a list of excluded categories or a list of categories. We have to main the list in either case. @mandiant/flare-vm opinions?
Add links for the choco community and VM-Packages wiki Improve font to make it bigger. Validate that the package didn't exist in the listBox before adding a new one. Disable the button Add Package if the text in the textbox changes when it is enabled. Allow to add additional packages to the listBox that are displayed as textboxes.
@Ana06 all the changes have been addressed in a separate commit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sara-rn thanks for making the improvements!
As I said in my previous review, please provide a screenshot in a comment when making changes related to the UI, it helps with the review:
It may not be clear for the user that Community Packages
and FLARE-VM Packages
are clickable. I think we should display the URL in addition to the name and make only the URL clickable (as I proposed in my previous review):
FLARE-VM uses Chocolatey packages. You can add additional packages from:
- Community packages: https://community.chocolatey.org/packages
- VM-Packages: https://github.com/mandiant/VM-Packages/wiki/Packages
I think we should remove the :
in Additional packages to install:
, as the other titles do not have it. I think it would also be easier to understand if we add a bit more of space before Additional packages to install:
and move the text FLARE-VM uses Chocolatey packages...
a bit down so that it is not at the same level than the title Additional packages to install
.
Please add more details to the error for a duplicated package, for example: Error to add the package: duplicated
.
[nitpick] Trailing whitespaces are still not removed. If you don't plan to implement it, please open an issue to track the improvement suggestion.
Ensure the linter passes and squash the commits (we do not keep fixup
commits in order to maintain a clean commit history). Please also update the installer screenshot in the README, I don't expect the UI to change much more after you address the proposed changes.
@@ -469,6 +469,10 @@ Start-Sleep 1 | |||
$configXml = [xml](Get-Content $configPath) | |||
|
|||
if (-not $noGui.IsPresent) { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick]
$textLabel = "The default configuration (recommended for reverse engineering) is pre-selected.`nClick on the reset button to restore the default configuration." | ||
} else { | ||
$textLabel = "Select packages to install. The provided custom configuration is pre-selected.`nClick on the reset button to restore the custom configuration." | ||
$textLabel = "The provided custom configuration is pre-selected.`nClick on the reset button to restore the custom configuration." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove the new line now that the message is shorter (as Select packages to install
is now in a different line) and display everything in the same line.
As part of the default config there are chocolatey packages that were not displayed previously.
Only display packages sorted by category that are relevant to RE, excluded hardcoded categories.
Add a listbox that displays additional packages that are either chocolatey packages or belong to excluded categories typically used in Commando-VM.
Add a new functionality to search across chocolatey packages including FLARE-VM packages to add new packages
Closes #669
Closes #692