Skip to content

Add WebSocket at /cp/serial/ #6584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 13, 2022
Merged

Add WebSocket at /cp/serial/ #6584

merged 11 commits into from
Jul 13, 2022

Conversation

tannewt
Copy link
Member

@tannewt tannewt commented Jul 11, 2022

This gives access to the serial output from a web browser.

Progress on #6174

@tannewt tannewt added supervisor espressif applies to multiple Espressif chips labels Jul 11, 2022
@tannewt tannewt added this to the 8.0.0 milestone Jul 11, 2022
@tannewt tannewt requested a review from jepler July 11, 2022 21:59
jepler
jepler previously approved these changes Jul 12, 2022
Copy link

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parts I understand seem OK, but I can't really comment on the websocket implementation. Probably fine to merge when green :)

@RetiredWizard
Copy link

I had to comment out the ESP_LOGE error messages in websocket.c (maybe missing esp-idf include?) but then the PR compiled and I was able to reach the serial WebSocket. The Ctrl-D, Ctrl-C and return worked but I wasn't able to send normal text/commands. If I connected via USB I could enter normally and the WebSocket screen correctly mirrored what was going on in the USB console.

@tannewt
Copy link
Member Author

tannewt commented Jul 12, 2022

@RetiredWizard Was this with Chrome? I've been mostly testing with Firefox.

@RetiredWizard
Copy link

Yes, with chrome. I didn't get as much time to play with it as I had hoped, got bogged down with other board issues :/

I think I was missing some error messages by commenting out the ESP_LOGE statements but it also seemed I was able to break the socket connection by spewing a lot of output to the REPL

@RetiredWizard
Copy link

I rebuilt the PR with today's main branch and replaced the ESP_LOGE statements in websocket.c with mp_printf statements. When I throw a lot of output at the serial port the websocket disconnects (stops mirroring and the SEND and CTRL buttons go grey). Nothing shows up on the DEBUG console but on the USB console the following message shows up at the point the socket disconnects:

 short send on 57 err 31 len 62

This is one of the ESP_LOGE messages I replaced in order to get the PR to compile.

Refreshing the WebSocket page reconnects it without issue.

@RetiredWizard
Copy link

RetiredWizard commented Jul 12, 2022

I changed the input.onbeforeinput to input.oninput in supervisor/shared/web_workflow/static/serial.js and the serial WebSocket started working. Really Really cool!!!!

@RetiredWizard
Copy link

RetiredWizard commented Jul 12, 2022

I seem to be able to reproduce this by running a uPyBasic program, the wifi connection crashes whether I'm using the serial WebSocket or not. uPyBasic does a LOT of flash i/o so maybe that has something to do with it.

 W (60218) wifi:<ba-del>idx
 W (60218) wifi:<ba-add>idx:0 (ifx:0, 48:5d:36:d1:30:e0), tid:0, ssn:235, winSize:64
 I (67598) wifi:bcn_timout,ap_probe_send_start
 W (67598) CP wifi: event 21 0x15
 W (67618) wifi:<ba-del>idx
 W (68618) wifi:<ba-add>idx:0 (ifx:0, 48:5d:36:d1:30:e0), tid:0, ssn:239, winSize:64
 W (70418) wifi:<ba-del>idx
 W (71218) wifi:<ba-add>idx:0 (ifx:0, 48:5d:36:d1:30:e0), tid:0, ssn:240, winSize:64
 I (99378) wifi:bcn_timout,ap_probe_send_start
 W (99378) CP wifi: event 21 0x15
 I (101878) wifi:ap_probe_send over, resett wifi status to disassoc
 I (101878) wifi:state: run -> init (c800)
 I (101878) wifi:pm stop, total sleep time: lu us / lu us

 W (101878) wifi:<ba-del>idx
 I (101888) wifi:new:<1,0>, old:<1,0>, ap:<255,255>, sta:<1,0>, prof:1
 W (101888) CP wifi: disconnected
 W (101898) CP wifi: reason 200 0xc8
 E (101908) CP websocket: short send on 58 err -113 len 2
 E (101908) CP websocket: short send on 58 err -128 len 2
 E (101918) CP websocket: short send on 58 err -128 len 14
 E (101918) CP websocket: short send on 58 err -128 len 4

The last four messages only appear if I have the WebSocket page up.

I'm still running with the supervisor_disable_tick commented out of the flash.c module.....

@RetiredWizard
Copy link

