Merge pull request #28 from n-rodriguez/feat/p5-idiomatic

P5a: idiomatic-Crystal cleanups

Author: stakach

spec/snmp_spec.cr

@@ -274,7 +274,7 @@ describe SNMP do
     socket = UDPSocket.new
     socket.connect(TEST_SNMP_SERVER, 161)
     socket.sync = false
-    socket.read_timeout = 3
+    socket.read_timeout = 3.seconds
 
     # Setup session
     session = SNMP::V3::Session.new("usr-md5-none", "authkey1")
@@ -301,7 +301,7 @@ describe SNMP do
     socket = UDPSocket.new
     socket.connect(TEST_SNMP_SERVER, 161)
     socket.sync = false
-    socket.read_timeout = 3
+    socket.read_timeout = 3.seconds
 
     # Setup session
     session = SNMP::V3::Session.new("usr-md5-aes", "authkey1", "privkey1", priv_protocol: SNMP::V3::Security::PrivacyProtocol::AES)
@@ -328,7 +328,7 @@ describe SNMP do
     socket = UDPSocket.new
     socket.connect(TEST_SNMP_SERVER, 161)
     socket.sync = false
-    socket.read_timeout = 3
+    socket.read_timeout = 3.seconds
 
     # Setup session
     session = SNMP::V3::Session.new("usr-md5-des", "authkey1", "privkey1")
@@ -353,7 +353,7 @@ describe SNMP do
     socket = UDPSocket.new
     socket.connect(TEST_SNMP_SERVER, 161)
     socket.sync = false
-    socket.read_timeout = 3
+    socket.read_timeout = 3.seconds
 
     # Make request
     session = SNMP::Session.new
@@ -371,7 +371,7 @@ describe SNMP do
     # socket.connect("localhost", 32771)
     socket.connect(TEST_SNMP_SERVER, 161)
     socket.sync = false
-    socket.read_timeout = 3
+    socket.read_timeout = 3.seconds
 
     # Make request
     session = SNMP::Session.new("public")

src/snmp.cr

@@ -3,6 +3,9 @@ require "bindata/asn1"
 class SNMP
   alias UniversalTags = ASN1::BER::UniversalTags
 
+  # Range for a randomly-generated PDU request-id / v3 message-id.
+  REQUEST_ID_RANGE = 1..Int32::MAX
+
   module V3
     # SNMPv3 message flags describing the features used
     @[Flags]

src/snmp/client.cr

@@ -32,7 +32,7 @@ class SNMP::Client
   private def build_socket : UDPSocket
     socket = UDPSocket.new
     socket.sync = false
-    socket.read_timeout = timeout
+    socket.read_timeout = timeout.seconds
     socket
   end
 

src/snmp/message.cr

@@ -21,7 +21,7 @@ class SNMP::Message
     end
   end
 
-  def initialize(@community, @request, varbind : VarBind? | Array(VarBind) = nil, request_id = rand(2147483647), error_status = ErrorStatus::NoError, error_index = 0, @version = Version::V2C)
+  def initialize(@community, @request, varbind : VarBind? | Array(VarBind) = nil, request_id = rand(REQUEST_ID_RANGE), error_status = ErrorStatus::NoError, error_index = 0, @version = Version::V2C)
     @pdu = PDU.new(request_id, varbind, error_status, error_index)
   end
 

src/snmp/pdu.cr

@@ -9,10 +9,10 @@ class SNMP::PDU
     end
   end
 
-  def initialize(@request_id = rand(2147483647), @varbinds : Array(VarBind) = [] of VarBind, @error_status = ErrorStatus::NoError, @error_index = 0)
+  def initialize(@request_id = rand(REQUEST_ID_RANGE), @varbinds : Array(VarBind) = [] of VarBind, @error_status = ErrorStatus::NoError, @error_index = 0)
   end
 
-  def initialize(@request_id = rand(2147483647), varbind : VarBind? = nil, @error_status = ErrorStatus::NoError, @error_index = 0)
+  def initialize(@request_id = rand(REQUEST_ID_RANGE), varbind : VarBind? = nil, @error_status = ErrorStatus::NoError, @error_index = 0)
     if varbind
       @varbinds = [varbind]
     else
@@ -31,7 +31,7 @@ class SNMP::PDU
   property max_repetitions : Int32 = 0
 
   def new_request_id
-    @request_id = rand(2147483647)
+    @request_id = rand(REQUEST_ID_RANGE)
   end
 
   # shortcut for `.varbinds[0].oid`

src/snmp/session.cr

@@ -10,7 +10,7 @@ class SNMP::Session
   end
 
   def engine_validation_probe : V3::Message
-    raise "engine probes are not required for SNMP V2C"
+    raise SNMP::Error.new("engine probes are not required for SNMP V2C")
   end
 
   def validate(message : V3::Message)
@@ -44,24 +44,24 @@ class SNMP::Session
     SNMP::Message.new(snmp)
   end
 
