-
Notifications
You must be signed in to change notification settings - Fork 380
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Added method to set random local port #116
Added method to set random local port #116
Conversation
Added method to set a random local port, so that the board needs not to use always the same embedded local port for receiving NTP packets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great change, small corrections necessary ;)
NTPClient.h
Outdated
@@ -15,7 +15,7 @@ class NTPClient { | |||
|
|||
const char* _poolServerName = "pool.ntp.org"; // Default time server | |||
IPAddress _poolServerIP; | |||
int _port = NTP_DEFAULT_LOCAL_PORT; | |||
long _port = NTP_DEFAULT_LOCAL_PORT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @luigigubello 👋 ☕ Is there any special reason why you change this data type from int
to long
? Certainly int
is capable of holding values up 2^16-1 even on AVR 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly int is capable of holding values up 2^16-1 even on AVR
Reading the documentation I have understood it's not, but your suggestion to use unsigned int
is a good idea. So, yes, I have chosen long
instead int
to be sure to maintain the AVR compatibility. It seems unsigned int
is the best choice 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked again - so yes, int
2 byte on AVR which covers from -2^15 to 2^15-1. unsigned int
covers 0 to 2^16-1 which is enough.
NTPClient.h
Outdated
/** | ||
* Set random local port | ||
*/ | ||
void setRandomPort(long minValue, long maxValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here ... I think int
or even better unsigned int
(as it excludes negative values) would do the job just nicely here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right
…al_port Added method to set random local port
At the moment, the variable
port
is a constant embedded value used by the board for receiving NTP packets via unencrypted UDP connection. Changing randomly port to receive NTP packets doesn't improve cryptographic security and it is not the final solution, but it adds a layer to make harder the attacker's job.How to use
How it works
setRandomPort()
sets a pseudorandom port andNTPClient::update()
launchesNTPClient.begin(port)
ifport
is different by default valueNTP_DEFAULT_LOCAL_PORT