[Git][NTPsec/ntpsec][master] Added explanatory comments, swapped some backwards error checks
Ian Bruene
gitlab at mg.gitlab.com
Wed Jan 24 16:44:55 UTC 2018
Ian Bruene pushed to branch master at NTPsec / ntpsec
Commits:
c2b0b132 by Ian Bruene at 2018-01-24T10:43:20-06:00
Added explanatory comments, swapped some backwards error checks
- - - - -
2 changed files:
- ntpclients/ntpsnmpd.py
- pylib/agentx.py
Changes:
=====================================
ntpclients/ntpsnmpd.py
=====================================
--- a/ntpclients/ntpsnmpd.py
+++ b/ntpclients/ntpsnmpd.py
@@ -43,11 +43,11 @@ DEFHOST = "localhost" # For now only know how to talk to the local ntp
class DataSource: # This will be broken up in future to be less NTP-specific
def __init__(self):
node = ax.mibnode
- # This is defined as a dict tree because simpler, and avoids
+ # This is defined as a dict tree because it is simpler, and avoids
# certain edge cases
# OIDs are relative from ntp root
- # ntpEntNotifications
self.oidTree = {
+ # ntpEntNotifications
0:
node(None, None, True,
# ntpEntNotifModeChange
@@ -252,10 +252,11 @@ class DataSource: # This will be broken up in future to be less NTP-specific
self.session = ntp.packet.ControlSession()
self.session.openhost(DEFHOST) # only local for now
# Cache so we don't hammer ntpd, default 1 second timeout
- # timeout default pulled from a hat
+ # Timeout default pulled from a hat: we don't want it to last for
+ # long, just not flood ntpd when we don't need to.
self.cache = ntp.util.Cache(1)
self.oldValues = {} # Used by notifications to detect changes
- self.lastHeartbeat = 0
+ self.lastHeartbeat = 0 # Timestamp used for heartbeat notifications
# The undo system is only for the last operation
self.inSetP = False # Are we currently in the set procedure?
self.setVarbinds = [] # Varbind of the current set operation
@@ -263,8 +264,8 @@ class DataSource: # This will be broken up in future to be less NTP-specific
self.setUndoData = [] # Previous values for undoing
self.heartbeatInterval = 0 # should save to disk
self.sentNotifications = 0
- # Notify bits, these should be saved to disk
- # Also most of them currently have no effect
+ # Notify bits, they control whether the daemon sends notifications.
+ # these should be saved to disk
self.notifyModeChange = False # 1
self.notifyStratumChange = False # 2
self.notifySyspeerChange = False # 3
@@ -279,22 +280,26 @@ class DataSource: # This will be broken up in future to be less NTP-specific
while True:
try:
oid, reader, writer = gen.next()
- if nextP is True:
+ if nextP is True: # GetNext
+ # For getnext any OID greater than the start qualifies
oidhit = (oid > searchoid)
- else:
+ else: # Get
+ # For get we need a *specific* OID
oidhit = (oid.subids == searchoid.subids)
if oidhit and (reader is not None):
+ # We only return OIDs that have a minimal implementation
+ # walkMIBTree handles the generation of dynamic trees
if returnGenerator is True:
return oid, reader, writer, gen
else:
return oid, reader, writer
- except StopIteration:
+ except StopIteration: # Couldn't find anything in the tree
if returnGenerator is True:
return None, None, None, None
else:
return None, None, None
- # These exist instead of just using getOID_core for clearer semantics
+ # These exist instead of just using getOID_core so semantics are clearer
def getOID(self, searchoid, returnGenerator=False):
"Get the requested OID"
return self.getOID_core(False, searchoid, returnGenerator)
@@ -348,7 +353,7 @@ class DataSource: # This will be broken up in future to be less NTP-specific
# comment divider lines represent not yet implemented callbacks
# =============================================================
- #########################
+ # Blank: notification OIDs
def cbr_systemInfo(self, oid, category=None):
if category == "name": # The product name of the running NTP
@@ -435,9 +440,10 @@ class DataSource: # This will be broken up in future to be less NTP-specific
# That is the opposite of what the spec claims.
#
# I am abandoning the spec, and going with what makes a lick of sense
- uptime = self.safeReadvar(0, ["ss_reset"])["ss_reset"] * 100
+ uptime = self.safeReadvar(0, ["ss_reset"])
if uptime is None:
return None
+ uptime = uptime["ss_reset"] * 100
return ax.Varbind(ax.VALUE_TIME_TICKS, oid, uptime)
def cbr_statusDateTime(self, oid):
@@ -471,9 +477,10 @@ class DataSource: # This will be broken up in future to be less NTP-specific
def cbr_statusLeapSecDirection(self, oid):
# range of int32
- leap = self.safeReadvar(0, ["leap"])["leap"]
+ leap = self.safeReadvar(0, ["leap"])
if leap is None:
return None
+ leap = leap["leap"]
if leap == 1:
pass # leap 1 == forward
elif leap == 2:
@@ -784,7 +791,7 @@ class DataSource: # This will be broken up in future to be less NTP-specific
if self.notifySyspeerChange is True:
self.doNotifySyspeerChange(control)
- # Both and and remove have to look at the same data, don't want them
+ # Both add and remove have to look at the same data, don't want them
# stepping on each other. Therefore the functions are combined.
if (self.notifyAddAssociation is True) and \
(self.notifyRMAssociation is True):
@@ -821,19 +828,20 @@ class DataSource: # This will be broken up in future to be less NTP-specific
def doNotifyStratumChange(self, control):
oldStratum = self.oldValues.get("stratum")
- newStratum = self.safeReadvar(0, ["stratum"])["stratum"]
+ newStratum = self.safeReadvar(0, ["stratum"])
if newStratum is None:
return # couldn't read
+ newStratum = newStratum["stratum"]
if oldStratum is None:
self.oldValues["stratum"] = newStratum
return
elif oldStratum != newStratum:
self.oldValues["stratum"] = newStratum
- datetime = self.safeReadvar(0, ["reftime"])["reftime"]
+ datetime = self.safeReadvar(0, ["reftime"])
if datetime is None:
datetime = ""
else:
- datetime = ntp.util.deformatNTPTime(datetime)
+ datetime = ntp.util.deformatNTPTime(datetime["reftime"])
vl = [ax.Varbind(ax.VALUE_OID, snmpTrapOID,
ax.OID(ntpRootOID + (0, 2))),
ax.Varbind(ax.VALUE_OCTET_STR, ntpRootOID + (1, 2, 9),
@@ -847,19 +855,20 @@ class DataSource: # This will be broken up in future to be less NTP-specific
def doNotifySyspeerChange(self, control):
oldSyspeer = self.oldValues.get("syspeer")
- newSyspeer = self.safeReadvar(0, ["peeradr"])["peeradr"]
+ newSyspeer = self.safeReadvar(0, ["peeradr"])
if newSyspeer is None:
return # couldn't read
+ newSyspeer = newSyspeer["peeradr"]
if oldSyspeer is None:
self.oldValues["syspeer"] = newSyspeer
return
elif oldSyspeer != newSyspeer:
self.oldValues["syspeer"] = newSyspeer
- datetime = self.safeReadvar(0, ["reftime"])["reftime"]
+ datetime = self.safeReadvar(0, ["reftime"])
if datetime is None:
datetime = ""
else:
- datetime = ntp.util.deformatNTPTime(datetime)
+ datetime = ntp.util.deformatNTPTime(datetime["reftime"])
syspeer = self.misc_getSyspeerID()
vl = [ax.Varbind(ax.VALUE_OID, snmpTrapOID,
ax.OID(ntpRootOID + (0, 3))),
@@ -878,11 +887,11 @@ class DataSource: # This will be broken up in future to be less NTP-specific
changes = self.misc_getAssocListChanges()
if changes is None:
return
- datetime = self.safeReadvar(0, ["reftime"])["reftime"]
+ datetime = self.safeReadvar(0, ["reftime"])
if datetime is None:
datetime = ""
else:
- datetime = ntp.util.deformatNTPTime(datetime)
+ datetime = ntp.util.deformatNTPTime(datetime["reftime"])
adds, rms = changes
print("Notify Change assoc:", changes)
if which in ("add", "both"):
@@ -919,9 +928,10 @@ class DataSource: # This will be broken up in future to be less NTP-specific
def doNotifyLeapSecondAnnounced(self, control):
oldLeap = self.oldValues["leap"]
- newLeap = self.safeReadvar(0, ["leap"])["leap"]
+ newLeap = self.safeReadvar(0, ["leap"])
if newLeap is None:
return
+ newLeap = newLeap["leap"]
if oldLeap is None:
self.oldValues["leap"] = newLeap
return
@@ -929,11 +939,11 @@ class DataSource: # This will be broken up in future to be less NTP-specific
self.oldValues["leap"] = newLeap
if (oldLeap in (0, 3)) and (newLeap in (1, 2)):
# changed noleap or unsync to a leap announcement
- datetime = self.safeReadvar(0, ["reftime"])["reftime"]
+ datetime = self.safeReadvar(0, ["reftime"])
if datetime is None:
datetime = ""
else:
- datetime = ntp.util.deformatNTPTime(datetime)
+ datetime = ntp.util.deformatNTPTime(datetime["reftime"])
vl = [ax.Varbind(ax.VALUE_OID, snmpTrapOID,
ax.OID(ntpRootOID + (0, 7))),
ax.Varbind(ax.VALUE_OCTET_STR, ntpRootOID + (1, 2, 9),
@@ -1080,7 +1090,6 @@ class DataSource: # This will be broken up in future to be less NTP-specific
return peerids
def misc_getPeerData(self):
- # not fully implemented
peerdata = self.cache.get("peerdata")
if peerdata is None:
associds = self.misc_getPeerIDs()
@@ -1111,6 +1120,7 @@ class PacketControl:
self.recievedPackets = [] # use as FIFO
self.timeout = timeout
self.sessionID = None # need this for all packets
+ self.highestTransactionID = 0 # used for exchanges we start
# indexed on pdu code
self.pduHandlers = {ax.PDU_GET: self.handle_GetPDU,
ax.PDU_GET_NEXT: self.handle_GetNextPDU,
@@ -1193,6 +1203,8 @@ class PacketControl:
pkt, extraData = ntp.agentx.decode_packet(self.recievedData)
self.recievedData = extraData
self.recievedPackets.append(pkt)
+ if pkt.transactionID > self.highestTransactionID:
+ self.highestTransactionID = pkt.transactionID
dolog("\npacketEater got a full packet: %s\n" % repr(pkt), 3)
except ax.ParseDataLengthError:
return None # this happens if we don't have all of a packet
@@ -1218,8 +1230,10 @@ class PacketControl:
self.packetLog[index] = packet
def sendNotify(self, varbinds, context=None):
- # DUMMY: transactionID, packetID
- pkt = ax.NotifyPDU(True, self.sessionID, 300, 1, varbinds, context)
+ # DUMMY: packetID
+ tid = self.highestTransactionID + 5 # +5 to avoid collisions
+ self.highestTransactionID = tid
+ pkt = ax.NotifyPDU(True, self.sessionID, tid, 1, varbinds, context)
self.sendPacket(pkt, True)
def sendErrorResponse(self, errorHeader, errorType, errorIndex):
@@ -1255,11 +1269,15 @@ class PacketControl:
for oidr in packet.oidranges:
target = oidr.start
oid, reader, _ = self.database.getOID(target)
- if (oid != target) or (reader is None): # Not implemented
+ if (oid != target) or (reader is None):
+ # This OID must not be implemented yet.
binds.append(ax.Varbind(ax.VALUE_NO_SUCH_OBJECT, target))
else:
vbind = reader(oid)
if vbind is None: # No data avaliable.
+ # I am not certain that this is the correct response
+ # when no data is available. snmpwalk appears to stop
+ # calling a particular sub-agent when it gets to a NULL.
binds.append(ax.Varbind(ax.VALUE_NULL, target))
else:
binds.append(vbind)
@@ -1280,6 +1298,9 @@ class PacketControl:
oid, reader, _ = oids[0]
vbind = reader(oid)
if vbind is None: # No data available
+ # I am not certain that this is the correct response
+ # when no data is available. snmpwalk appears to stop
+ # calling a particular sub-agent when it gets to a NULL.
binds.append(ax.Varbind(ax.VALUE_NULL, oid))
else:
binds.append(vbind)
@@ -1329,7 +1350,7 @@ class PacketControl:
# Find an OID, then validate it
oid, reader, writer = self.database.getOID(varbind.oid)
if oid is None: # doesn't exist, can we create it?
- # Dummy, assume we can't create anything
+ # DUMMY, assume we can't create anything
error = ax.ERR_NO_ACCESS
break
elif writer is None: # exists, writing not implemented
=====================================
pylib/agentx.py
=====================================
--- a/pylib/agentx.py
+++ b/pylib/agentx.py
@@ -129,7 +129,7 @@ class ParsePDUTypeError(ParseError):
# packet, ready for transmission.
#
# Decoders take just the body sliced away from everything else,
-# and a dict containing the already pared information from the header.
+# and a dict containing the already parsed information from the header.
# They return the relevant class instance for the packet in question,
# they do not return extra data as they are never supposed to receive it.
#
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/c2b0b13200b8fdde0180e6604dcea9c5d4be0321
---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/c2b0b13200b8fdde0180e6604dcea9c5d4be0321
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20180124/5632e91f/attachment.html>
More information about the vc
mailing list