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

SNMPv1 support #36

Open
GhostofGoes opened this issue Feb 21, 2018 · 21 comments
Open

SNMPv1 support #36

GhostofGoes opened this issue Feb 21, 2018 · 21 comments
Labels

Comments

@GhostofGoes
Copy link
Contributor

Tried to use puresnmp against a SNMPv1 device, and was unsuccessful. After digging into the docs some more, it seems it only supports SNMPv2c currently, with v3 support planned.

However, there are still a significant number of SNMPv1 devices "out in the wild". Would you consider supporting SNMPv1, or do you consider that to be out of scope for this project?

I'd be willing to write up an implementation when I get some time.

@exhuma
Copy link
Owner

exhuma commented Feb 21, 2018

Hi,

It would not shock me to have support for SNMPv1. But implementation may need some refactoring of existing code. I don't like what I did with the external API. I am talking about the fact that you simply do:

from puresnmp import get
get(...)

The idea behind this was making it as simple as possible to use. However, this not causes problems with the other protocol versions (v1 and v3). Especially v3 takes a complete different set of arguments aside from the OIDs. So the signature or puresnmp.get would become quite messy.

I was thinking about having some kind of factory returning a "client" instance with the .get, .walk, ... methods. Something along the lines of this:

from puresnmp import Client
client = Client.create(Client.V2C, 'community')
result = client.get(ip, oid)

There are some open questions in my mind though about this:

  • Which arguments do you pass to Client.create (ip, community, timeout, ...)?
  • Is the factory method a good idea? Why not have separate classes? Or move the existing functions to separate modules and completely forego classes?
  • ...?

I have not really put much thought into it yet as SNMPv3 is planned, yes, but it is currently low-priority. All devices I am querying are in a secure and isolated network and would only suffer from the SNMPv3 overhead. So migration to v3 is not yet really planned, and as long as that's not the case I am leaving this on the backlog.

If you would like I would more than welcome a proposal for a SNMPv1 implementation!

My primary concerns about the end-result are:

  • Is the external API simple?
  • Does the external API not force any arguments on the end-user which are unnecessary?
  • Is the external API not overly repetitive. The choice of classes, factories and which arguments will be specified where would have a big impact on this point.
  • Are the in/out arguments "pythonic"?

@GhostofGoes
Copy link
Contributor Author

GhostofGoes commented Feb 22, 2018

Thanks for the quick reply.

Yeah, I see the issues with the external API. As far as I know, v1 is similar to v2 in terms of configuration (ip, oid, port, community, timeout). So to add v1 support, you could add a "version" argument, and have the public functions just call the proper version for v1 or v2.

The problem is, as you said, that doesn't work for v3 without contaminating the version-agnostic API calls with a bunch of v3-specific arguments.

The class approach would work well I think. It's a common idiom I've seen in other projects, and of course brings the benefits of using classes.

Another option I've seen is having separate sub-modules for each version. An example of this would be:

from puresnmp.v1 import get
get(ip, community, oid)

from puresnmp.v2 import get
get(ip, community, oid)

It's somewhat ugly, but it does have the advantage of providing the same API with only minor changes to the imports. Also, multiple versions could still be used in the same program, it would just require extra imports for each version:

import puresnmp.v1 as v1
import puresnmp.v2 as v2
import puresnmp.v3 as v3
v3.get(ip, community, oid, user=username, key=key)  # Silly contrived example of v3

A little more work, but still infinitely better than PySNMP IMO.

@exhuma
Copy link
Owner

exhuma commented Mar 18, 2018

I'm currently drowning a bit in work on other projects. I will look into this in a few weeks time.

Considering the notes on the overall API design you might also want to look at #3. I never received any replies on my last comments and the PR has run stale.

Maybe we could merge both efforts into a more useful API... even if it's not backwards compatible. This is where semver comes in handy :)

@exhuma
Copy link
Owner

exhuma commented Jan 13, 2019

I've started refactoring the API in the past few days and looked at this issue again. It seems that all that is needed to support SNMPv1 is to change the version number in the header. At least that is all that I could find. If that's really all that is needed, it should be trivial to implement.

I don't know if this is still something you need one year after you initially added this issue, but I will see to implement it nonetheless. The new architecture should make this feasible.

@etingof
Copy link

etingof commented Jan 13, 2019

I think there are a couple of other nuisances one might want to keep in mind when implementing dual v1/v2c support. Notably:

  • v2c hasCounter64 type
  • v2c misses NetworkAddress type
  • v1 has unconstrained INTEGER type, while v2c has is as a 32-bit signed int
  • v1 has completely unique TRAP PDU while v2c reuses command PDU layout

May be something else, take a look at RFC3584 for complete information on that matter.

@exhuma
Copy link
Owner

exhuma commented Jan 14, 2019

