Skip to content
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

restart EMBA functionality #1078

Merged
merged 1 commit into from
Mar 6, 2024
Merged

restart EMBA functionality #1078

merged 1 commit into from
Mar 6, 2024

Conversation

m-1-k-3
Copy link
Member

@m-1-k-3 m-1-k-3 commented Mar 6, 2024

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Bug fix

  • What is the current behavior? (You can also link to an open issue here)

see #1077 and #1073

  • What is the new behavior (if this is a feature change)? If possible add a screenshot.

the original "buggy" restart functionality should be recovered
closes #1077

image

@m-1-k-3 m-1-k-3 added bug Something isn't working EMBA labels Mar 6, 2024
@m-1-k-3 m-1-k-3 requested a review from HoxhaEndri March 6, 2024 11:27
@HoxhaEndri HoxhaEndri merged commit 3eb3f96 into e-m-b-a:master Mar 6, 2024
14 checks passed
@CowBoy4mH3LL
Copy link

Ok so that did not completely solve the problem.

If we consider the profile full and final and something was terminated, the string "Test ended" would not be there and check_emba_ended() would not return true and restart ensues.

But, if the analyst incrementally changes the profile, we have a problem. For example, in my profile I had everything black listed except the P02 -- cause I wanted to just see the initial analysis and make my choices after that. In this case, the analysis obviously finished successfully and "Test ended" was logged at the end of the log file

[!] Wed Mar  6 23:49:49 EST 2024 - Test ended on Wed Mar  6 23:49:49 EST 2024 and took about 0 days and 00:00:36 

Next time, when I enabled the corresponding extraction/unpacking module etc. the restart would fail. As such, the differential restart requirement was raised in #1073. That is, at this point, if I replied to using "n" at the log deletion prompt EMBA would terminate because check_emba_ended() would not correctly indicate that emba has NOT_FINISHED in the log_folder() function.

@CowBoy4mH3LL
Copy link

Currently the way I am handling this is by exporting a KEEP_GOING=1 variable in my profile file and making two changes

  1. in check_emba_ended() I add a check at the beginning of the function. This ensures the NOT_FINISHED env is set to 1 at line 38 in log_folder()
check_emba_ended() {
  if [ $KEEP_GOING -eq 1 ]; then
    return 1
  fi
  if grep -q "Test ended" "${LOG_DIR}""/""${MAIN_LOG_FILE}"; then
    # EMBA is already finished
    return 0
  fi
  return 1
}
  1. In log_folder(), I added a patch at line 69 that ensures the KEEP_GOING variable is respected
if [[ ${OVERWRITE_LOG} -eq 1 ]] ; then
      ANSWER="y"
    elif [[ ${KEEP_GOING} -eq 1 ]] ; then
      ANSWER="n" 
    else
      read -p "(Y/n)  " -r ANSWER
    fi

Now it works.

@CowBoy4mH3LL
Copy link

But ideally we should the following

  1. If the firmware digest is different -- terminate and ask for a new log path
  2. If they are the same, keep going without prompting for deletion at all because there is already a switch (-y) to do that. Instead, act as if this is a restart.

@m-1-k-3
Copy link
Member Author

m-1-k-3 commented Mar 7, 2024

If we consider the profile full and final and something was terminated, the string "Test ended" would not be there and check_emba_ended() would not return true and restart ensues.

fine, this is the idea of this functionality.

But, if the analyst incrementally changes the profile, we have a problem. For example, in my profile I had everything black listed except the P02 -- cause I wanted to just see the initial analysis and make my choices after that. In this case, the analysis obviously finished successfully and "Test ended" was logged at the end of the log file

You could remove the "Test ended" entry in the log file via sed or so.

[!] Wed Mar  6 23:49:49 EST 2024 - Test ended on Wed Mar  6 23:49:49 EST 2024 and took about 0 days and 00:00:36 

Next time, when I enabled the corresponding extraction/unpacking module etc. the restart would fail. As such, the differential restart requirement was raised in #1073. That is, at this point, if I replied to using "n" at the log deletion prompt EMBA would terminate because check_emba_ended() would not correctly indicate that emba has NOT_FINISHED in the log_folder() function.

Yes, if the test is ended then the current functionality does not cover your use case and you need to adjust emba.log or log to a new log directory or overwrite the current log.

@m-1-k-3
Copy link
Member Author

m-1-k-3 commented Mar 8, 2024

But ideally we should the following

  1. If the firmware digest is different -- terminate and ask for a new log path
  2. If they are the same, keep going without prompting for deletion at all because there is already a switch (-y) to do that. Instead, act as if this is a restart.

ad 2 - As the complete rescan feature is not that well tested I would not do it as default but we can introduce your approach you have shown in here #1078 (comment)
With this in place a user can set an environment variable and then the rescan will catch in, otherwise everything stays the same. Could you do a PR for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working EMBA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scan restart not working as expected
3 participants