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

Add deploy/zyxel_gs1900.sh #5043

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

coderjoe
Copy link

@coderjoe coderjoe commented Mar 7, 2024

Add support for deploying to the Zyxel GS1900 line of switches as long as those switches are running at least firmware V2.80.

Tested on a Zyxel GS1900-8 and GS1900-24E

Resolves #5042

@coderjoe
Copy link
Author

coderjoe commented Mar 8, 2024

I'm now using this version locally for my switches. Please let me know if there is anything you'd like me to do to improve the PR. No rush. :)

@coderjoe
Copy link
Author

A bug has occurred on one of my servers with a different version of sed. Working on a fix now.

@coderjoe
Copy link
Author

Most recent commit should have resolved the issue I found.

deploy/zyxel_gs1900.sh Outdated Show resolved Hide resolved
@coderjoe coderjoe marked this pull request as draft March 27, 2024 01:49
Copy link
Author

@coderjoe coderjoe left a comment

Choose a reason for hiding this comment

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

These are the differences between the two implementations and why I was using curl rather than _post. I have not been able to get _post to work.

deploy/zyxel_gs1900.sh Outdated Show resolved Hide resolved
deploy/zyxel_gs1900.sh Outdated Show resolved Hide resolved
@coderjoe coderjoe force-pushed the feature/deploy-to-zyxel-gs1900-switches branch from 90a91a2 to 2bedbad Compare April 6, 2024 17:30
@coderjoe
Copy link
Author

coderjoe commented Apr 6, 2024

I've updated for the shellcheck and shfmt linting in the previous run, have removed the defunct curl errors (now that curl is not a requirement) and have squashed the commit to clean up history a bit.

@coderjoe
Copy link
Author

coderjoe commented Apr 6, 2024

I've verified that the updated commits continue to work in all of my deployed environments.
Thank you very much @Neilpang for your help/guidance on wget support.

I don't think I would have figured it out without your suggestion. :)

@coderjoe coderjoe marked this pull request as ready for review April 6, 2024 17:56
@coderjoe coderjoe force-pushed the feature/deploy-to-zyxel-gs1900-switches branch from 8646c3b to 721e57c Compare April 7, 2024 14:29
@coderjoe
Copy link
Author

coderjoe commented Jul 2, 2024

I have now been using this in production for a few months. @Neilpang please let me know if there is anything else you would like me to do - or if there's anything I can do to help get this included. :)

Thanks again for your time and help!

@coderjoe coderjoe requested a review from Neilpang August 18, 2024 22:07
Add support for deploying to the Zyxel GS1900 line of switches as long
as those switches are running at least firmware V2.80.

Tested on a Zyxel GS1900-8 and GS1900-24E

Resolves acmesh-official#5042
@coderjoe coderjoe force-pushed the feature/deploy-to-zyxel-gs1900-switches branch from 721e57c to 304dbcb Compare October 22, 2024 02:08
@coderjoe
Copy link
Author

Rebased onto the latest dev.
Please let me know what else needs to be done for a review.

@Neilpang
Copy link
Member

Neilpang commented Nov 3, 2024

why do you need to "ReadBodyFromFile"?

@coderjoe
Copy link
Author

coderjoe commented Nov 6, 2024

why do you need to "ReadBodyFromFile"?

Great question @Neilpang. I responded in the gitter.im chat while the repo was restricted, but I'll repeat it here just in case it wasn't seen.

I need to read from file because this switch expects a binary exact, not encoded, upload of the certificate. The uploaded certificate contains many \0 NULL bytes which cannot be piped within the shell script without being truncated.

Making wget or curl read the certificate file directly is the only way I have been able to get the full file uploaded in the manner the switch expects without the file becoming truncated by the null bytes.

I hope this helps clarify.

@dark-m0de
Copy link

Hi, I would really appreciate if we could add support for Zyxel devices. I'm using a XMG1915 switch, as well as NWA50AX PRO and NWA130BE access points. My assumption is that with a few changes, those devices could be supported as well.
I'd be happy to support the development for this.

@coderjoe
Copy link
Author

coderjoe commented Jan 7, 2025

Hello @dark-m0de , I appreciate your interest. I don't think in its current form my PR will be accepted. I think my modifications to the _post method in acme.sh are not of sufficient quality and I'm currently investigating alternatives.

Some notes:

  • Web interfaces between different series of Zyxel switches tend to be different enough that they will likely require separate integrations. At least that has been my past experience.
  • I'm entirely unfamiliar with their AP products so I cannot provide any recommendation, but much like the XGS series I would be very hesistant to support them in this script.
  • I have an open feature request on the Zyxel community to support other methods of upload other than just web based upload of binary PKCS12 certificates. That conversation is ongoing at this time and if you'd like to leave a polite "me too" that would be welcome.

For now I think I'm going to put this PR back into draft until I can come up with some alternative methods to get the certificates into the GS1900 series switches.

@coderjoe coderjoe marked this pull request as draft January 7, 2025 02:26
@dark-m0de
Copy link

Thank you @coderjoe, I just prepared my answer for the zyxel community. But somehow I'm not able to post it yet.

Another thought: Do you think it would help if zyxel adds support for uploading certificates to their CLI (e.g. ssh based)? This would add ssh as a dependency to acme in such scenarios. But it would definitely ease the process to manage certificates in a scripted way.

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

Successfully merging this pull request may close these issues.

3 participants