-  def get(oid, request_id = rand(2147483647))
+  def get(oid, request_id = rand(REQUEST_ID_RANGE))
     SNMP::Message.new(@community, Request::Get, VarBind.new(oid), request_id)
   end
 
   # Multi-varbind Get: one GetRequest carrying every OID (RFC 3416 allows a PDU
   # to bind several variables), answered by a single Response with N varbinds.
-  def get(oids : Enumerable(String), request_id = rand(2147483647))
+  def get(oids : Enumerable(String), request_id = rand(REQUEST_ID_RANGE))
     varbinds = oids.map { |oid| VarBind.new(oid) }.to_a
     SNMP::Message.new(@community, Request::Get, varbinds, request_id)
   end
 
-  def get_next(oid, request_id = rand(2147483647))
+  def get_next(oid, request_id = rand(REQUEST_ID_RANGE))
     message = get(oid, request_id)
     message.request = Request::GetNext
     message
   end
 
-  def get_next(oids : Enumerable(String), request_id = rand(2147483647))
+  def get_next(oids : Enumerable(String), request_id = rand(REQUEST_ID_RANGE))
     message = get(oids, request_id)
     message.request = Request::GetNext
     message
@@ -70,35 +70,35 @@ class SNMP::Session
   # GetBulk (RFC 3416): retrieve up to *max_repetitions* successors for each
   # repeating varbind in one round-trip. The first *non_repeaters* OIDs are
   # treated as plain GetNext, the rest as repeaters.
-  def get_bulk(oids : Enumerable(String), non_repeaters = 0, max_repetitions = 10, request_id = rand(2147483647))
+  def get_bulk(oids : Enumerable(String), non_repeaters = 0, max_repetitions = 10, request_id = rand(REQUEST_ID_RANGE))
     varbinds = oids.map { |oid| VarBind.new(oid) }.to_a
     message = SNMP::Message.new(@community, Request::GetBulk, varbinds, request_id)
     message.non_repeaters = non_repeaters
     message.max_repetitions = max_repetitions
     message
   end
 
-  def set(oid, value, request_id = rand(2147483647))
-    SNMP::Message.new(@community, Request::Set, to_varbind(oid, value), request_id)
+  def set(oid, value, request_id = rand(REQUEST_ID_RANGE))
+    SNMP::Message.new(@community, Request::Set, VarBind.from_value(oid, value), request_id)
   end
 
   # Standard first two varbinds of an SNMPv2 notification (RFC 3416 4.2.6).
   SYS_UPTIME_OID    = "1.3.6.1.2.1.1.3.0"
   SNMP_TRAP_OID_OID = "1.3.6.1.6.3.1.1.4.1.0"
 
   # Build an SNMPv2-Trap: sysUpTime.0 + snmpTrapOID.0 followed by *varbinds*.
-  def trap_v2(oid, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(2147483647))
+  def trap_v2(oid, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(REQUEST_ID_RANGE))
     SNMP::Message.new(@community, Request::V2_Trap, notification_varbinds(oid, uptime, varbinds), request_id)
   end
 
   # Build an Inform (same shape as a v2 trap, but confirmed by the receiver).
-  def inform(oid, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(2147483647))
+  def inform(oid, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(REQUEST_ID_RANGE))
     SNMP::Message.new(@community, Request::Inform, notification_varbinds(oid, uptime, varbinds), request_id)
   end
 
   # Build an RFC 1157 SNMPv1 Trap (its own wire structure). *enterprise* is the
   # enterprise OID, *agent_address* a dotted-quad IPv4 string.
-  def trap_v1(enterprise, agent_address, generic_trap : GenericTrap, specific_trap = 0, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(2147483647))
+  def trap_v1(enterprise, agent_address, generic_trap : GenericTrap, specific_trap = 0, uptime = 0, varbinds : Array(VarBind) = [] of VarBind, request_id = rand(REQUEST_ID_RANGE))
     pdu = V1Trap.new(agent_address, generic_trap, specific_trap.to_i32,
       oid: enterprise, time_ticks: uptime.to_u32, varbinds: varbinds, request_id: request_id)
     SNMP::Message.new(@community, Request::V1_Trap, pdu, version: Version::V1)
@@ -116,36 +116,8 @@ class SNMP::Session
 
   # Multi-varbind Set: one SetRequest assigning every OID => value pair. The Hash
   # keeps insertion order, so the varbinds go out in the order they were given.
-  def set(values : Hash(String, _), request_id = rand(2147483647))
-    varbinds = values.map { |oid, value| to_varbind(oid, value) }
+  def set(values : Hash(String, _), request_id = rand(REQUEST_ID_RANGE))
+    varbinds = values.map { |oid, value| VarBind.from_value(oid, value) }
     SNMP::Message.new(@community, Request::Set, varbinds, request_id)
   end
