Skip to content
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

Some security issues #270

Open
MatejKovacic opened this issue Jan 20, 2025 · 12 comments
Open

Some security issues #270

MatejKovacic opened this issue Jan 20, 2025 · 12 comments

Comments

@MatejKovacic
Copy link

This device is really great, however there is a lot of work for security improvement. I completely understand that this is not a priority right now - first are usability and performance issues - but I hope developers will address these issues soon.

Here are some problems (this is not a complete list, just a quick observation).

  1. If no account configuration file is found, it defaults to a predefined admin account (admin/admin). Better solution would be, that when you run the NanoKVM the first time, you would need to set up it - and set strong password.

  2. By default ssh is enabled, and has default root password (root). Better solution would be to enable ssh via web interface and set initial password there.

  3. EncryptSecretKey is hardcoded for all "encrypt" operations.

  4. There is no CSRF protection on websocket and API endpoints.

  5. Auth token does not have refresh call, TTL is 30 days.

  6. User can not invalidate session (if session gets stolen).

  7. There is a lot of cases where developers decided to use sleep function between operations. The problem is, that it is possible that there could be situations, where the previous operation does not end in the sleep time. This can lead to weird and unpredictable situations.

  8. Why is NanoKVM sending your device key to the Sipeed?

  9. Network problems. Some users are using their own Wireguard servers (see issue here). This works, however I found out that the Wireguard version on NanoKVM does not work in some networks with more restricted frirewalls. In my company they have some restrictions in place (at least I know that they are blocking 3rd party DNS servers), and NanoKVM does not work there. I mean, it works, but only locally, and can not connect to the internet (i. e. can not check for updates), and Wireguard is also not working. On the other hand, Wireguard version on my computer works fine.

  10. NanoKVM is coneccting (and presumalby downloading something) from here: https://wiki.sipeed.com/maixvision. Why?

  11. NanoKVM has tcpdump and aircrack installed. (Seee issue here. I understand that is needed for development, but for production version sounds bad.

  12. Download code (for updates) does not check the integrity, it just downloads zip file.

  13. API is running as root. My suggestion would be to isolate this as well...

  14. Device uses some "custom" DNS servers and it is quite hard for users to change that... There should be network settings in the web GUI with all the transparent information. Also, option to use proxy would be nice.

This is not an exhaustive list, just some random observations, which show, that there needs to be done a lot in the area of security.

Also, I would suggest that instead of current Linux version you use this port of Debian. It is using apt (so you can normally install software - for instance right now there is no OpenVPN package on the device), systemd (so you can run services normally), etc.

My suggestion would be to help this developer with porting Debian to NanoKVM board (basically it is just SG200x board) and cooperate with Debian so that SG200x boards would be officially supported by Debian. That will increase trust in devices, make them much more usable and also lower the cost of development (because underlying Linux would be supported by community).

Also, it would be nice to have some security review of RISC-V CPU (like this one), but OK, I understand this is a little out of the scope at this point.

@MatejKovacic
Copy link
Author

Ah, and also... I would suggest to have an optin for modules. In GUI. That would allow users to install module for OpenVPN, custom Wireguard settings, etc.

@devoxel
Copy link

devoxel commented Jan 20, 2025

NanoKVM is coneccting (and presumalby downloading something) from here: https://wiki.sipeed.com/maixvision. Why?

Reading the source, it appears to be downloading a shared binary object - libmaixcam_lib.so [^] which it then detects and places into /kvmapp/kvm_system/dl_lib.

The purpose of this library is referenced here. It is not clear why this library is downloaded on boot instead of being included.

In any case, it's very insecure - since if an attacker gains control of the endpoint they can place malware in the shared binary. These binary sources should be at least be checked to ensure they match a specified hash sum; but ideally, these binaries could just be included in the release.

@scpcom
Copy link
Contributor

scpcom commented Jan 20, 2025

1. If no account configuration file is found, it defaults to a predefined admin account (admin/admin). Better solution would be, that when you run the NanoKVM the first time, you would need to set up it - and set strong password.

This is already done in the latest version. If you login to the web interface you get at notification to set the password. If you change the password the ssh password will also be changed.

8. Why is NanoKVM sending your device key to the Sipeed?

This is required by the server to generate the downloaded libmaixcam_lib.so which is locked to work only on this device. Which is also the reason why it is not included in the image.

The purpose of this library is referenced here. It is not clear why this library is downloaded on boot instead of being included.

If you follow the link which devoxel already mentioned, you find a fully functional open source replacement. I do not know why Sipeed still prefers to ship a dongled version, there is no secret inside anymore :-)

@MatejKovacic
Copy link
Author

So, if I understand this libmaixcam_lib.so, which is locked for a device could be replaced with this streameye/kvm_stream?

Well, I would be really happy if someone would make a truly opensource version of this, based on this Debian port and code mentioned here.

@scpcom
Copy link
Contributor

scpcom commented Jan 20, 2025

No, libmaixcam_lib.so can be replaced with this:
https://github.com/scpcom/sophgo-middleware/tree/maix_mmf-cvisdk/sample/test_mmf/maixcam_lib
kvm_stream is obsolete, you need libkvm.so for the latest NanoKVM version, which can be found here:
https://github.com/scpcom/sophgo-middleware/tree/maix_mmf-cvisdk/sample/test_mmf/kvm_vision

This variant is truly open source:
https://github.com/scpcom/LicheeSG-Nano-Build
But yes, Debian may be better to get latest updates. Maybe I will take a look at it.