Thanks for the link @etingof. I'll have a look. It seems that most implementation difference would lie in the "set" command (by simply refusing unsupported types), and, as you mention, traps.

Traps are currently not yet supported, but I realise that this would be good to have a possibility to receive those as well.

@etingof
Copy link

etingof commented Jan 14, 2019

It seems that most implementation difference would lie in the "set" command (by simply refusing unsupported types)

More or less, modulo slightly different NetworkAddress PDU structure.

On the other hand, it may not be convenient for the clients to use differing library calls when talking to v1 or v2c agent....

In the RFC linked above the designers suggested building a mediation layer (called proxy) which would convert differing types/layouts so that the caller would always deal with the same abstractions regardless of the underlying protocol version used.

@exhuma
Copy link
Owner

exhuma commented Jan 14, 2019

Yes. But that's one of the premises of this library anyway. To transparently convert between Python and SNMP types wherever possible. For set commands this is unfortunately not 100% transparent as the client must specify which type should be sent to the agent. Especially in cases where one Python type maps to multiple SNMP types.

@GhostofGoes
Copy link
Contributor Author

GhostofGoes commented Jan 15, 2019

v1 support would definitely still be useful for my work. However, I just haven't had the time, energy, or access to v1 devices at home to implement anything, apologies.

@exhuma
Copy link
Owner

exhuma commented Jan 15, 2019

@GhostofGoes no worries. I'm in the same boat, which explains the delayed reply 😃

I have pushed back the refactoring for too long now because I did not want to break backwards compatibility too soon. But I am motivated to go ahead with it now that the library has been battle-tested in our internal infrastructure for quite some time.

I might be able to to a transitional release which still supports the old API with added deprecation warnings, so it should be actually possible to avoid breaking too many things.

And given that the differences between v1 and v2 seem to be fairly small this shouldn't take too long. I just need to read through the RFCs first.

@etingof
Copy link

etingof commented Jan 15, 2019

I just haven't had the time, energy, or access to v1 devices at home to implement anything

JFYI: you could experiment/test against the publicly available SNMP agent simulator.

@exhuma
Copy link
Owner

exhuma commented Jan 15, 2019

@etingof That is awesome. Thanks for letting me know. I was unaware of this!

@revmischa
Copy link
Contributor

I would really love the ability to make a v1 query

@exhuma
Copy link
Owner

exhuma commented Jul 20, 2020

The latest release (v1.9.0) should have some support for SNMPv1, via #77 (thanks @revmischa)

@exhuma
Copy link
Owner

exhuma commented Jul 20, 2020

I am planning to follow RFC-3411 (see #84) for the next major release, which should make it much cleaner to support SNMPv1.

This RFC outlines an architecutre for SNMP applications with different message processing models (f.ex. SNMPv1 and SNMPv2). Following that RFC, supporting SNMPv1 would only need an implementation of that message processing model.

I will leave this issue open as a way to keep an eye on SNMPv1 support.

Given that SNMPv1 and SNMPv2 are fairly similar it should be possible to reuse much of the SNMPv2 code (which is already evident from #77).

@AnnaLupanova
Copy link

Please explain how to use the library with snmp version 1???

@multilan-tarek
Copy link

Please explain how to use the library with snmp version 1???

from puresnmp import get
from puresnmp.const import Version

print(get("10.0.20.4", "public", "1.3.6.1.4.1.318.1.1.1.1.1.1.0", version=Version.V1))

See the "version" attribute

@exhuma exhuma added the v1.x label May 19, 2022
@exhuma
Copy link
Owner

exhuma commented May 19, 2022

A new version of puresnmp has been released today. Support for SNMPv1 is still experimental but should be working for basic requests. To enable SNMPv1, you have to pass in a V1() credential instance into the client.

Let me know if this works for you.

@GhostofGoes
Copy link
Contributor Author

Wow, thanks for implementing this!

I'll definitely give it a whirl if I get some time in the next week or two. Would love to be able to use puresnmp again instead of pySNMP 😄 (not to imply that pySNMP is bad, it's just way too complicated for my relatively simple use cases).

@exhuma
Copy link
Owner

exhuma commented May 20, 2022

This was incidentally what led me to implement this. To make it easy to use. The new versions is ever so slightly more complicated because I decided to introduce the "Client" and "PyWrapper" classes. But I feel confident that it is still an acceptable compromise between ease-of-use, proper decoupling and flexibility.

If you find something that feels "unnatural", cumbersome or annoying, let me know and I will see if I can come up with an improvment.

@GhostofGoes
Copy link
Contributor Author

I was able to test this change against a v1 device with a few OIDs that return simple strings.

The wrong bit was being set for the version, but that was a simple fix (the PR #102 is literally a single-character change). It seems to be working fine now for simple OIDs.

I'll continue to test next week (if I have time) with more complex data types like tables.

ziesemer added a commit to ziesemer/puresnmp that referenced this issue Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants