Skip to content

Commit

Permalink
826: Restrict permissions to conf files.
Browse files Browse the repository at this point in the history
So that, on Windows, normal Users are not able to access any files
that were backed up.
Longer term strategy might be to move the sensitive files to a
different directory.

Change-Id: Ibe25169bfe4a691dd944fbf2e8a0512b54879170
  • Loading branch information
grke committed Jun 1, 2021
1 parent 524f5c1 commit def887e
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 2 deletions.
15 changes: 15 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
Things to watch out for when upgrading.

2.5.0
-----
The Windows installer sets Administrator and SYSTEM full access permissions
only on:
C:/Program Files/Burp/bin/burp.conf
C:/Program Files/Burp/bin/ssl_cert_ca.pem
C:/Program Files/Burp/bin/ssl_cert-client.key
C:/Program Files/Burp/bin/ssl_cert-client.pem
That is, it runs:
icacls.exe "<path>" /inheritance:r /grant:r Administrators:F SYSTEM:F
If you have these files on non-default locations, and you are concerned about
restricting permissions, you may want to run the command by hand on them.
The Windows client will automatically apply the same permissions to new ssl
cert files on certificate exchange.

2.3.12
------
On Windows, the '-x' option has been split into two different options - '-x'
Expand Down
23 changes: 22 additions & 1 deletion src/client/ca.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@
#include "cvss.h"
#include "ca.h"

#ifdef HAVE_WIN32
// Want to avoid giving standard users access to the conf files, otherwise
// they can get access to all the backed up files, which may include files
// they shouldn't be able to get.
static void hack_windows_perms(const char *path)
{
char cmd[512]="";
snprintf(cmd, sizeof(cmd), "icacls.exe \"%s\" /inheritance:r /grant:r Administrators:F SYSTEM:F", path);
system(cmd);
}
#endif

static int generate_key_and_csr(struct asfd *asfd,
struct conf **confs, const char *csr_path)
{
Expand Down Expand Up @@ -48,6 +60,11 @@ static int generate_key_and_csr(struct asfd *asfd,
return -1;
}

#ifdef HAVE_WIN32
hack_windows_perms(ssl_key);
hack_windows_perms(csr_path);
#endif

return 0;
}

Expand Down Expand Up @@ -213,7 +230,6 @@ int ca_client_setup(struct asfd *asfd, struct conf **confs)

// The server will then sign it, and give it back.
if(receive_a_file(asfd, ssl_cert_tmp, cntr)) goto end_cleanup;

// The server will also send the CA certificate.
if(receive_a_file(asfd, ssl_cert_ca_tmp, cntr)) goto end_cleanup;

Expand All @@ -223,6 +239,11 @@ int ca_client_setup(struct asfd *asfd, struct conf **confs)
|| do_rename(ssl_cert_ca_tmp, ssl_cert_ca))
goto end_cleanup;

#ifdef HAVE_WIN32
hack_windows_perms(ssl_cert);
hack_windows_perms(ssl_cert_ca);
#endif

// Need to rewrite our configuration file to contain the server
// name (ssl_peer_cn) if the name differs from the config file.
if(strncmp_w(ssl_peer_cn_old, get_string(confs[OPT_SSL_PEER_CN])))
Expand Down
3 changes: 2 additions & 1 deletion src/win32/Makefile.installer
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ winburp.nsi:

$(INSTALL_EXE): winburp.nsi $(addprefix release$(BITS)/,$(BURP_BINARIES) $(DEPKGS_BINARIES) $(MINGW_LIBS))
# Some hackery to make the final structure nicer.
rm -f bin
rm -rf bin
mkdir bin
rm -rf CA
mkdir CA
mv release$(BITS)/* bin
mv bin/*.conf release$(BITS)
Expand Down
12 changes: 12 additions & 0 deletions src/win32/installer/winburp.nsi
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,18 @@ SectionGroupEnd
Section "-Finish"
Push $R0

; Permissions
nsExec::ExecToLog '$SYSDIR\icacls.exe "$INSTDIR\burp.conf" /inheritance:r /grant:r Administrators:F SYSTEM:F'
; More permissions.
; These files are created at runtime by the burp client, which also
; sets their permissions. See src/client/ca.c.
; Clients older than 2.5.0 did not do that, so the following fixes up
; the files in their default locations on upgrade instead.
nsExec::ExecToLog '$SYSDIR\icacls.exe "$INSTDIR\CA\*" /inheritance:r /grant:r Administrators:F SYSTEM:F'
nsExec::ExecToLog '$SYSDIR\icacls.exe "$INSTDIR\ssl_cert_ca.pem" /inheritance:r /grant:r Administrators:F SYSTEM:F'
nsExec::ExecToLog '$SYSDIR\icacls.exe "$INSTDIR\ssl_cert-client.key" /inheritance:r /grant:r Administrators:F SYSTEM:F'
nsExec::ExecToLog '$SYSDIR\icacls.exe "$INSTDIR\ssl_cert-client.pem" /inheritance:r /grant:r Administrators:F SYSTEM:F'

; Write the uninstall keys for Windows.
WriteRegStr HKLM "Software\Microsoft\Windows\CurrentVersion\Uninstall\${PACKAGE_NAME}" "DisplayName" "${PACKAGE_NAME}"
WriteRegStr HKLM "Software\Microsoft\Windows\CurrentVersion\Uninstall\${PACKAGE_NAME}" "InstallLocation" "$INSTDIR"
Expand Down

0 comments on commit def887e

Please sign in to comment.