@rtfmkiesel
Copy link

I investigated the download of libmaixcam_lib.so a bit yesterday. It was a suspicious to me that the endpoint was called encryption. (see this line)

This is also the endpoint that receives you /device_key as a uid GET parameter.

If you want to test this, use the following command: (Linux, add .exe to curl for Windows)

curl "https://maixvision.sipeed.com/api/v1/nanokvm/encryption?uid=<YOUR DEVICE KEY>" -H "token: MaixVision2024" -o libmaxicam_lib.so

Depending on your device key, this file differs. This means, each user will get a unique shared object. I verified this using checksums. This is very weird.

If you want to verify this, here is a list of device keys, which I guessed by comparing two device keys of NanoKVM's:

d1c94d9c1359c262 
d1c94d9e1359c262 
d1c94da21359c262 
d1c94da41359c262 
d1c94da61359c262 
d1c94daa1359c262 
d1c94dac1359c262 
d1c94dae1359c262 
d1c94db01359c262 
d1c94db21359c262 
d1c94dba1359c262 
d1c94dbc1359c262 
d1c94e031359c262 
d1c94e041359c262 
d1c94e061359c262 
d1c94e0a1359c262 
d1c94e0c1359c262 
d1c94e0e1359c262 
d1c94e131359c262 
d1c94e171359c262 
d1c94e191359c262 
d1c94e1b1359c262 
d1c94e1d1359c262 
d1c94e1f1359c262 
d1c94e231359c262 
d1c94e251359c262 
d1c94e271359c262 
d1c94e291359c262 
d1c94e2e1359c262 
d1c94e2f1359c262 
d1c94e311359c262 
d1c94e331359c262 
d1c94e371359c262 

I'm not into reverse engineering (RISC-V) Linux stuff. Maybe someone wants to investigate this further. The ELF header with readelf -a seems to be the same. It might also just be a freshly compiled shared object each time someone requests this API endpoint, hence the checksum difference.

@wj-xiao
Copy link
Collaborator

wj-xiao commented Jan 22, 2025

Thank you for raising these questions. Enhancing security will be our primary focus in the upcoming development phases.

The issue with libmaixcam_lib.so is a legacy matter. We reused code from our previous project (MaixCAM), which is why this dependencie is still present.
Currently, some HDMI features rely on this library. We are planning to refactor this part of the code and aim to remove the dependency by Q1 2025.

@wj-xiao
Copy link
Collaborator

wj-xiao commented Jan 22, 2025

A brief response to the above questions:

  1. If no account configuration file is found, it defaults to a predefined admin account (admin/admin). Better solution would be, that when you run the NanoKVM the first time, you would need to set up it - and set strong password.
  2. By default ssh is enabled, and has default root password (root). Better solution would be to enable ssh via web interface and set initial password there.

Now we prompt users to change their passwords. And the changed password will apply to both web and SSH access. We are also considering disabling SSH access by default.

  1. EncryptSecretKey is hardcoded for all "encrypt" operations.

This EncryptSecretKey is only used to encrypt password strings, preventing plaintext transmission of passwords.

  1. There is no CSRF protection on websocket and API endpoints.

Enabling CSRF may cause api unavailability in some cases. We will fix this issue.

  1. Auth token does not have refresh call, TTL is 30 days.
  2. User can not invalidate session (if session gets stolen).

The JWT token key is dynamically generated upon each service restart, therefore, all existing logins will be invalidated.
You can also configure a static key in the configuration file to avoid this behavior.

  1. There is a lot of cases where developers decided to use sleep function between operations. The problem is, that it is possible that there could be situations, where the previous operation does not end in the sleep time. This can lead to weird and unpredictable situations.

This is bug and we will fix it.

  1. Why is NanoKVM sending your device key to the Sipeed?

To download libmaixcam_lib.so, please refer to my previous comment.

  1. Network problems. Some users are using their own Wireguard servers (see issue here). This works, however I found out that the Wireguard version on NanoKVM does not work in some networks with more restricted frirewalls.n my company they have some restrictions in place (at least I know that they are blocking 3rd party DNS servers), and NanoKVM does not work there. I mean, it works, but only locally, and can not connect to the internet (i. e.
    can not check for updates), and Wireguard is also not working. On the other hand, Wireguard version on my computer works fine.

we will fix it.

  1. NanoKVM is coneccting (and presumalby downloading something) from here: https://wiki.sipeed.com/maixvision. Why?

To download libmaixcam_lib.so.

  1. NanoKVM has tcpdump and aircrack installed. (Seee tcpdump and aircrack? #248. I understand that is needed for development, but for production version sounds bad.
  2. Download code (for updates) does not check the integrity, it just downloads zip file.
  3. API is running as root. My suggestion would be to isolate this as well...
  4. Device uses some "custom" DNS servers and it is quite hard for users to change that... There should be network settings in the web GUI with all the transparent information. Also, option to use proxy would be nice.

We will optimize these issues later.

@MatejKovacic
Copy link
Author

Great! I think this is a great product and I am glad it will be even better.

@gshpychka
Copy link

To download libmaixcam_lib.so, please refer to my previous comment.

Why is the device key required to download?

@wj-xiao
Copy link
Collaborator

wj-xiao commented Jan 22, 2025

Why is the device key required to download?

Device key verification is required before downloading. An incorrect key will prevent the download.

@gshpychka
Copy link

Why is the device key required to download?

Device key verification is required before downloading. An incorrect key will prevent the download.

Right - I'm asking why?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants