Skip to content

Commit

Permalink
cram-md5: do not accept challenge if own resource name is used
Browse files Browse the repository at this point in the history
Fixes #1250: Authentication bypass in Director

use the unified-resource-name for the cram challenge
i.e. auth cram-md5 <1001326377.1591525437@R_CLIENT::backup-bareos-test-fd>
  • Loading branch information
franku committed Jun 29, 2020
1 parent 94a0525 commit 93f2db6
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 13 deletions.
42 changes: 37 additions & 5 deletions core/src/lib/cram_md5.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
BAREOS® - Backup Archiving REcovery Open Sourced
Copyright (C) 2001-2011 Free Software Foundation Europe e.V.
Copyright (C) 2013-2018 Bareos GmbH & Co. KG
Copyright (C) 2013-2020 Bareos GmbH & Co. KG
This program is Free Software; you can redistribute it and/or
modify it under the terms of version three of the GNU Affero General Public
Expand All @@ -29,6 +29,7 @@
#include "include/bareos.h"
#include "lib/cram_md5.h"
#include "lib/bsock.h"
#include "lib/util.h"


CramMd5Handshake::CramMd5Handshake(BareosSocket* bs,
Expand All @@ -39,10 +40,34 @@ CramMd5Handshake::CramMd5Handshake(BareosSocket* bs,
, password_(password)
, local_tls_policy_(local_tls_policy)
, own_qualified_name_(own_qualified_name)
, own_qualified_name_bashed_spaces_(own_qualified_name_)
{
return;
BashSpaces(own_qualified_name_bashed_spaces_);
}

CramMd5Handshake::ComparisonResult
CramMd5Handshake::CompareChallengeWithOwnQualifiedName(
const char* challenge) const
{
uint32_t a, b;
char buffer[128]{"?"}; // at least one character

bool scan_success = sscanf(challenge, "<%u.%u@%s", &a, &b, buffer) == 3;

// string contains the closing ">" of the challange
std::string challenge_qualified_name(buffer, strlen(buffer) - 1);

Dmsg1(debuglevel_, "my_name: <%s> - challenge_name: <%s>",
own_qualified_name_bashed_spaces_.c_str(),
challenge_qualified_name.c_str());

if (!scan_success) { return ComparisonResult::FAILURE; }

// check if the name in the challange matches the daemons name
return own_qualified_name_bashed_spaces_ == challenge_qualified_name
? ComparisonResult::IS_SAME
: ComparisonResult::IS_DIFFERENT;
}

/* Authorize other end
* Codes that tls_local_need and tls_remote_need can take:
Expand All @@ -60,12 +85,10 @@ bool CramMd5Handshake::CramMd5Challenge()

InitRandom();

host.check_size(MAXHOSTNAMELEN);
if (!gethostname(host.c_str(), MAXHOSTNAMELEN)) { PmStrcpy(host, my_name); }

/* Send challenge -- no hashing yet */
Mmsg(chal, "<%u.%u@%s>", (uint32_t)random(), (uint32_t)time(NULL),
host.c_str());
own_qualified_name_bashed_spaces_.c_str());

if (bs_->IsBnetDumpEnabled()) {
Dmsg2(debuglevel_, "send: auth cram-md5 %s ssl=%d qualified-name=%s\n",
Expand Down Expand Up @@ -167,6 +190,15 @@ bool CramMd5Handshake::CramMd5Response()
}
}

auto result = CompareChallengeWithOwnQualifiedName(chal.c_str());

if (result == ComparisonResult::IS_SAME ||
result == ComparisonResult::FAILURE) {
bs_->fsend(_("1999 Authorization failed.\n"));
Bmicrosleep(bs_->sleep_time_after_authentication_error, 0);
return false;
}

hmac_md5((uint8_t*)chal.c_str(), strlen(chal.c_str()), (uint8_t*)password_,
strlen(password_), hmac);
bs_->message_length =
Expand Down
13 changes: 12 additions & 1 deletion core/src/lib/cram_md5.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
BAREOS® - Backup Archiving REcovery Open Sourced
Copyright (C) 2018-2018 Bareos GmbH & Co. KG
Copyright (C) 2018-2020 Bareos GmbH & Co. KG
This program is Free Software; you can redistribute it and/or
modify it under the terms of version three of the GNU Affero General Public
Expand Down Expand Up @@ -43,9 +43,20 @@ class CramMd5Handshake {
TlsPolicy local_tls_policy_ = kBnetTlsUnknown;
TlsPolicy remote_tls_policy_ = kBnetTlsUnknown;
const std::string own_qualified_name_;
std::string own_qualified_name_bashed_spaces_;
bool CramMd5Challenge();
bool CramMd5Response();
void InitRandom() const;

enum class ComparisonResult
{
FAILURE,
IS_SAME,
IS_DIFFERENT
};

ComparisonResult CompareChallengeWithOwnQualifiedName(
const char* challenge) const;
};

void hmac_md5(uint8_t* text,
Expand Down
11 changes: 6 additions & 5 deletions core/src/lib/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "include/version_numbers.h"

#include <algorithm>
#include <string>

/*
* Various BAREOS Utility subroutines
Expand Down Expand Up @@ -136,10 +137,11 @@ void BashSpaces(char* str)
}
}

/*
* Convert spaces to non-space character.
* This makes scanf of fields containing spaces easier.
*/
void BashSpaces(std::string& str)
{
std::replace(str.begin(), str.end(), ' ', static_cast<char>(0x1));
}

void BashSpaces(PoolMem& pm)
{
char* str = pm.c_str();
Expand All @@ -149,7 +151,6 @@ void BashSpaces(PoolMem& pm)
}
}


/*
* Convert non-space characters (0x1) back into spaces
*/
Expand Down
1 change: 1 addition & 0 deletions core/src/lib/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ void EscapeString(PoolMem& snew, const char* old, int len);
bool IsBufZero(char* buf, int len);
void lcase(char* str);
void BashSpaces(char* str);
void BashSpaces(std::string& str);
void BashSpaces(PoolMem& pm);
void UnbashSpaces(char* str);
void UnbashSpaces(PoolMem& pm);
Expand Down
7 changes: 5 additions & 2 deletions core/src/tests/bsock_test_connection_setup.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
BAREOS® - Backup Archiving REcovery Open Sourced
Copyright (C) 2018-2019 Bareos GmbH & Co. KG
Copyright (C) 2018-2020 Bareos GmbH & Co. KG
This program is Free Software; you can redistribute it and/or
modify it under the terms of version three of the GNU Affero General Public
Expand Down Expand Up @@ -48,7 +48,7 @@ bool DoReloadConfig() { return false; }

static void InitSignalHandler()
{
struct sigaction sig{};
struct sigaction sig = {};
sig.sa_handler = SIG_IGN;
sigaction(SIGUSR2, &sig, nullptr);
sigaction(SIGPIPE, &sig, nullptr);
Expand Down Expand Up @@ -88,8 +88,10 @@ static PConfigParser ConsolePrepareResources(const std::string& path_to_config)
console::director_resource = dynamic_cast<console::DirectorResource*>(
console_config->GetNextRes(console::R_DIRECTOR, NULL));
EXPECT_NE(console::director_resource, nullptr);

console::console_resource = dynamic_cast<console::ConsoleResource*>(
console_config->GetNextRes(console::R_CONSOLE, NULL));
console::my_config->own_resource_ = console::console_resource;
EXPECT_EQ(console::console_resource, nullptr); // no console resource present

return console_config;
Expand All @@ -113,6 +115,7 @@ static PConfigParser DirectorPrepareResources(const std::string& path_to_config)
directordaemon::me =
(directordaemon::DirectorResource*)director_config->GetNextRes(
directordaemon::R_DIRECTOR, nullptr);
directordaemon::my_config->own_resource_ = directordaemon::me;

return director_config;
}
Expand Down

0 comments on commit 93f2db6

Please sign in to comment.