Remove rustdoc's plugins feature #52194

steveklabnik · 2018-07-09T22:17:26Z

This fixes CVE-2018-1000622.

https://cve.mitre.org/cgi-bin/cvename.cgi?name=%20CVE-2018-1000622

steveklabnik · 2018-07-09T22:18:08Z

src/librustdoc/lib.rs

@@ -163,9 +163,6 @@ pub fn opts() -> Vec<RustcOptGroup> {
        stable("extern", |o| {
            o.optmulti("", "extern", "pass an --extern to rustc", "NAME=PATH")
        }),
-        stable("plugin-path", |o| {
-            o.optmulti("", "plugin-path", "directory to load plugins from", "DIR")
-        }),


We could also keep the flags and have them do nothing; not sure what your opinion is!

That's an interesting point. These flags are "stable", but enabled insecure code. While we can change stable things in the interest of security, i think it would be better to keep them and do nothing. At least then we can choose to link to this PR or the CVE instead of just reporting their absence.

I think of something a bit different: keeping the option but printing that it's doing nothing.

Sounds good to me.

rust-highfive · 2018-07-09T22:32:39Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Downloading https://files.pythonhosted.org/packages/e7/d9/f7e8d725a8f19dddfb4807753ad70d2935e1b8a425f4e9d5c87e06e83931/awscli-1.15.54-py2.py3-none-any.whl (1.3MB)
    0% |▎                               | 10kB 39.2MB/s eta 0:00:01
    1% |▌                               | 20kB 1.7MB/s eta 0:00:01
    2% |▊                               | 30kB 2.1MB/s eta 0:00:01
    3% |█                               | 40kB 1.8MB/s eta 0:00:01
---

[00:03:56] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:56] tidy error: /checkout/src/librustdoc/plugins.rs: missing trailing newline
[00:03:57] some tidy checks failed
[00:03:57] 
[00:03:57] 
[00:03:57] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:57] 
[00:03:57] 
[00:03:57] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:57] Build completed unsuccessfully in 0:00:48
[00:03:57] Build completed unsuccessfully in 0:00:48
[00:03:57] Makefile:79: recipe for target 'tidy' failed
[00:03:57] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:12fca9fb
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:0fa3bb99:start=1531175017357319914,finish=1531175017364949793,duration=7629879
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1b29ad16
$ head -30 ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
head: cannot open ‘./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers’ for reading: No such file or directory
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0df10810
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

QuietMisdreavus · 2018-07-09T22:49:56Z

History:

In Deprecate several flags in rustdoc #44138 (merged 2017-10-18) we added a deprecation notice to the --passes, --plugins, and --plugin-path CLI flags (among others), citing disuse, instability/unsafeness, and a generally flawed design. This was tracked in Tracking issue for rustdoc deprecations/cleanup #44136 but that issue was ultimately closed when the PR was merged.
- Any comments on the deprecation have all been about deprecating --passes, as that is used to make rustdoc document private items.
- The attributes #![doc(plugins="...")], #![doc(passes="...")], and #![doc(no_default_passes)] were also accepted, but these were changed to emit a deprecation warning in rustdoc: deprecate #![doc(passes, plugins, no_default_passes)] #50669 (merged 2018-05-16). No uses of these attributes have been found.
In Compile rustdoc on-demand #43482 (merged (2017-07-27), the Rust build system was changed to turn rustdoc into a "tool", removing it from the compiler dependencies and divorcing its compilation from the main compiler's flow.
- Its relevance here is that ever since then, we haven't distributed librustdoc in the sysroot the same way we distribute libsyntax, librustc, et al. See librustdoc no longer part of distributable #43729.
- Therefore, one of the requirements of building a rustdoc plugin (being able to use the internal clean::Crate type) became nearly impossible. To build a rustdoc plugin, you would need to have the source from a librustdoc that matches the compiler you're using, build it manually, and somehow use this rlib in your own crate. This differs from using (for example) librustc because rlibs distributed in the sysroot can be linked without having to specify an --extern to rustc or a [dependencies] entry to cargo.

So, in general, i'm in favor. Rustdoc plugins are unsafe (requiring manually loading a dynamic-link library), dubiously stable (all the individual components - the CLI flag, librustdoc's types - are marked stable, but in practice they are not), and only marginally useful. And to top it all off, they have been leaning on code that is insecure.

cuviper · 2018-07-09T23:52:10Z

Should this be beta-nominated too? Or will you apply the smaller CVE patch to beta?

cuviper · 2018-07-09T23:57:16Z

BTW, the CVE link is missing the last 2 in 1000622.

See CVE-2018-1000622.

steveklabnik · 2018-07-10T14:42:40Z

@cuviper my plan is to apply whatever lands in this PR to beta, and then additionally, merge some more refactoring into nightly afterwards; PluginManager is a bit mis-named after this patch :)

@QuietMisdreavus @GuillaumeGomez what do you think of this iteration?

QuietMisdreavus · 2018-07-10T15:22:20Z

I still kinda think it would be useful to link to this PR as well as the CVE, but there's enough context in the CVE description so i won't push too hard. r=me if @GuillaumeGomez is okay with it.

GuillaumeGomez · 2018-07-10T20:54:58Z

I am. Next release, we'll have to deprecate/remove the option too.

GuillaumeGomez · 2018-07-10T20:55:12Z

@bors: r=QuietMisdreavus,GuillaumeGomez

bors · 2018-07-10T20:55:13Z