Hmm, after hanging the wifi several times from uPyBasic, I just ran the program sucessfully without losing the WebSocket, not quite repeatable.... I am getting a few Warning messages on the DEBUG console but everything seems to be running fine this time.

 I (156658) wifi:bcn_timout,ap_probe_send_start
 W (156658) CP wifi: event 21 0x15
 W (159028) wifi:<ba-del>idx
 W (159028) wifi:<ba-add>idx:0 (ifx:0, 48:5d:36:d1:30:e0), tid:0, ssn:174, winSize:64
 W (209958) wifi:<ba-del>idx
 W (210028) wifi:<ba-add>idx:0 (ifx:0, 48:5d:36:d1:30:e0), tid:0, ssn:266, winSize:64
 W (253028) wifi:<ba-del>idx
 W (253028) wifi:<ba-add>idx:0 (ifx:0, 48:5d:36:d1:30:e0), tid:0, ssn:326, winSize:64

@tannewt
Copy link
Member Author

tannewt commented Jul 12, 2022

Reason 200 is WIFI_REASON_BEACON_TIMEOUT so I'm not sure it is a websocket issue.

@RetiredWizard
Copy link

Looks like you might be right, I moved the microcontroller in the direction of my router and the program is running clean now

@RetiredWizard
Copy link

RetiredWizard commented Jul 13, 2022

I threw a hack into _origin_ok to compare against the DHCP host name in addition to the mDNS host name. It's not real pretty but it seems to get the job done.

That being said, I'm not sure this test is actually doing what we're hoping it is. I don't think it's filtering out requests from rogue or outside sources. The origin variable isn't actually the origin of the request, it's the host name used in the request query, the destination may be a better name.

If an IP scanner was probing a LAN with these devices, I'm assuming the rogue requests would come through with IP addresses which we're currently not blocking.

I did create a hostname called badhost in my local host file and when I try to bring up one of the secure CP pages using this host name, the bad origin message is displayed and access is blocked.

static bool _origin_ok(const char *origin) {
    const char *http = "http://";
    const char *local = ".local";

    if (memcmp(origin, http, strlen(http)) != 0) {
        return false;
    }
    // These are prefix checks up to : so that any port works.
    const char *hostname = common_hal_mdns_server_get_hostname(&mdns);
    const char *end = origin + strlen(http) + strlen(hostname) + strlen(local);
    if (memcmp(origin + strlen(http), hostname, strlen(hostname)) == 0 &&
        memcmp(origin + strlen(http) + strlen(hostname), local, strlen(local)) == 0 &&
        (end[0] == '\0' || end[0] == ':')) {
        return true;
    }

    end = origin + strlen(http) + strlen(_our_ip_encoded);
    if (memcmp(origin + strlen(http), _our_ip_encoded, strlen(_our_ip_encoded)) == 0 &&
        (end[0] == '\0' || end[0] == ':')) {
        return true;
    }

    const char *localhost = "127.0.0.1:";
    if (memcmp(origin + strlen(http), localhost, strlen(localhost)) == 0) {
        return true;
    }

    for (size_t i = 0; i < MP_ARRAY_SIZE(ok_hosts); i++) {
        // This checks exactly.
        if (strcmp(origin + strlen(http), ok_hosts[i]) == 0) {
            return true;
        }
    }

    const char *dhcp_host = mp_obj_str_get_str(common_hal_wifi_radio_get_hostname(&common_hal_wifi_radio_obj));
    const char *origptr = origin + strlen(http);

    ESP_LOGI(TAG,"DHCP host name %s" , dhcp_host);
    ESP_LOGI(TAG,"MDNS host name %s" , hostname);

    if ( *dhcp_host == '\0' || *origptr == '\0' ) {
        return false;
    }

    for (;; dhcp_host++, origptr++) {
        if ( tolower( (unsigned char)*dhcp_host ) != tolower( (unsigned char)*origptr ) ) {
            return false;
        }
        if ( *dhcp_host == '\0' || *origptr == '\0' ) {
            return true;
        }
    }

    return false;
}

@RetiredWizard
Copy link

I've built the PR with @tannewt's chrome tweak and it works great.

@tannewt
Copy link
Member Author

tannewt commented Jul 13, 2022

@jepler Please re-review. The two build failures are unrelated to the PR.

Copy link

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I didn't see anything in the later updates that was concerning, and now it's tested by someone else too.

@tannewt tannewt merged commit 2f9de1c into adafruit:main Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
espressif applies to multiple Espressif chips supervisor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants