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

SNMPv3 AES CFB data padding #430

Closed
Flexikon opened this issue Nov 29, 2022 · 1 comment
Closed

SNMPv3 AES CFB data padding #430

Flexikon opened this issue Nov 29, 2022 · 1 comment

Comments

@Flexikon
Copy link

In /proto/secmod/rfc3826/priv/aes.py the clear text message is padded if the data is not a modulus of 16 bytes. For AES CFB used in SNMPv3 USM, this is not needed.

RFC3826 [3.1.3] Data Encryption, states:
The plaintext is divided into 128-bit blocks. The last block may
have fewer than 128 bits, and no padding is required.

This becomes a problem when the data recipient parses ASN1 strictly and the padding is not expected.

last bit of decrypted packet data with padding:
06 01 02 01 01 01 00 05 00 00

decrypted hex data without padding:
06 01 02 01 01 01 00 05 00

Debug logs only show the correct length output because the padding is applied after debug message generation and thus this wasn´t found until taking a closer look at packet captures.

Commenting out line 121 and 151 in aes.py:
Line 121, dataToEncrypt = dataToEncrypt + univ.OctetString((0,) * (16 - len(dataToEncrypt) % 16)).asOctets()
Line 151, encryptedData = encryptedData + univ.OctetString((0,) * (16 - len(encryptedData) % 16)).asOctets()

Seems to work in my tests and perhaps this can be helpful for someone else!

@lextm
Copy link

lextm commented Nov 30, 2022

According to the comments "PyCrypto seems to require padding", it seems that the paddings were added by the author when the underlying dependency PyCrypto has a serious bug. PySNMP has moved to PyCryptodome a while ago, so not sure why the paddings were not removed.

If you like to help more people, you can send a pull request to my fork, https://github.com/lextudio/pysnmp so that I can ship it in a new release.

BTW, wonder if you can also close this issue, so that #429 can remain at top to warn future readers that this repo is no longer maintained (original author Ilya passed away) and I am taking over the maintenance work.

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

No branches or pull requests

2 participants