security(config): reject publish_url credentials before they are published

Changelog.txt

@@ -77,6 +77,28 @@ Newest first. Dates are ISO 8601 (YYYY-MM-DD).
     entry (the official schema pins only the first two and allows extras); the
     context is pinned in the fail-closed allowlist and covered by the drift
     watchdog (#239).
+  - security(config): reject a `[issuer]` / `[badge_*]` URL that embeds
+    userinfo (`https://user:password@host/`) when the config is loaded. Those
+    URLs are public identifiers: `publish_url` is printed by every publish
+    command, `urljoin`ed into the ids written to the publicly served
+    organization.json / badge.json / key.json (urljoin preserves the
+    `user:pass@`), turned into the issuer's did:web, and used to build the
+    `credentialStatus` list URLs signed into every OB3 credential — so a
+    password there leaked into user-visible output, files served on the open
+    web and the badges delivered to recipients alike. Only one narrow path
+    guarded against it (did:web derivation, reached solely by
+    `[issuer] did = auto`), and that guard echoed the password back in its own
+    error for a non-https URL. The check now lives once in
+    `ConfParser.read_conf()` — after `${...}` interpolation, so a reference
+    cannot smuggle a credential past it — and every consumer (OB1/OB2/OB3
+    publishing, DID derivation, status lists) inherits it; the ConfigError
+    names the offending section and key but never the value. `did_web_from_url`
+    keeps its own guard as defence in depth, with the userinfo check moved
+    ahead of the scheme and host checks and their messages redacted. Behaviour
+    change: a config that protected a staging host with credentials in a
+    published URL no longer loads — drop the userinfo and use an allow-list or
+    a token the library never sees. `[smtp] username` / `password` are
+    untouched; they never leave the issuer's machine (#252).
   - security(eudi): document and pin the X.509 / eIDAS trust boundary for
     SD-JWT VC verification. All x5c chain validation is delegated to openvc-core
     (path building, temporal validity, SAN<->iss binding); openbadgeslib only

openbadgeslib/config.ini.example

@@ -27,6 +27,10 @@ mail_from = no-reply@issuer.badge
 ;password =
 
 ; Configuration of the OpenBadges issuer.
+; Every URL here and in the badge sections is public: it is printed by the
+; tools, published in the badge metadata and embedded in signed credentials.
+; A URL that embeds credentials (https://user:password@host/) is rejected when
+; the config is loaded, so the password cannot leak into any of them.
 [issuer]
 name           = OpenBadge issuer
 url            = https://www.issuer.badge

openbadgeslib/confparser.py

@@ -282,6 +282,53 @@ def badge_section_config(conf: ConfigParser,
     )
 
 
+#: Sections whose values are the *public* identifiers of a deployment, checked
+#: by :func:`reject_url_userinfo`. [smtp] is deliberately excluded: its
+#: username/password are credentials that stay on the issuer's machine.
+_PUBLIC_URL_SECTIONS = ('issuer',)
+_PUBLIC_URL_SECTION_PREFIX = 'badge_'
+
+
+def reject_url_userinfo(parser: ConfigParser, config_file: str) -> None:
+    """Reject any [issuer]/[badge_*] value that is a URL carrying userinfo.
+
+    Those sections hold the identifiers the tools publish: ``publish_url`` is
+    printed by every publish command, joined into the ids written to
+    organization.json / badge.json / key.json (``urljoin`` keeps the
+    ``user:pass@``), turned into the issuer's did:web, and used to build the
+    ``credentialStatus`` list URLs embedded in signed OB3 credentials. A
+    ``https://user:password@host/`` value would therefore carry the credential
+    into user-visible output, publicly served files and the credentials
+    delivered to recipients alike — so it is refused once here, at load time,
+    and every consumer (OB1/OB2/OB3, DID derivation, status lists) inherits the
+    guarantee instead of re-checking it.
+
+    The message names the offending section and key but never echoes the value:
+    printing it would leak the very password the check exists to protect.
+    """
+    from urllib.parse import urlsplit
+    for section in parser.sections():
+        if (section not in _PUBLIC_URL_SECTIONS
+                and not section.startswith(_PUBLIC_URL_SECTION_PREFIX)):
+            continue
+        for key, value in parser.items(section):
+            try:
+                authority = urlsplit(value).netloc
+            except ValueError:
+                # Too malformed for urlsplit (an unbalanced IPv6 bracket).
+                # Read the authority textually rather than skip the value:
+                # it is still written out verbatim downstream.
+                authority = value.partition('//')[2].split('/', 1)[0]
+            if '@' in authority:
+                raise ConfigError(
+                    "Configuration file %s: [%s] %s must not carry userinfo "
+                    "(user:password@host) in its URL. That URL is printed by "
+                    "the tools, published in badge metadata and embedded in "
+                    "signed credentials, so the credential would leak; drop "
+                    "the userinfo and protect the host another way."
+                    % (config_file, section, key))
+
+
 class ConfParser():
     def __init__(self, config_file: str = 'config.ini') -> None:
         self.config_file = config_file
@@ -343,6 +390,10 @@ def read_conf(self) -> Optional[ConfigParser]:
             raise ConfigError(
                 "Configuration file %s has an invalid value: %s" % (self.config_file, exc)) from exc
 
+        # Every value is resolved by now, so a ${...} reference cannot smuggle
+        # a credential past this check.
+        reject_url_userinfo(self.parser, self.config_file)
+
         return self.parser
 
 

openbadgeslib/ob3/did.py

@@ -288,12 +288,18 @@ def did_web_from_url(url: str) -> str:
     """
     from urllib.parse import quote, urlsplit
     parts = urlsplit(url)
-    if parts.scheme != 'https':
-        raise ValueError('did:web requires an https URL, got %r' % (url,))
-    if parts.username or parts.password:
+    # Userinfo first: the two checks below quote the URL in their messages, and
+    # a CLI prints those to stdout (and into the 'error' field of --json), so
+    # 'http://user:password@host/' used to publish its own password on the way
+    # out. What they quote is redacted too, so the guarantee no longer depends
+    # on this ordering surviving a future edit.
+    if '@' in parts.netloc:
         raise ValueError('did:web URL must not contain userinfo (user:pass@)')
+    safe = parts._replace(netloc=parts.netloc.rpartition('@')[2]).geturl()
+    if parts.scheme != 'https':
+        raise ValueError('did:web requires an https URL, got %r' % (safe,))
     if not parts.hostname:
-        raise ValueError('URL %r has no host' % (url,))
+        raise ValueError('URL %r has no host' % (safe,))
     authority = parts.hostname
     if parts.port is not None:
         authority += ':%d' % parts.port

tests/test_cli_smoke.py

@@ -508,6 +508,42 @@ def test_publish_ob2_missing_badge_key_exits_cleanly(tmp_path):
     assert 'missing required config key' in str(exc.value)
 
 
+def test_publish_never_leaks_publish_url_userinfo(tmp_path, capsys):
+    # End-to-end guarantee behind the confparser check: with a
+    # 'https://user:password@host/' publish_url, no -V publishes anything and
+    # no byte of the password reaches stdout/stderr or the output tree. Before
+    # the check, publish printed the URL on success and urljoin'd it into the
+    # ids of organization.json / badge.json / key.json.
+    import pytest
+    from pathlib import Path
+    from openbadgeslib import openbadges_publish
+    tests_dir = Path(__file__).parent
+    password = 'hunter2-zzz'
+    cfg = tmp_path / 'cfg.ini'
+    cfg.write_text('\n'.join([
+        '[paths]', 'base = %s' % tmp_path,
+        '[issuer]', 'name = I', 'url = https://example.com',
+        'publish_url = https://user:%s@example.com/issuer/' % password,
+        'revocationList = revoked.json', 'image = logo.png', 'email = i@x.example',
+        '[badge_1]', 'name = B', 'description = d', 'image = badge.svg',
+        'criteria = https://example.com/c', 'status_lists = revocation',
+        'public_key = %s' % (tests_dir / 'test_verify_rsa.pem'),
+        'private_key = %s' % (tests_dir / 'test_sign_rsa.pem'),
+    ]) + '\n')
+    for version in ('1', '2', '3'):
+        out = tmp_path / ('out%s' % version)
+        argv = ['openbadges-publish', '-c', str(cfg), '-o', str(out),
+                '-V', version]
+        with patch.object(sys, 'argv', argv):
+            with pytest.raises(SystemExit) as exc:
+                openbadges_publish.main()
+        assert exc.value.code == 1
+        captured = capsys.readouterr()
+        assert password not in captured.out
+        assert password not in captured.err
+        assert not out.exists()
+
+
 def test_publish_existing_output_exits_cleanly(tmp_path):
     # -o pointing at an existing path must exit cleanly (SystemExit) with an
     # operator-facing message, not a raw FileExistsError.

tests/test_confparser.py

@@ -87,6 +87,136 @@ def test_config_dir_with_dollar_sign_round_trips(self, tmp_path):
     def test_nonexistent_file_returns_none(self, tmp_path):
         assert ConfParser(str(tmp_path / 'missing.ini')).read_conf() is None
 
+
+#: A password distinctive enough that its absence from an error message is
+#: meaningful (no substring of the config, the paths or the boilerplate).
+_PW = 'hunter2-zzz'
+
+
+class TestUrlUserinfoRejected:
+    """A publish_url (or any other [issuer]/[badge_*] URL) carrying
+    ``user:password@`` is refused at load time: that URL is printed by the
+    CLIs, joined into the ids written to organization.json / badge.json /
+    key.json, turned into the issuer's did:web and embedded in the
+    credentialStatus of signed OB3 credentials — so accepting it would leak the
+    credential into user-visible output, public files and issued badges alike.
+    No message may echo the password."""
+
+    def test_publish_url_with_userinfo_rejected(self, tmp_path):
+        from openbadgeslib.errors import ConfigError
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[issuer]\nname = x\n'
+            'publish_url = https://user:%s@issuer.example/issuer/\n' % _PW)
+        with pytest.raises(ConfigError) as exc:
+            ConfParser(path).read_conf()
+        assert '[issuer] publish_url' in str(exc.value)
+        assert _PW not in str(exc.value)
+
+    def test_password_absent_from_repr_too(self, tmp_path):
+        # repr() is what a traceback shows; neither it nor str() may carry it.
+        from openbadgeslib.errors import ConfigError
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[issuer]\nname = x\n'
+            'publish_url = https://user:%s@issuer.example/\n' % _PW)
+        with pytest.raises(ConfigError) as exc:
+            ConfParser(path).read_conf()
+        assert _PW not in repr(exc.value)
+
+    @pytest.mark.parametrize('url', [
+        'https://user:%s@issuer.example/' % _PW,   # user:password@
+        'https://user@issuer.example/',            # bare user@
+        'https://@issuer.example/',                # empty userinfo
+        'http://user:%s@issuer.example/' % _PW,    # any scheme, not just https
+    ])
+    def test_every_userinfo_shape_rejected(self, tmp_path, url):
+        from openbadgeslib.errors import ConfigError
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[issuer]\nname = x\npublish_url = %s\n' % url)
+        with pytest.raises(ConfigError):
+            ConfParser(path).read_conf()
+
+    @pytest.mark.parametrize('key', ['url', 'image'])
+    def test_other_issuer_url_keys_rejected(self, tmp_path, key):
+        # publish_url is the documented leak, but [issuer] url is the OB3
+        # issuer-id fallback and image is urljoin'd into published metadata:
+        # every identifier in the section is public.
+        from openbadgeslib.errors import ConfigError
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[issuer]\nname = x\n%s = '
+            'https://user:%s@issuer.example/x\n' % (key, _PW))
+        with pytest.raises(ConfigError) as exc:
+            ConfParser(path).read_conf()
+        assert '[issuer] %s' % key in str(exc.value)
+        assert _PW not in str(exc.value)
+
+    @pytest.mark.parametrize('key', ['image', 'criteria', 'status_base',
+                                     'hosted_assertions_base'])
+    def test_badge_section_url_keys_rejected(self, tmp_path, key):
+        from openbadgeslib.errors import ConfigError
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[badge_1]\n%s = '
+            'https://user:%s@issuer.example/badge_1/\n' % (key, _PW))
+        with pytest.raises(ConfigError) as exc:
+            ConfParser(path).read_conf()
+        assert '[badge_1] %s' % key in str(exc.value)
+        assert _PW not in str(exc.value)
+
+    def test_interpolated_userinfo_rejected(self, tmp_path):
+        # The check runs after ${...} resolution, so a credential cannot be
+        # smuggled in through a reference to an unchecked section.
+        from openbadgeslib.errors import ConfigError
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\nhost = user:%s@issuer.example\n\n'
+            '[issuer]\nname = x\n'
+            'publish_url = https://${paths:host}/issuer/\n' % _PW)
+        with pytest.raises(ConfigError) as exc:
+            ConfParser(path).read_conf()
+        assert _PW not in str(exc.value)
+
+    def test_clean_urls_accepted(self, tmp_path):
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[issuer]\nname = x\n'
+            'url = https://www.issuer.example\n'
+            'email = issuer_mail@issuer.example\n'
+            'publish_url = https://issuer.example:8443/issuer/\n'
+            'revocationList = revoked.json\n\n'
+            '[badge_1]\nimage = https://issuer.example/b/badge1.svg\n'
+            'private_key = ${paths:base}/keys/sign.pem\n'
+            'mail = ${paths:base}/badge_1_mail.txt\n')
+        conf = ConfParser(path).read_conf()
+        # A bare mail address is not userinfo, and neither are paths or ports.
+        assert conf['issuer']['email'] == 'issuer_mail@issuer.example'
+        assert conf['issuer']['publish_url'] == 'https://issuer.example:8443/issuer/'
+
+    def test_smtp_credentials_are_not_touched(self, tmp_path):
+        # [smtp] username/password are legitimate secrets that stay on the
+        # issuer's machine — they are never published, so they are not checked.
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[smtp]\nsmtp_server = localhost\n'
+            'username = postmaster\npassword = %s\n' % _PW)
+        conf = ConfParser(path).read_conf()
+        assert conf['smtp']['password'] == _PW
+
+    def test_cli_wrapper_exits_without_echoing_the_password(self, tmp_path, capsys):
+        from openbadgeslib.confparser import read_config_or_exit
+        path = _write(
+            tmp_path,
+            '[paths]\nbase = .\n\n[issuer]\nname = x\n'
+            'publish_url = https://user:%s@issuer.example/issuer/\n' % _PW)
+        with pytest.raises(SystemExit):
+            read_config_or_exit(path)
+        out = capsys.readouterr()
+        assert out.out.startswith('[!]')
+        assert _PW not in out.out and _PW not in out.err
+
     def test_bad_interpolation_reference_raises_clean_value_error(self, tmp_path):
         # ExtendedInterpolation resolves ${...} lazily; a bad reference in a
         # section other than [paths] must surface here, at load time, as a

tests/test_ob3_did_doc.py

@@ -39,13 +39,29 @@ def test_rejects_non_https(self, url):
     @pytest.mark.parametrize('url', [
         'https://user:secret@example.com/',
         'https://user@example.com/badges/',
+        'https://@example.com/',
     ])
     def test_rejects_userinfo(self, url):
         # A user:pass@ credential must never be embedded into the DID (which
         # is carried by every issued credential); reject rather than leak it.
         with pytest.raises(ValueError):
             did_web_from_url(url)
 
+    @pytest.mark.parametrize('url', [
+        'http://user:hunter2-zzz@example.com/',   # fails the scheme check too
+        'https://user:hunter2-zzz@',              # fails the host check too
+        'ftp://user:hunter2-zzz@example.com/',
+    ])
+    def test_no_message_echoes_the_password(self, url):
+        # The userinfo check must run *before* the scheme and host checks,
+        # which quote the URL: a CLI prints the ValueError to stdout (and into
+        # the 'error' field of --json), so a password reaching those messages
+        # would be published by the very error meant to reject it.
+        with pytest.raises(ValueError) as exc:
+            did_web_from_url(url)
+        assert 'hunter2-zzz' not in str(exc.value)
+        assert 'hunter2-zzz' not in repr(exc.value)
+
     @pytest.mark.parametrize('url,doc_url', [
         ('https://example.com', 'https://example.com/.well-known/did.json'),
         ('https://example.com/badges/', 'https://example.com/badges/did.json'),

wiki/Configuration.md

@@ -100,6 +100,8 @@ revocationList = revoked.json
 
 For OB3 the issuer `id` is taken from `publish_url`, falling back to `url` if `publish_url` is absent — unless `did` is set (see the table above). For OB1/OB2, `openbadges-publish` writes an `organization.json` and joins `image` and `revocationList` onto `publish_url`; `-V 2` additionally writes a `CryptographicKey` `key.json` per badge and a conformant `RevocationList`.
 
+> **No credentials in URLs.** Every URL in `[issuer]` and `[badge_<name>]` is a *public* identifier: the tools print it, publish it inside `organization.json` / `badge.json` / `key.json`, derive the issuer's did:web from it and embed it (as `credentialStatus`) in the credentials delivered to recipients. A URL carrying userinfo — `https://user:password@host/` — would leak that password into all of them, so the config is **rejected at load time** with an error naming the key (never the value). Protect a staging host with an IP allow-list or a token the library never sees, not with credentials in `publish_url`. `[smtp] username` / `password` are unaffected: they stay on the issuer's machine.
+
 ### `[badge_<name>]`
 
 Define one section per badge. The part after `badge_` is the badge id you pass to the scripts (e.g. `[badge_1]` is used as `-b 1` / `-g 1`).