📌 Commit 0df68d8 has been approved by QuietMisdreavus,GuillaumeGomez

bors · 2018-07-12T04:12:27Z

⌛ Testing commit 0df68d8 with merge c946c25...

@QuietMisdreavus

…,GuillaumeGomez Remove rustdoc's plugins feature This fixes CVE-2018-1000622. https://cve.mitre.org/cgi-bin/cvename.cgi?name=%20CVE-2018-1000622 r? @QuietMisdreavus @GuillaumeGomez

bors · 2018-07-12T06:16:17Z

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus,GuillaumeGomez
Pushing c946c25 to master...

pietroalbini · 2018-07-12T15:09:09Z

Ping @rust-lang/rustdoc, can someone approve this for backport?

GuillaumeGomez · 2018-07-12T15:24:15Z

Done.

@ghost

[beta] Rollup backports Merged and approved: * #51722: Updated RELEASES for 1.28.0 * #52193: step_by: leave time of item skip unspecified * #52194: Remove rustdoc's plugins feature * #52196: rustdoc: Hide struct and enum variant constructor imports * #52310: Backport 1.27.1 release notes to master r? @ghost

Refactor rustdoc This is based on #52194 and so shouldn't be merged until it gets merged. Now that plugin functionality has been removed, let's kill PluginManager. Additionally, rustdoc now follows the standard cargo layout instead of dumping everything at the top level. r? @rust-lang/rustdoc

…eGomez Cleanup rustdoc warnings ## Clean up error reporting for deprecated passes Using `error!` here goes all the way back to the original commit, rust-lang#8540. I don't see any reason to use logging; rustdoc should use diagnostics wherever possible. See rust-lang#81932 (comment) for further context. - Use spans for deprecated attributes - Use a proper diagnostic for unknown passes, instead of error logging - Add tests for unknown passes - Improve some wording in diagnostics ## Report that `doc(plugins)` doesn't work using diagnostics instead of `eprintln!` This also adds a test for the output. This was added in rust-lang#52194. I don't see any particular reason not to use diagnostics here, I think it was just missed in rust-lang#50541.

rust-highfive assigned QuietMisdreavus Jul 9, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2018

steveklabnik commented Jul 9, 2018

View reviewed changes

kennytm added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 10, 2018

Remove rustdoc plugins

0df68d8

See CVE-2018-1000622.

steveklabnik force-pushed the remove-plugins branch from e99e48b to 0df68d8 Compare July 10, 2018 14:41

QuietMisdreavus mentioned this pull request Jul 10, 2018

Misc rustdoc changes #52211

Merged

QuietMisdreavus added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 10, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2018

steveklabnik mentioned this pull request Jul 11, 2018

Refactor rustdoc #52257

Merged

bors merged commit 0df68d8 into rust-lang:master Jul 12, 2018

GuillaumeGomez added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 12, 2018

pietroalbini mentioned this pull request Jul 12, 2018

[beta] Rollup backports #52308

Merged

pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 12, 2018

jyn514 mentioned this pull request Mar 1, 2021

Cleanup rustdoc warnings #82651

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove rustdoc's plugins feature #52194

Remove rustdoc's plugins feature #52194

steveklabnik commented Jul 9, 2018 •

edited by kennytm

Loading

steveklabnik Jul 9, 2018

QuietMisdreavus Jul 9, 2018

GuillaumeGomez Jul 10, 2018

steveklabnik Jul 10, 2018

rust-highfive commented Jul 9, 2018

QuietMisdreavus commented Jul 9, 2018 •

edited

Loading

cuviper commented Jul 9, 2018

cuviper commented Jul 9, 2018

steveklabnik commented Jul 10, 2018 •

edited

Loading

QuietMisdreavus commented Jul 10, 2018 •

edited

Loading

GuillaumeGomez commented Jul 10, 2018

GuillaumeGomez commented Jul 10, 2018

bors commented Jul 10, 2018

bors commented Jul 12, 2018

bors commented Jul 12, 2018

pietroalbini commented Jul 12, 2018

GuillaumeGomez commented Jul 12, 2018

Remove rustdoc's plugins feature #52194

Remove rustdoc's plugins feature #52194

Conversation

steveklabnik commented Jul 9, 2018 • edited by kennytm Loading

steveklabnik Jul 9, 2018

Choose a reason for hiding this comment

QuietMisdreavus Jul 9, 2018

Choose a reason for hiding this comment

GuillaumeGomez Jul 10, 2018

Choose a reason for hiding this comment

steveklabnik Jul 10, 2018

Choose a reason for hiding this comment

rust-highfive commented Jul 9, 2018

QuietMisdreavus commented Jul 9, 2018 • edited Loading

cuviper commented Jul 9, 2018

cuviper commented Jul 9, 2018

steveklabnik commented Jul 10, 2018 • edited Loading

QuietMisdreavus commented Jul 10, 2018 • edited Loading

GuillaumeGomez commented Jul 10, 2018

GuillaumeGomez commented Jul 10, 2018

bors commented Jul 10, 2018

bors commented Jul 12, 2018

bors commented Jul 12, 2018

pietroalbini commented Jul 12, 2018

GuillaumeGomez commented Jul 12, 2018

steveklabnik commented Jul 9, 2018 •

edited by kennytm

Loading

QuietMisdreavus commented Jul 9, 2018 •

edited

Loading

steveklabnik commented Jul 10, 2018 •

edited

Loading

QuietMisdreavus commented Jul 10, 2018 •

edited

Loading