-
-  # Encode a single OID => value assignment into a VarBind. Accepts the typed
-  # SNMP values (`TypedValue`), the Crystal primitives, a raw `ASN1::BER`, or a
-  # pre-built `VarBind`.
-  private def to_varbind(oid, value) : VarBind
-    data = value.is_a?(VarBind) ? value : VarBind.new(oid)
-
-    case value
-    when TypedValue
-      data.value = value.to_ber
-    when String
-      data.value.set_string(value)
-    when Int
-      data.value.set_integer(value)
-    when Bool
-      data.value.set_boolean(value)
-    when Nil
-      data.value.tag_number = UniversalTags::Null
-    when ASN1::BER
-      data.value = value
-    when VarBind
-      data.oid = oid
-    else
-      raise ArgumentError.new("unsupported varbind value. For complex values pass a pre-constructed `ASN1::BER`")
-    end
-
-    data
-  end
 end

src/snmp/v3/message.cr

@@ -52,9 +52,9 @@ class SNMP::V3::Message < SNMP::Message
     @pdu = @scoped_pdu.pdu
   end
 
-  def initialize(@scoped_pdu : ScopedPDU, @security_params : SecurityParams, security : Security? = nil, @security_model = SecurityModel::USM, @id = rand(2147483647))
+  def initialize(@scoped_pdu : ScopedPDU, @security_params : SecurityParams, security : Security? = nil, @security_model = SecurityModel::USM, @id = rand(REQUEST_ID_RANGE))
     @version = Version::V3
-    @max_size = 65507
+    @max_size = MAX_MESSAGE_SIZE
     if security
       @flags = security.security_level | MessageFlags::Reportable
     else
@@ -116,11 +116,15 @@ class SNMP::V3::Message < SNMP::Message
 
   def new_request_id
     @pdu.new_request_id
-    @id = rand(2147483647)
+    @id = rand(REQUEST_ID_RANGE)
   end
 
+  # Largest UDP payload (65535 - 20-byte IP - 8-byte UDP header) — advertised as
+  # msgMaxSize, the biggest response this engine will accept.
+  MAX_MESSAGE_SIZE = 65507
+
   PRIVNONE     = ASN1::BER.new.set_string("", tag: UniversalTags::OctetString)
-  MSG_MAX_SIZE = ASN1::BER.new.set_integer(65507)
+  MSG_MAX_SIZE = ASN1::BER.new.set_integer(MAX_MESSAGE_SIZE)
   MSG_VERSION  = ASN1::BER.new.set_integer(Version::V3.to_i)
 
   def verify(security, scoped_pdu = @scoped_pdu.to_ber)

src/snmp/v3/security.cr

@@ -219,20 +219,24 @@ class SNMP::V3::Security
     @digest.final
   end
 
+  # RFC 3414 A.2 expands the password to 2^20 octets, hashed in 64-byte chunks.
+  PASSKEY_EXPANSION_SIZE = 1 << 20
+  PASSKEY_CHUNK_SIZE     = 64
+
   def passkey(password)
     @digest.reset
 
-    # RFC 3414 A.2 expands the password (a sequence of octets) to 2^20 bytes by
-    # cycling through it, then hashes the stream. Feed it in a single reused
-    # 64-byte chunk indexed cyclically, instead of allocating two Strings per
-    # iteration (the old rotated/buffer concats) × 16384 iterations.
+    # Expand the password (a sequence of octets) to PASSKEY_EXPANSION_SIZE by
+    # cycling through it, then hash the stream. Feed it in a single reused chunk
+    # indexed cyclically, instead of allocating two Strings per iteration (the
+    # old rotated/buffer concats) × 16384 iterations.
     bytes = password.to_slice
     length = bytes.size
-    buffer = Bytes.new(64)
+    buffer = Bytes.new(PASSKEY_CHUNK_SIZE)
     offset = 0
 
-    (1048576 // 64).times do
-      64.times do |i|
+    (PASSKEY_EXPANSION_SIZE // PASSKEY_CHUNK_SIZE).times do
+      PASSKEY_CHUNK_SIZE.times do |i|
         buffer[i] = bytes[offset]
         offset += 1
         offset = 0 if offset == length

src/snmp/v3/security/aes.cr

@@ -36,7 +36,7 @@ class SNMP::V3::Security::AES
 
   def decrypt(encrypted_data : Bytes, salt : Bytes, engine_boots, engine_time)
     # 3.3.2.1
-    raise "invalid privacy salt received" unless (salt.size % 8).zero?
+    raise SNMP::ParseError.new("invalid privacy salt received") unless (salt.size % 8).zero?
 
     cipher = OpenSSL::Cipher.new(cipher_name)
     cipher.padding = false

src/snmp/v3/security/des.cr

@@ -37,8 +37,8 @@ class SNMP::V3::Security::DES
   end
 
   def decrypt(encrypted_data : Bytes, salt : Bytes, engine_boots = nil, engine_time = nil)
-    raise "invalid priv salt received" unless (salt.size % 8).zero?
-    raise "invalid encrypted PDU received" unless (encrypted_data.size % 8).zero?
+    raise SNMP::ParseError.new("invalid priv salt received") unless (salt.size % 8).zero?
+    raise SNMP::ParseError.new("invalid encrypted PDU received") unless (encrypted_data.size % 8).zero?
 
     cipher = OpenSSL::Cipher.new("des-cbc")
     cipher.padding = false