diff --git a/doc/autoinclude/FeatureTest.doc b/doc/autoinclude/FeatureTest.doc index 61987923b7..c325581ab6 100644 --- a/doc/autoinclude/FeatureTest.doc +++ b/doc/autoinclude/FeatureTest.doc @@ -79,6 +79,8 @@ Packages requiring a feature may use the corresponding \c Requires: in their .sp
Also support user defined repo variables in /etc/zypp/vars.d.
version 1.1
Support repo variables in an URIs \c host and \c port component.
+
version 1.2
+
Support repo variables forming valid URIs after expansion - but not before.
diff --git a/doc/autoinclude/RepoVariables.doc b/doc/autoinclude/RepoVariables.doc index 89c6d71630..f44b9209c6 100644 --- a/doc/autoinclude/RepoVariables.doc +++ b/doc/autoinclude/RepoVariables.doc @@ -15,7 +15,7 @@ The variable expander also supports shell like definition of default and alterna \see \ref zypp::repo::RepoVarExpand Variable expander -\subsection zypp-repoars-builtin Builtin repository variables +\subsection zypp-repovars-builtin Builtin repository variables \li \c $arch - The system's CPU architecture. @@ -31,10 +31,11 @@ The variable expander also supports shell like definition of default and alterna \c $releasever_major will be set to the leading portion up to (but not including) the 1st dot; \c $releasever_minor to the trailing portion after the 1st dot. If there's no dot in \c $releasever, \c $releasever_major is the same as \c $releasever and \c $releasever_minor is empty. -\subsection zypp-repoars-userdefined User defined repository variables [requires 'libzypp(repovarexpand) >= 1'] +\subsection zypp-repovars-userdefined User defined repository variables [requires 'libzypp(repovarexpand) >= 1'] A custom repository variable is defined by creating a file in \c /etc/zypp/vars.d. The variable name equals the file name. The files first line (up to but not including the newline character) defines the variables value. Valid variable(file) names consist of alphanumeric chars and '_' only. Variable substitution within an URIs authority [requires 'libzypp(repovarexpand) >= 1.1'] is limited to \c host and \c port. Bash style definition of default and alternate values is not supported. No variables can be used in an URIs \c scheme, \c user and \c password. +The use of arbitrary strings which form a valid url after variable substitution - but not before - requires 'libzypp(repovarexpand) >= 1.2'. The \c zypp-rawurl: schema is used to wrap the unexpanded strings into a valid \ref Url until they get expanded (\see \ref zypp::RawUrl). */ diff --git a/libzypp.spec.cmake b/libzypp.spec.cmake index 5d5d98be33..9dd0448cc8 100644 --- a/libzypp.spec.cmake +++ b/libzypp.spec.cmake @@ -62,7 +62,7 @@ Provides: libzypp(plugin:services) = 1 Provides: libzypp(plugin:system) = 1 Provides: libzypp(plugin:urlresolver) = 0 Provides: libzypp(plugin:repoverification) = 0 -Provides: libzypp(repovarexpand) = 1.1 +Provides: libzypp(repovarexpand) = 1.2 %if 0%{?suse_version} Recommends: logrotate diff --git a/tests/parser/RepoFileReader_test.cc b/tests/parser/RepoFileReader_test.cc index 6b861d0c8e..1192234b2e 100644 --- a/tests/parser/RepoFileReader_test.cc +++ b/tests/parser/RepoFileReader_test.cc @@ -9,6 +9,23 @@ using std::stringstream; using std::string; using namespace zypp; +// This is kind of ugly for testing: Whenever a zypp lock is +// acquired the repo variables are cleared and re-read from disk, +// which is ok (by ZYppFactory). +// The VarReplacer itself however acquires a lock to resolve the +// Target variables (like $releasever), which is ok as well. +// For testing however - if we want to manipulate the variable +// definitions - we must hold a lock ourselves. Otherwise the +// VarReplacer will acquired one and so discard our custom +// settings. +#include +auto guard { getZYpp() }; +namespace zyppintern { + std::map repoVariablesGet(); + void repoVariablesSwap( std::map & val_r ); +} +// --- + static std::string suse_repo = "[factory-oss]\n" "name=factory-oss $releasever - $basearch\n" "failovermethod=priority\n" @@ -56,3 +73,78 @@ BOOST_AUTO_TEST_CASE(read_repo_file) BOOST_CHECK_EQUAL( Url("http://serv.er/loc1"), repo.mirrorListUrl() ); } } + +BOOST_AUTO_TEST_CASE(rawurl2repoinfo) +{ + // Set up repo variables... + std::map vardef { ::zyppintern::repoVariablesGet() }; + vardef["releasever"] = "myversion"; + vardef["basearch"] = "myarch"; + vardef["OPENSUSE_DISTURL"] = "https://cdn.opensuse.org/repositories/"; + ::zyppintern::repoVariablesSwap( vardef ); + + { + std::stringstream input( + "[leap-repo]\n" + "name=leap-repo $releasever - $basearch\n" + "baseurl= ${OPENSUSE_DISTURL}leap/repo\n" + "gpgkey= ${OPENSUSE_DISTURL}leap/repo\n" + "mirrorlist= ${OPENSUSE_DISTURL}leap/repo\n" + "metalink= ${OPENSUSE_DISTURL}leap/repo\n" + ); + RepoCollector collector; + parser::RepoFileReader parser( input, bind( &RepoCollector::collect, &collector, _1 ) ); + BOOST_CHECK_EQUAL(1, collector.repos.size()); + + const RepoInfo & repo( collector.repos.front() ); + BOOST_CHECK_EQUAL( repo.name(), "leap-repo myversion - myarch" ); + + BOOST_CHECK_EQUAL( repo.url().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" ); + BOOST_CHECK_EQUAL( repo.gpgKeyUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" ); + BOOST_CHECK_EQUAL( repo.mirrorListUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" ); + + BOOST_CHECK_EQUAL( repo.rawUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" ); + BOOST_CHECK_EQUAL( repo.rawGpgKeyUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" ); + BOOST_CHECK_EQUAL( repo.rawMirrorListUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" ); + } + + // RepoFileReader unfortunately encodes "proxy=" into the URLs + // query part (by now just for the baseurl). For RawUrls the + // VarReplacer needs to take this into account. + { + std::stringstream input( + "[leap-repo]\n" + "name=leap-repo $releasever - $basearch\n" + "proxy=myproxy.host:1234\n" + "baseurl=${OPENSUSE_DISTURL}leap/repo\n" + "baseurl=https://cdn.opensuse.org/repositories/leap/repo\n" + "baseurl=https://cdn.opensuse.org/repositories/leap/repo?proxy=otherproxy.host\n" + "gpgkey=${OPENSUSE_DISTURL}leap/repo\n" + "mirrorlist=${OPENSUSE_DISTURL}leap/repo\n" + "metalink=${OPENSUSE_DISTURL}leap/repo\n" + ); + RepoCollector collector; + parser::RepoFileReader parser( input, bind( &RepoCollector::collect, &collector, _1 ) ); + BOOST_CHECK_EQUAL(1, collector.repos.size()); + + const RepoInfo & repo( collector.repos.front() ); + BOOST_CHECK_EQUAL( repo.name(), "leap-repo myversion - myarch" ); + + BOOST_CHECK_EQUAL( repo.baseUrlsSize(), 3 ); + const auto & baseurls = repo.baseUrls(); + auto iter = baseurls.begin(); + BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=myproxy.host&proxyport=1234" ); + BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=myproxy.host&proxyport=1234" ); + BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=otherproxy.host" ); + BOOST_CHECK_EQUAL( repo.gpgKeyUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" ); + BOOST_CHECK_EQUAL( repo.mirrorListUrl().asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo" ); + + const auto & rawbaseurls = repo.rawBaseUrls(); + iter = rawbaseurls.begin(); + BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "zypp-rawurl:?proxy=myproxy.host&proxyport=1234#${OPENSUSE_DISTURL}leap/repo" ); + BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=myproxy.host&proxyport=1234" ); + BOOST_CHECK_EQUAL( (iter++)->asCompleteString(), "https://cdn.opensuse.org/repositories/leap/repo?proxy=otherproxy.host" ); + BOOST_CHECK_EQUAL( repo.rawGpgKeyUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" ); + BOOST_CHECK_EQUAL( repo.rawMirrorListUrl().asCompleteString(), "zypp-rawurl:#${OPENSUSE_DISTURL}leap/repo" ); + } +} diff --git a/zypp/RepoInfo.cc b/zypp/RepoInfo.cc index d32e8b55f7..7cd3bbf9e4 100644 --- a/zypp/RepoInfo.cc +++ b/zypp/RepoInfo.cc @@ -884,10 +884,12 @@ namespace zypp std::ostream & RepoInfo::dumpOn( std::ostream & str ) const { + // Don't dump raw Urls while the fragment is hidden in the default view + // (because it may otherwise expose embedded passwords). RepoInfoBase::dumpOn(str); if ( _pimpl->baseurl2dump() ) { - for ( const auto & url : _pimpl->baseUrls().raw() ) + for ( const auto & url : _pimpl->baseUrls().transformed() ) // not .raw()! { str << "- url : " << url << std::endl; } @@ -899,7 +901,7 @@ namespace zypp str << tag_r << value_r << std::endl; }); - strif( (_pimpl->_mirrorListForceMetalink ? "- metalink : " : "- mirrorlist : "), rawMirrorListUrl().asString() ); + strif( (_pimpl->_mirrorListForceMetalink ? "- metalink : " : "- mirrorlist : "), mirrorListUrl().asString() ); // not rawMirrorListUrl() strif( "- path : ", path().asString() ); str << "- type : " << type() << std::endl; str << "- priority : " << priority() << std::endl; @@ -913,7 +915,7 @@ namespace zypp << std::endl; #undef OUTS - for ( const auto & url : _pimpl->gpgKeyUrls().raw() ) + for ( const auto & url : _pimpl->gpgKeyUrls().transformed() ) // not .raw()! { str << "- gpgkey : " << url << std::endl; } @@ -932,6 +934,8 @@ namespace zypp std::ostream & RepoInfo::dumpAsIniOn( std::ostream & str ) const { + // NOTE: hotfix1050625::asString provides the Url asRawSring, + // which is what we want when writing a .repo file. RepoInfoBase::dumpAsIniOn(str); if ( _pimpl->baseurl2dump() ) @@ -970,7 +974,7 @@ namespace zypp std::string indent( "gpgkey="); for ( const auto & url : _pimpl->gpgKeyUrls().raw() ) { - str << indent << url << endl; + str << indent << hotfix1050625::asString( url ) << endl; if ( indent[0] != ' ' ) indent = " "; } diff --git a/zypp/RepoInfo.h b/zypp/RepoInfo.h index 003034ed59..e6833eb80c 100644 --- a/zypp/RepoInfo.h +++ b/zypp/RepoInfo.h @@ -67,6 +67,15 @@ namespace zypp * * \note Name, baseUrls and mirrorUrl are subject to repo variable replacement * (\see \ref RepoVariablesStringReplacer). + * + * \note Methods returning RawUrls are deprecated because printing them may + * accidentally expose credentials (password,proxypassword) embedded in the + * raw url-string. An url-string with embedded RepoVariables, forms a valid + * \ref Url after variables have been expanded. In the expanded Url passwords + * are protected. The unexpanded url-string however does not even need to form + * a valid Url, so sensitive data in the string can not be hidden. This is why + * the zypp-rawurl: schema, which is used to store the unexpanded url-strings, + * per default prints just it's schema, but not it's content. */ class RepoInfo : public repo::RepoInfoBase { @@ -132,8 +141,9 @@ namespace zypp { return( baseUrlsEmpty() ? Url() : *baseUrlsBegin()); } /** * Pars pro toto: The first repository raw url (no variables replaced) + * \deprecated \see \ref RepoInfo */ - Url rawUrl() const; + Url rawUrl() const ZYPP_DEPRECATED; /** * The complete set of repository urls @@ -144,8 +154,9 @@ namespace zypp url_set baseUrls() const; /** * The complete set of raw repository urls (no variables replaced) + * \deprecated \see \ref RepoInfo */ - url_set rawBaseUrls() const; + url_set rawBaseUrls() const ZYPP_DEPRECATED; /** * Add a base url. \see baseUrls @@ -197,8 +208,9 @@ namespace zypp Url mirrorListUrl() const; /** * The raw mirrorListUrl (no variables replaced). + * \deprecated \see \ref RepoInfo */ - Url rawMirrorListUrl() const; + Url rawMirrorListUrl() const ZYPP_DEPRECATED; /** * Set mirror list url. \see mirrorListUrl * \param url The base url for the list @@ -391,15 +403,19 @@ namespace zypp /** The list of gpgkey URLs defined for this repo */ url_set gpgKeyUrls() const; - /** The list of raw gpgkey URLs defined for this repo (no variables replaced) */ - url_set rawGpgKeyUrls() const; + /** The list of raw gpgkey URLs defined for this repo (no variables replaced) + * \deprecated \see \ref RepoInfo + */ + url_set rawGpgKeyUrls() const ZYPP_DEPRECATED; /** Set a list of gpgkey URLs defined for this repo */ void setGpgKeyUrls( url_set urls ); /** (leagcy API) The 1st gpgkey URL defined for this repo */ Url gpgKeyUrl() const; - /** (leagcy API) The 1st raw gpgkey URL defined for this repo (no variables replaced) */ - Url rawGpgKeyUrl() const; + /** (leagcy API) The 1st raw gpgkey URL defined for this repo (no variables replaced) + * \deprecated \see \ref RepoInfo + */ + Url rawGpgKeyUrl() const ZYPP_DEPRECATED; /** (leagcy API) Set the gpgkey URL defined for this repo */ void setGpgKeyUrl( const Url &gpgkey ); diff --git a/zypp/parser/RepoFileReader.cc b/zypp/parser/RepoFileReader.cc index 04d2b56665..f5b46d25a5 100644 --- a/zypp/parser/RepoFileReader.cc +++ b/zypp/parser/RepoFileReader.cc @@ -115,7 +115,7 @@ namespace zypp // #285: Fedora/dnf allows WS separated urls (and an optional comma) strv::splitRx( line_r, "[,[:blank:]]*[[:blank:]][,[:blank:]]*", [&store_r]( std::string_view w ) { if ( ! w.empty() ) - store_r.push_back( Url(std::string(w)) ); + store_r.push_back( RawUrl(std::string(w)) ); // RawUrl! to support repo var replacement }); } diff --git a/zypp/repo/RepoVariables.cc b/zypp/repo/RepoVariables.cc index 1221d6c10f..efaff29271 100644 --- a/zypp/repo/RepoVariables.cc +++ b/zypp/repo/RepoVariables.cc @@ -567,6 +567,16 @@ namespace zypp if ( value.schemeIsRawUrl() ) { const std::string & replaced { RepoVarExpand()( value.getFragment(), RepoVarsMap::lookup ) }; newurl = Url( replaced ); + // Fixup some legacy issue: RepoFileReader encodes a 'proxy=' in .repo in + // the Url's Query part. We must restore it from there unless the original Url + // defined proxy in it's query part. + if ( newurl.getQueryParam( "proxy" ).empty() ) { + const std::string & p { value.getQueryParam( "proxy" ) }; + if ( not p.empty() ) { + newurl.setQueryParam( "proxy", p ); + newurl.setQueryParam( "proxyport", value.getQueryParam( "proxyport" ) ); + } + } } else { // The traditional Url::ViewOptions toReplace = value.getViewOptions() - url::ViewOption::WITH_USERNAME - url::ViewOption::WITH_PASSWORD;