Control: tags -1 patch Please find attached the patch to address CVE-2019-18823 for version 8.6.8~dfsg.1-2, also applied in Debian 10 "Buster".
Markus
From: Markus Koschany <a...@debian.org> Date: Fri, 20 May 2022 14:08:49 +0200 Subject: CVE-2019-18823 Bug-Debian: https://bugs.debian.org/963777 Origin: https://github.com/htcondor/htcondor/commit/95eaee86e7ad3852c17df46a1b8b193dabd1fd14 Origin: https://github.com/htcondor/htcondor/commit/07e33c8b14aa00e04d045d4d79c963db082a3129 Origin: https://github.com/htcondor/htcondor/commit/cbcb93695a932d511c1c7bd40aed1eabeff01d8d Origin: https://github.com/htcondor/htcondor/commit/3916209123a8ef762b7a9cd84ca0cf8b2cd99716 Origin: https://github.com/htcondor/htcondor/commit/5c84c6f0b3db4eda1eec42c2c708069bb9393f0b --- src/condor_daemon_client/dc_schedd.cpp | 2 +- src/condor_daemon_client/dc_startd.cpp | 40 +++++++++++++++++++++++++++--- src/condor_includes/condor_commands.h | 2 ++ src/condor_schedd.V6/qmgmt.cpp | 26 +++++++++++++++++--- src/condor_schedd.V6/qmgmt.h | 3 +++ src/condor_schedd.V6/qmgmt_receivers.cpp | 14 +++-------- src/condor_schedd.V6/schedd.cpp | 6 ++--- src/condor_shadow.V6.1/shadow.cpp | 4 ++- src/condor_startd.V6/Resource.cpp | 4 +-- src/condor_startd.V6/command.cpp | 28 ++++++++++++++------- src/condor_startd.V6/command.h | 2 +- src/condor_starter.V6.1/baseStarter.cpp | 4 +-- src/condor_status.V6/prettyPrint.cpp | 2 +- src/condor_tools/history.cpp | 4 +-- src/condor_tools/transform_ads.cpp | 4 +-- src/condor_utils/compat_classad.cpp | 42 +++++++++++++++++++++++++++----- src/condor_utils/compat_classad.h | 8 +++--- src/condor_utils/file_transfer.cpp | 8 +++++- src/condor_who/who.cpp | 4 +-- 19 files changed, 153 insertions(+), 54 deletions(-) diff --git a/src/condor_daemon_client/dc_schedd.cpp b/src/condor_daemon_client/dc_schedd.cpp index fe935b0..bf291fa 100644 --- a/src/condor_daemon_client/dc_schedd.cpp +++ b/src/condor_daemon_client/dc_schedd.cpp @@ -1878,7 +1878,7 @@ bool DCSchedd::getJobConnectInfo( if( IsFulldebug(D_FULLDEBUG) ) { std::string adstr; - sPrintAd(adstr, output, true); + sPrintAd(adstr, output); dprintf(D_FULLDEBUG,"Response for GET_JOB_CONNECT_INFO:\n%s\n", adstr.c_str()); } diff --git a/src/condor_daemon_client/dc_startd.cpp b/src/condor_daemon_client/dc_startd.cpp index be0101f..9e5693c 100644 --- a/src/condor_daemon_client/dc_startd.cpp +++ b/src/condor_daemon_client/dc_startd.cpp @@ -135,6 +135,12 @@ ClaimStartdMsg::writeMsg( DCMessenger * /*messenger*/, Sock *sock ) { m_job_ad.Assign("_condor_SEND_PAIRED_SLOT", param_boolean("CLAIM_PAIRED_SLOT",true)); + // Insert an attribute in the request ad to inform the + // startd that this schedd is capable of understanding + // the newer protocol where any claim id in the response + // is encrypted. + m_job_ad.Assign("_condor_SECURE_CLAIM_ID", true); + if( !sock->put_secret( m_claim_id.c_str() ) || !putClassAd( sock, m_job_ad ) || !sock->put( scheduler_addr_to_send.c_str() ) || @@ -234,14 +240,29 @@ ClaimStartdMsg::readMsg( DCMessenger * /*messenger*/, Sock *sock ) { Reply of 4 (REQUEST_CLAIM_PAIR) means claim accepted by a slot that is paired, and the partner slot ad and claim id will be sent next. + Reply of 5 (REQUEST_CLAIM_LEFTOVERS_2) is the same as 3, but + the claim id is encrypted. + Reply of 6 (REQUEST_CLAIM_PAIR_2) is the same as 4, but + the claim id is encrypted. */ if( m_reply == OK ) { // no need to log success, because DCMsg::reportSuccess() will } else if( m_reply == NOT_OK ) { dprintf( failureDebugLevel(), "Request was NOT accepted for claim %s\n", description() ); - } else if( m_reply == REQUEST_CLAIM_LEFTOVERS ) { - if( !sock->get(m_leftover_claim_id) || + } else if( m_reply == REQUEST_CLAIM_LEFTOVERS || m_reply == REQUEST_CLAIM_LEFTOVERS_2 ) { + bool recv_ok = false; + if ( m_reply == REQUEST_CLAIM_LEFTOVERS_2 ) { + char *val = NULL; + recv_ok = sock->get_secret(val); + if ( recv_ok ) { + m_leftover_claim_id = val; + free(val); + } + } else { + recv_ok = sock->get(m_leftover_claim_id); + } + if( !recv_ok || !getClassAd( sock, m_leftover_startd_ad ) ) { // failed to read leftover partitionable slot info @@ -256,8 +277,19 @@ ClaimStartdMsg::readMsg( DCMessenger * /*messenger*/, Sock *sock ) { // change reply to OK cuz claim was a success m_reply = OK; } - } else if( m_reply == REQUEST_CLAIM_PAIR ) { - if( !sock->get(m_paired_claim_id) || + } else if( m_reply == REQUEST_CLAIM_PAIR || m_reply == REQUEST_CLAIM_PAIR_2 ) { + bool recv_ok = false; + if ( m_reply == REQUEST_CLAIM_PAIR_2 ) { + char *val = NULL; + recv_ok = sock->get_secret(val); + if ( recv_ok ) { + m_paired_claim_id = val; + free(val); + } + } else { + recv_ok = sock->get(m_paired_claim_id); + } + if( !recv_ok || !getClassAd( sock, m_paired_startd_ad ) ) { // failed to read paired slot info diff --git a/src/condor_includes/condor_commands.h b/src/condor_includes/condor_commands.h index c6b029c..5656c27 100644 --- a/src/condor_includes/condor_commands.h +++ b/src/condor_includes/condor_commands.h @@ -536,6 +536,8 @@ NAMETABLE_DIRECTIVE:END_SECTION:collector /* Replies specific to the REQUEST_CLAIM command */ #define REQUEST_CLAIM_LEFTOVERS 3 #define REQUEST_CLAIM_PAIR 4 +#define REQUEST_CLAIM_LEFTOVERS_2 5 +#define REQUEST_CLAIM_PAIR_2 6 /* Replies specific to the SWAP_CLAIM_AND_ACTIVATION command */ #define SWAP_CLAIM_ALREADY_SWAPPED 4 diff --git a/src/condor_schedd.V6/qmgmt.cpp b/src/condor_schedd.V6/qmgmt.cpp index f052531..e6dd116 100644 --- a/src/condor_schedd.V6/qmgmt.cpp +++ b/src/condor_schedd.V6/qmgmt.cpp @@ -158,7 +158,7 @@ static QmgmtPeer *Q_SOCK = NULL; // running. Used by SuperUserAllowedToSetOwnerTo(). static HashTable<MyString,int> owner_history(MyStringHash); -int do_Q_request(ReliSock *,bool &may_fork); +int do_Q_request(QmgmtPeer &,bool &may_fork); void FindPrioJob(PROC_ID &); static bool qmgmt_was_initialized = false; @@ -505,6 +505,7 @@ QmgmtPeer::QmgmtPeer() myendpoint = NULL; sock = NULL; transaction = NULL; + readonly = false; allow_protected_attr_changes_by_superuser = true; unset(); @@ -574,6 +575,16 @@ QmgmtPeer::setEffectiveOwner(char const *o) return true; } +bool +QmgmtPeer::setReadOnly(bool val) +{ + bool old_val = readonly; + + readonly = val; + + return old_val; +} + void QmgmtPeer::unset() { @@ -598,6 +609,7 @@ QmgmtPeer::unset() next_proc_num = 0; active_cluster_num = -1; xact_start_time = 0; // time at which the current transaction was started + readonly = false; } const char* @@ -1626,6 +1638,12 @@ OwnerCheck(int cluster_id,int proc_id) bool OwnerCheck(ClassAd *ad, const char *test_owner) { + if ( Q_SOCK->getReadOnly() ) { + errno = EACCES; + dprintf( D_FULLDEBUG, "OwnerCheck: reject read-only client\n" ); + return false; + } + // check if the IP address of the peer has daemon core write permission // to the schedd. we have to explicitly check here because all queue // management commands come in via one sole daemon core command which @@ -1849,7 +1867,7 @@ unsetQSock() int -handle_q(Service *, int, Stream *sock) +handle_q(Service *, int cmd, Stream *sock) { int rval; bool all_good; @@ -1868,13 +1886,15 @@ handle_q(Service *, int, Stream *sock) } ASSERT(Q_SOCK); + Q_SOCK->setReadOnly(cmd == QMGMT_READ_CMD); + BeginTransaction(); bool may_fork = false; ForkStatus fork_status = FORK_FAILED; do { /* Probably should wrap a timer around this */ - rval = do_Q_request( Q_SOCK->getReliSock(), may_fork ); + rval = do_Q_request( *Q_SOCK, may_fork ); if( may_fork && fork_status == FORK_FAILED ) { fork_status = schedd_forker.NewJob(); diff --git a/src/condor_schedd.V6/qmgmt.h b/src/condor_schedd.V6/qmgmt.h index 42e86b2..e090adb 100644 --- a/src/condor_schedd.V6/qmgmt.h +++ b/src/condor_schedd.V6/qmgmt.h @@ -55,6 +55,8 @@ class QmgmtPeer { bool setAllowProtectedAttrChanges(bool val); bool getAllowProtectedAttrChanges() { return allow_protected_attr_changes_by_superuser; } + bool setReadOnly(bool val); + bool getReadOnly() const { return readonly; } ReliSock *getReliSock() const { return sock; }; const CondorVersionInfo *get_peer_version() const { return sock->get_peer_version(); }; @@ -68,6 +70,7 @@ class QmgmtPeer { protected: char *owner; + bool readonly; bool allow_protected_attr_changes_by_superuser; char *fquser; // owner@domain char *myendpoint; diff --git a/src/condor_schedd.V6/qmgmt_receivers.cpp b/src/condor_schedd.V6/qmgmt_receivers.cpp index b71b2b6..7d9b46c 100644 --- a/src/condor_schedd.V6/qmgmt_receivers.cpp +++ b/src/condor_schedd.V6/qmgmt_receivers.cpp @@ -50,11 +50,12 @@ static bool QmgmtMayAccessAttribute( char const *attr_name ) { } int -do_Q_request(ReliSock *syscall_sock,bool &may_fork) +do_Q_request(QmgmtPeer &Q_PEER, bool &may_fork) { int request_num = -1; int rval; + ReliSock *syscall_sock = Q_PEER.getReliSock(); syscall_sock->decode(); assert( syscall_sock->code(request_num) ); @@ -97,12 +98,6 @@ do_Q_request(ReliSock *syscall_sock,bool &may_fork) { // dprintf( D_ALWAYS, "InitializeReadOnlyConnection()\n" ); - // Since InitializeConnection() does nothing, and we need - // to record the fact that this is a read-only connection, - // but we have to do it in the socket (since we don't have - // any other persistent data structure, and it's probably - // the right place anyway), set the FQU. - // // We need to record if this is a read-only connection so that // we can avoid expanding $$ in GetJobAd; simply checking if the // connection is authenticated isn't sufficient, because the @@ -110,7 +105,7 @@ do_Q_request(ReliSock *syscall_sock,bool &may_fork) // be authenticated by a previous authenticated connection from // the same address (when using host-based security) less than // the expiration period ago. - syscall_sock->setFullyQualifiedUser( "read-only" ); + Q_PEER.setReadOnly(true); // same as InitializeConnection but no authenticate() InitializeConnection( NULL, NULL ); @@ -819,8 +814,7 @@ do_Q_request(ReliSock *syscall_sock,bool &may_fork) // Only fetch the jobad for legal values of cluster/proc if( cluster_id >= 1 ) { if( proc_id >= 0 ) { - const char * fqu = syscall_sock->getFullyQualifiedUser(); - if( fqu != NULL && strcmp( fqu, "read-only" ) != 0 ) { + if( !Q_PEER.getReadOnly() ) { // expand $$() macros in the jobad as required by GridManager. // The GridManager depends on the fact that the following call // expands $$ and saves the expansions to disk in case of diff --git a/src/condor_schedd.V6/schedd.cpp b/src/condor_schedd.V6/schedd.cpp index b751ced..5a38e9e 100644 --- a/src/condor_schedd.V6/schedd.cpp +++ b/src/condor_schedd.V6/schedd.cpp @@ -2597,7 +2597,7 @@ int Scheduler::guessJobSlotWeight(JobQueueJob * job) ad->Assign(ATTR_MEMORY, 1024); ad->Assign(ATTR_DISK, 1024); dprintf(D_ALWAYS, "Creating slotWeightGuessAd as:\n"); - dPrintAd(D_ALWAYS, *ad, false); + dPrintAd(D_ALWAYS, *ad); } } @@ -9071,7 +9071,7 @@ Scheduler::spawnJobHandlerRaw( shadow_rec* srec, const char* path, // handler is now alive and can read from the pipe. ASSERT( job_ad ); MyString ad_str; - sPrintAd(ad_str, *job_ad); + sPrintAdWithSecrets(ad_str, *job_ad); const char* ptr = ad_str.Value(); int len = ad_str.Length(); while (len) { @@ -9705,7 +9705,7 @@ Scheduler::start_sched_universe_job(PROC_ID* job_id) dprintf ( D_FAILURE|D_ALWAYS, "Open of %s failed (%s, errno=%d).\n", job_ad_path.c_str(), strerror(errno), errno ); } else { // fPrindAd does not have any usable error reporting. - fPrintAd(job_ad_fp, *userJob, true); + fPrintAd(job_ad_fp, *userJob); wrote_job_ad = true; fclose(job_ad_fp); } diff --git a/src/condor_shadow.V6.1/shadow.cpp b/src/condor_shadow.V6.1/shadow.cpp index e9e4f6d..ff7b09e 100644 --- a/src/condor_shadow.V6.1/shadow.cpp +++ b/src/condor_shadow.V6.1/shadow.cpp @@ -122,7 +122,7 @@ UniShadow::init( ClassAd* job_ad, const char* schedd_addr, const char *xfer_queu // In this case we just pass the pointer along... remRes->setJobAd( jobAd ); - + // Register command which gets updates from the starter // on the job's image size, cpu usage, etc. Each kind of // shadow implements it's own version of this to deal w/ it @@ -132,6 +132,7 @@ UniShadow::init( ClassAd* job_ad, const char* schedd_addr, const char *xfer_queu (CommandHandlercpp)&UniShadow::updateFromStarter, "UniShadow::updateFromStarter", this, DAEMON ); +#if ! defined( WIN32 ) // Register command which the starter uses to fetch a user // credential if it needs to. daemonCore-> @@ -139,6 +140,7 @@ UniShadow::init( ClassAd* job_ad, const char* schedd_addr, const char *xfer_queu (CommandHandler)&get_cred_handler, "get_cred_handler", NULL, DAEMON, D_COMMAND, true /*force authentication*/ ); +#endif } void diff --git a/src/condor_startd.V6/Resource.cpp b/src/condor_startd.V6/Resource.cpp index c78c1ba..27b545b 100644 --- a/src/condor_startd.V6/Resource.cpp +++ b/src/condor_startd.V6/Resource.cpp @@ -3300,9 +3300,9 @@ Resource * initialize_resource(Resource * rip, ClassAd * req_classad, Claim* &le dprintf(D_MATCH | D_FULLDEBUG, "STARTD Requirements do not match, %s MODIFY_REQUEST_EXPR_ edits. Job ad was ============================\n", unmodified_req_classad ? "with" : "w/o"); - dPrintAd(D_MATCH | D_FULLDEBUG, *req_classad, true); + dPrintAd(D_MATCH | D_FULLDEBUG, *req_classad); dprintf(D_MATCH | D_FULLDEBUG, "Machine ad was ============================\n"); - dPrintAd(D_MATCH | D_FULLDEBUG, *mach_classad, true); + dPrintAd(D_MATCH | D_FULLDEBUG, *mach_classad); } if (unmodified_req_classad) { // our modified req_classad no longer matches, put back the original diff --git a/src/condor_startd.V6/command.cpp b/src/condor_startd.V6/command.cpp index ef5fbea..053cbbc 100644 --- a/src/condor_startd.V6/command.cpp +++ b/src/condor_startd.V6/command.cpp @@ -1217,6 +1217,7 @@ request_claim( Resource* rip, Claim *claim, char* id, Stream* stream ) char *client_addr = NULL; int interval; ClaimIdParser idp(id); + bool secure_claim_id = false; // Used in ABORT macro, yuck bool new_dynamic_slot = false; @@ -1321,6 +1322,10 @@ request_claim( Resource* rip, Claim *claim, char* id, Stream* stream ) ABORT; } + // If we include a claim id in our response, should it be + // encrypted? In the old protocol, it was sent in the clear. + req_classad->LookupBool("_condor_SECURE_CLAIM_ID", secure_claim_id); + // At this point, the schedd has registered this socket (stream) // and likely has gone off to service other requests. Thus, // we need change the timeout on the socket to the schedd to be @@ -1530,7 +1535,7 @@ request_claim( Resource* rip, Claim *claim, char* id, Stream* stream ) // function after the preemption has completed when the startd // is finally ready to reply to the and finish the claiming // process. - accept_request_claim( rip, leftover_claim, and_pair ); + accept_request_claim( rip, secure_claim_id, leftover_claim, and_pair ); // We always need to return KEEP_STREAM so that daemon core // doesn't try to delete the stream we've already deleted. @@ -1569,7 +1574,7 @@ abort_accept_claim( Resource* rip, Stream* stream ) bool -accept_request_claim( Resource* rip, Claim* leftover_claim, bool and_pair ) +accept_request_claim( Resource* rip, bool secure_claim_id, Claim* leftover_claim, bool and_pair ) { int interval = -1; char *client_addr = NULL; @@ -1593,18 +1598,22 @@ accept_request_claim( Resource* rip, Claim* leftover_claim, bool and_pair ) Reply of 4 (REQUEST_CLAIM_PAIR) means claim accepted by a slot that is paired, and the partner slot ad and claim id will be sent next. + Reply of 5 (REQUEST_CLAIM_LEFTOVERS_2) is the same as 3, but + the claim id is encrypted. + Reply of 6 (REQUEST_CLAIM_PAIR_2) is the same as 4, but + the claim id is encrypted. */ int cmd = OK; if ( leftover_claim && leftover_claim->id() && leftover_claim->rip()->r_classad ) { - // schedd wants leftovers, send reply code 3 - cmd = REQUEST_CLAIM_LEFTOVERS; + // schedd wants leftovers, send reply code 3 (or 5) + cmd = secure_claim_id ? REQUEST_CLAIM_LEFTOVERS_2 : REQUEST_CLAIM_LEFTOVERS; } else if (rip->r_pair_name) { ripb = resmgr->get_by_name(rip->r_pair_name); if (ripb && and_pair) { - cmd = REQUEST_CLAIM_PAIR; + cmd = secure_claim_id ? REQUEST_CLAIM_PAIR_2 : REQUEST_CLAIM_PAIR; } } @@ -1616,7 +1625,7 @@ accept_request_claim( Resource* rip, Claim* leftover_claim, bool and_pair ) abort_accept_claim( rip, stream ); return false; } - if ( cmd == REQUEST_CLAIM_LEFTOVERS ) + if ( cmd == REQUEST_CLAIM_LEFTOVERS || cmd == REQUEST_CLAIM_LEFTOVERS_2 ) { // schedd just claimed a dynamic slot, and it wants // us to send back to the classad and the new claim id for @@ -1625,7 +1634,7 @@ accept_request_claim( Resource* rip, Claim* leftover_claim, bool and_pair ) leftover_claim->rip()->r_classad->Assign(ATTR_LAST_SLOT_NAME, rip->r_name); MyString claimId(leftover_claim->id()); - if ( !stream->put(claimId) || + if ( !(secure_claim_id ? stream->put_secret(claimId.c_str()) : stream->put(claimId)) || !putClassAd(stream, *leftover_claim->rip()->r_classad) ) { rip->dprintf( D_ALWAYS, @@ -1634,14 +1643,15 @@ accept_request_claim( Resource* rip, Claim* leftover_claim, bool and_pair ) return false; } } - else if (cmd == REQUEST_CLAIM_PAIR) + else if (cmd == REQUEST_CLAIM_PAIR || cmd == REQUEST_CLAIM_PAIR_2) { dprintf(D_FULLDEBUG,"Sending paired slot claim to schedd\n"); //PRAGMA_REMIND("remove these next two dprintfs later") //dprintf(D_FULLDEBUG,"\tmain slot claim id is %s\n", rip->r_cur->id()); //dprintf(D_FULLDEBUG,"\tpaired slot claim id is %s\n", ripb->r_cur->id()); MyString claimId(ripb->r_cur->id()); - if ( !stream->put(claimId) || ! putClassAd(stream, *ripb->r_classad)) { + if ( !(secure_claim_id ? stream->put_secret(claimId.c_str()) : stream->put(claimId)) || + ! putClassAd(stream, *ripb->r_classad)) { rip->dprintf( D_ALWAYS, "Can't send paired slot claim & ad to schedd.\n" ); abort_accept_claim( rip, stream ); diff --git a/src/condor_startd.V6/command.h b/src/condor_startd.V6/command.h index 540c85d..02eb3f3 100644 --- a/src/condor_startd.V6/command.h +++ b/src/condor_startd.V6/command.h @@ -118,7 +118,7 @@ int match_info( Resource*, char* ); int request_claim( Resource*, Claim *, char*, Stream* ); // Accept claim from schedd agent -bool accept_request_claim( Resource* , Claim * = NULL, bool and_pair = false ); +bool accept_request_claim( Resource* , bool secure_claim_id = true, Claim * = NULL, bool and_pair = false ); // Activate a claim with a given starter int activate_claim( Resource*, Stream* ); diff --git a/src/condor_starter.V6.1/baseStarter.cpp b/src/condor_starter.V6.1/baseStarter.cpp index 0cd8062..028bea5 100644 --- a/src/condor_starter.V6.1/baseStarter.cpp +++ b/src/condor_starter.V6.1/baseStarter.cpp @@ -3693,7 +3693,7 @@ CStarter::WriteAdFiles() } else { - fPrintAd(fp, *ad, true); + fPrintAd(fp, *ad); fclose(fp); } } @@ -3718,7 +3718,7 @@ CStarter::WriteAdFiles() } else { - fPrintAd(fp, *ad, true); + fPrintAd(fp, *ad); fclose(fp); } } diff --git a/src/condor_status.V6/prettyPrint.cpp b/src/condor_status.V6/prettyPrint.cpp index 9489492..c9cc29c 100644 --- a/src/condor_status.V6/prettyPrint.cpp +++ b/src/condor_status.V6/prettyPrint.cpp @@ -477,7 +477,7 @@ void prettyPrintAd(ppOption pps, ClassAd *ad, int output_index, StringList * whi classad::References attrs; classad::References *proj = NULL; if (PP_IS_LONGish(pps) && ( ! fHashOrder || whitelist)) { - sGetAdAttrs(attrs, *ad, false, whitelist); + sGetAdAttrs(attrs, *ad, true, whitelist); proj = &attrs; } diff --git a/src/condor_tools/history.cpp b/src/condor_tools/history.cpp index f841f34..75cc574 100644 --- a/src/condor_tools/history.cpp +++ b/src/condor_tools/history.cpp @@ -743,7 +743,7 @@ main(int argc, char* argv[]) ad.InsertAttr(ATTR_OWNER, 1); ad.InsertAttr("StreamResults", true); dprintf(D_FULLDEBUG, "condor_history: sending streaming ACK header ad:\n"); - dPrintAd(D_FULLDEBUG, ad, false); + dPrintAd(D_FULLDEBUG, ad); if ( ! putClassAd(socks[0], ad) || ! socks[0]->end_of_message()) { dprintf(D_ALWAYS, "condor_history: Failed to write streaming ACK header ad\n"); exit(1); @@ -768,7 +768,7 @@ main(int argc, char* argv[]) ad.InsertAttr("MalformedAds", writetosocket_failcount); ad.InsertAttr("AdCount", adCount); dprintf(D_FULLDEBUG, "condor_history: sending final ad:\n"); - dPrintAd(D_FULLDEBUG, ad, false); + dPrintAd(D_FULLDEBUG, ad); if ( ! putClassAd(socks[0], ad) || ! socks[0]->end_of_message()) { dprintf(D_ALWAYS, "condor_history: Failed to write final ad to client\n"); exit(1); diff --git a/src/condor_tools/transform_ads.cpp b/src/condor_tools/transform_ads.cpp index 5224e9c..5f9160f 100644 --- a/src/condor_tools/transform_ads.cpp +++ b/src/condor_tools/transform_ads.cpp @@ -1740,8 +1740,6 @@ bool ReportSuccess(const ClassAd * job, apply_transform_args & xform_args) if ( ! job) return false; if (testing.no_output) return true; - StringList * whitelist = NULL; - // if we have not yet picked and output format, do that now. if (DashOutFormat == ClassAdFileParseType::Parse_auto) { if (xform_args.input_helper) { @@ -1759,7 +1757,7 @@ bool ReportSuccess(const ClassAd * job, apply_transform_args & xform_args) classad::References attrs; classad::References *print_order = NULL; if ( ! DashOutAttrsInHashOrder) { - sGetAdAttrs(attrs, *job, false, whitelist); + sGetAdAttrs(attrs, *job); print_order = &attrs; } switch (DashOutFormat) { diff --git a/src/condor_utils/compat_classad.cpp b/src/condor_utils/compat_classad.cpp index 3fed02e..ca9fafa 100644 --- a/src/condor_utils/compat_classad.cpp +++ b/src/condor_utils/compat_classad.cpp @@ -1731,7 +1731,7 @@ int CondorClassAdListWriter::appendAd(const ClassAd & ad, std::string & output, classad::References attrs; classad::References *print_order = NULL; if ( ! hash_order || whitelist) { - sGetAdAttrs(attrs, ad, false, whitelist); + sGetAdAttrs(attrs, ad, true, whitelist); print_order = &attrs; } @@ -2574,7 +2574,12 @@ fPrintAd( FILE *file, const classad::ClassAd &ad, bool exclude_private, StringLi { MyString buffer; - sPrintAd( buffer, ad, exclude_private, attr_white_list ); + if( exclude_private ) { + sPrintAd( buffer, ad, attr_white_list ); + } else { + sPrintAdWithSecrets( buffer, ad, attr_white_list ); + } + if ( fprintf(file, "%s", buffer.Value()) < 0 ) { return FALSE; } else { @@ -2588,14 +2593,19 @@ dPrintAd( int level, const classad::ClassAd &ad, bool exclude_private ) if ( IsDebugCatAndVerbosity( level ) ) { MyString buffer; - sPrintAd( buffer, ad, exclude_private ); + if( exclude_private ) { + sPrintAd( buffer, ad ); + } else { + sPrintAdWithSecrets( buffer, ad ); + } dprintf( level|D_NOHEADER, "%s", buffer.Value() ); } } + int -sPrintAd( MyString &output, const classad::ClassAd &ad, bool exclude_private, StringList *attr_white_list ) +_sPrintAd( MyString &output, const classad::ClassAd &ad, bool exclude_private, StringList *attr_white_list ) { classad::ClassAd::const_iterator itr; @@ -2640,10 +2650,30 @@ sPrintAd( MyString &output, const classad::ClassAd &ad, bool exclude_private, St } int -sPrintAd( std::string &output, const classad::ClassAd &ad, bool exclude_private, StringList *attr_white_list ) +sPrintAd( MyString &output, const classad::ClassAd &ad, StringList *attr_white_list ) { + return _sPrintAd( output, ad, true, attr_white_list ); +} + +int +sPrintAdWithSecrets( MyString &output, const classad::ClassAd &ad, StringList *attr_white_list ) { + return _sPrintAd( output, ad, false, attr_white_list ); +} + + +int +sPrintAd( std::string &output, const classad::ClassAd &ad, StringList *attr_white_list ) +{ + MyString myout; + int rc = sPrintAd( myout, ad, attr_white_list ); + output += myout; + return rc; +} + +int +sPrintAdWithSecrets( std::string &output, const classad::ClassAd &ad, StringList *attr_white_list ) { MyString myout; - int rc = sPrintAd( myout, ad, exclude_private, attr_white_list ); + int rc = sPrintAdWithSecrets( myout, ad, attr_white_list ); output += myout; return rc; } diff --git a/src/condor_utils/compat_classad.h b/src/condor_utils/compat_classad.h index 15afd8a..33d41e5 100644 --- a/src/condor_utils/compat_classad.h +++ b/src/condor_utils/compat_classad.h @@ -58,7 +58,7 @@ typedef std::set<std::string, classad::CaseIgnLTStr> AttrNameSet; @param file The file handle to print to. @return TRUE */ -int fPrintAd(FILE *file, const classad::ClassAd &ad, bool exclude_private = false, StringList *attr_white_list = NULL); +int fPrintAd(FILE *file, const classad::ClassAd &ad, bool exclude_private = true, StringList *attr_white_list = NULL); /** Print the ClassAd as an old ClasAd with dprintf @param level The dprintf level. @@ -69,13 +69,15 @@ void dPrintAd( int level, const classad::ClassAd &ad, bool exclude_private = tru @param output The MyString to write into @return TRUE */ -int sPrintAd( MyString &output, const classad::ClassAd &ad, bool exclude_private = false, StringList *attr_white_list = NULL ); +int sPrintAd( MyString &output, const classad::ClassAd &ad, StringList *attr_white_list = NULL ); +int sPrintAdWithSecrets( MyString &output, const classad::ClassAd &ad, StringList *attr_white_list = NULL ); /** Format the ClassAd as an old ClassAd into the std::string. @param output The std::string to write into @return TRUE */ -int sPrintAd( std::string &output, const classad::ClassAd &ad, bool exclude_private = false, StringList *attr_white_list = NULL ); +int sPrintAd( std::string &output, const classad::ClassAd &ad, StringList *attr_white_list = NULL ); +int sPrintAdWithSecrets( std::string &output, const classad::ClassAd & ad, StringList *attr_white_list = NULL ); /** Get a sorted list of attributes that are in the given ad, and also match the given whitelist (if any) @param attrs the set of attrs to insert into. This is set is NOT cleared first. diff --git a/src/condor_utils/file_transfer.cpp b/src/condor_utils/file_transfer.cpp index 3be2e61..6aab8c3 100644 --- a/src/condor_utils/file_transfer.cpp +++ b/src/condor_utils/file_transfer.cpp @@ -3218,7 +3218,13 @@ FileTransfer::DoUpload(filesize_t *total_bytes, ReliSock *s) return_and_resetpriv( -1 ); } - // compute the size of what we sent + // + // This comment used to read 'compute the size of what we sent', + // but obviously the wire format and the string format of + // ClassAds are not the same and can't be expected to be the + // same length. Since the size will be wrong anyway, simplify + // future security audits but not printing the private attrs. + // MyString junkbuf; sPrintAd(junkbuf, file_info); bytes = junkbuf.Length(); diff --git a/src/condor_who/who.cpp b/src/condor_who/who.cpp index 7be0d66..19f2a04 100644 --- a/src/condor_who/who.cpp +++ b/src/condor_who/who.cpp @@ -1464,7 +1464,7 @@ main( int argc, char *argv[] ) std::string tmp; if (App.print_mask.IsEmpty()) { classad::References attrs; - sGetAdAttrs(attrs, ready_ad, false, NULL); + sGetAdAttrs(attrs, ready_ad); sPrintAdAttrs(tmp, ready_ad, attrs); } else { if (App.print_mask.has_headings()) { @@ -1692,7 +1692,7 @@ main( int argc, char *argv[] ) while (ClassAd *ad = result.Next()) { tmp.clear(); classad::References attrs; - sGetAdAttrs(attrs, *ad, false, NULL); + sGetAdAttrs(attrs, *ad); sPrintAdAttrs(tmp, *ad, attrs); tmp += "\n"; fputs(tmp.c_str(), stdout);
signature.asc
Description: This is a digitally signed message part
