cmsis-pack-manager fails to download some CMSIS-packs #121
Comments
|
Nope. We support redirects. All of the keil.com pdcs redirect. These are not special. And we download the others. Perhaps they do not use a 300 for the redirect. The linked code does not implement downloading, it calls the actual download function across a stream of files. What does |
|
They give a 302. Well, actually they give likely just 200 OK as the error code and then a page with |
That sounds like a 200. Also, that's a lot of javascript. |
|
Yeah, it's actually really hard to write a command line tool that takes a 200 OK and realizes that it's actually a redirect. Also note that the error might have nothing to do with this, as it's about the file ending too soon. |
So it's probably not the redirect code. |
|
Seems like this is a bug: |
|
In a debug build, it looks like it does not receive the redirect: If it worked it would look like: |
|
I am unable to reproduce the problem: From my perspective (I am just uploading a pack through a Web Interface) there is nothing special about the Keil.STM32MH7xx_DFP.pdsc. Once you instruct curl to follow the redirect I do see the pdsc file being properly downloaded on my machine () . Sorry if I cannot provide any more insights. Please note that I also validated that the MDK Pack Installer would be able to download successfully. |
|
I think you're right; I think it's something going wrong in cmsis-pack-manager. It's odd that it only affects these 3 packs, and that it's consistent. |
|
Perhaps a strange idea, but what if we replaced that download code by a call to "curl" itself, i.e. |
|
Because that would be a nasty dependency to manage. |
|
@JanneKiiskila all this talk about handling redirects is a red herring, as this fails before invoking any redirection code. |
JanneKiiskila
changed the title
|
Yep, that's what you get when a noob starts looking at the rust code we have in this repo without any real understanding. Title and description modified. |
|
The older I get, the more I know what I don't know. |
|
Thanks for changing the title. I'm trying to root cause the issue here. This is weird that curl does not have the same issue. |
|
I have ruled out concurrency, as I removed concurrent downloads in a test build without affecting the problem behavior. |
|
Thanks for looking into this! |
|
Snippet from strace: A I'm thinking that CPM might be sending an invalid GET to the server and it's responds by shutting down the connection. |
|
I just traced a clean download of everything and it looks like all of these are https keil.com packs being, and these failing to download packs are the only packs that use https from keil.com. |
|
Why are there so many Starting GET of for the same file? Should not one be enough, or is the debug print just slightly misleading? |
|
It looks like this might be a TLS cyphersuite miss-match. CMP uses Rustls for TLS, and as such does not support TLS < 1.2. Using Openssl, I was able to figure out that www.keil.com supports the following cyphersuites: This might be a TLS cyphersuite mismatch. |
|
If (And this reply came way after the TLS analysis, that's interesting and |
|
@JanneKiiskila I'm not sure. I was thinking the same thing: why are we downloading the same thing 3-4 times? I'm going to put that aside for now and try to track down this TLS issue. |
|
I'm just wondering could it be an DDoS protection mechanism, i.e. it blocks the connection because it thinks someone is doing too much at same time? |
|
It could be a DDoS protection mechanism. I tried the Rustls example with www.keil.com: It's the middle one. Other commands provided for context. |
|
@JanneKiiskila Good digging in libraries. I'm using |
|
A multi-curl should theoretically then also fail, if there was a DDoS mechanism there, but it seems to work. At least this; Got all |
|
have you tried capturing the tls handshake with wireshark? |
|
Ah, dang. I'll check if |
|
It looks like ridirection support (https://docs.rs/reqwest/0.9.21/reqwest/async/struct.ClientBuilder.html#method.redirect) it's already built into I'm going to try replacing |
|
@theotherjimmy Thanks! |
|
I can confirm that Since I have around 1 hour of experience with Rust, I didn't manage to do anything complex but enough to prove that it works. And the file is downloaded correctly; I think you could just use even the |
|
So much for @JanneKiiskila being a Rust noob. Thanks for confirming that switching to reqwest will fix this issue, Janne. |
|
Hi |
|
@jeromecoutant, I think so, so long as we call https://docs.rs/reqwest/0.9.21/reqwest/async/struct.ClientBuilder.html#method.use_sys_proxy |
|
@JanneKiiskila I think we have further narrowed this down to a rustls-related issue, as the following diff makes it go away. This diff is applied to my https://github.com/theotherjimmy/cmsis-pack-manager/tree/use-reqwest branch. ❯ git diff
diff --git a/rust/cmsis-update/Cargo.toml b/rust/cmsis-update/Cargo.toml
index 91dd9b49b..c96bce7ad 100644
--- a/rust/cmsis-update/Cargo.toml
+++ b/rust/cmsis-update/Cargo.toml
@@ -19,5 +19,3 @@ pdsc = { path = "../pdsc" }
[dependencies.reqwest]
version = "0.9.21"
-default_features = false
-features = ["rustls-tls"]
diff --git a/rust/cmsis-update/src/download.rs b/rust/cmsis-update/src/download.rs
index 612bbaf23..43c680b13 100644
--- a/rust/cmsis-update/src/download.rs
+++ b/rust/cmsis-update/src/download.rs
@@ -134,7 +134,6 @@ where
{
pub fn new(config: &'a Conf, prog: Prog, log: &'a Logger) -> Result<Self, Error> {
let client = ClientBuilder::new()
- .use_rustls_tls()
.use_sys_proxy()
.redirect(RedirectPolicy::limited(5))
.build()?;
|
|
Shell session showing the result of that diff: |
Manually put them in, since we do not have a way to update the CMSIS packs (yet, details in pyocd/cmsis-pack-manager#121).
Updating of all CMSIS packs that running ``` python project.py -m STM32F7xx --update-packs ``` brings, while using a customer version of cmsis-pack-manager. (Essentially using the reqwest version from theOtherJimmy with the TLS fix mentioned in the comments). See: - pyocd/cmsis-pack-manager#121 - pyocd/cmsis-pack-manager#124 for more details.
|
always failed: |
|
That is unfortunately the expected results now when using the RusTLS. It only works without it, but if we do it without it - we can't do a fully generic PIP wheel binary (which PIP requires). |
|
Dang, the number of failures increased to 5. That's not great. |
|
Number of failures has grown even more, seems likely almost every NXP pack fails now. So, the problems keeps growing. |
|
Any update on this point ? |
JanneKiiskila
changed the title
|
Is there any news on this? As of now pyOCD gives me the following errors when updating the index: Furthermore, when I look at
|
|
@rapgenic Apologies, it keeps being fixed and then broken again. *sigh* Thanks for reporting these problems. I'm putting in a request for this latest case to be fixed. The root cause is that the keil.com server is old. Its TLS configuration doesn't include any modern ciphers that are considered secure, so the Rust rustls crate refuses to connect (cmsis-pack-manager's backend is implemented in Rust for performance). Sometimes when the list of Keil packs is updated, the author uses an https scheme for the pack, and thus cmsis-pack-manager via rustls will not allow it to be accessed. There is a ticket to update the server and fix the root cause, but it's moving very slowly. TLS analysis report for keil.com The second issue may be caused by errors by the silicon vendors, but clearly needs investigation. (And CPM should be detecting that the files are not XML, anyway.) I created issue #154 to track it. |
|
I thought the keil web site was supposed to be "fixed" by now?! |
|
@rapgenic It's fixed now! |
|
@flit Thank you very much! |
|
Since a while cmsis-packs for several microcontrollers can't be downloaded in pyocd. A few examples where the problem currently exists.
At least the STM32G0xx pack can be used in pyocd by manually downloading it and supplying it's path as argument. |
|
I see something has been done now; Has the issue been fixed/resolved now? @flit @mbrossard ? |
If you try to update the cmsis packs in Mbed OS with
python project.py -m STM32F7xx --update-packsit will start downloading the pack files.For a few ST packs, you will get the following error note:
(Edit: removed the faulty analysis of any redirects, title updated, too).
The text was updated successfully, but these errors were encountered: