scapy icon indicating copy to clipboard operation
scapy copied to clipboard

add bmp option

Open eldadcool opened this issue 1 year ago • 4 comments

This PR is a suggestion for a fix to #4536.

All I did is added the BMP_STRING format to the relevant type

eldadcool avatar Sep 20 '24 08:09 eldadcool

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.69%. Comparing base (8e08cbf) to head (eb78e88).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4537      +/-   ##
==========================================
- Coverage   81.62%   78.69%   -2.94%     
==========================================
  Files         358      333      -25     
  Lines       85652    80454    -5198     
==========================================
- Hits        69915    63313    -6602     
- Misses      15737    17141    +1404     
Files with missing lines Coverage Δ
scapy/layers/x509.py 97.79% <ø> (-0.01%) :arrow_down:

... and 284 files with indirect coverage changes

codecov[bot] avatar Sep 20 '24 08:09 codecov[bot]

Thanks for the fix! I guess that a test should also be added, using a pattern similar to https://github.com/secdev/scapy/blob/master/test/scapy/layers/x509.uts#L53

I managed to decrease the size of the certificate to 480 bytes using the following script:

from scapy.all import *

load_layer("tls")

c = Cert("example_cert.pem")

del(c.tbsCertificate.serialNumber)
del(c.tbsCertificate.signature)
del(c.tbsCertificate.subject)
del(c.tbsCertificate.validity)
del(c.tbsCertificate.subjectPublicKeyInfo)
del(c.tbsCertificate.extensions)
del(c.x509Cert.signatureAlgorithm)
del(c.x509Cert.signatureValue)

open("example_cert.new.der", "wb").write(raw(c.x509Cert))
c = Cert("example_cert.new.pem")
c.x509Cert.show()

guedou avatar Sep 21 '24 09:09 guedou

Thanks for the fix! I guess that a test should also be added, using a pattern similar to https://github.com/secdev/scapy/blob/master/test/scapy/layers/x509.uts#L53

I tried to add a test but encounter a few difficulties:

  1. How can I run the tests? just running pytest does not work and I did not found anything in the docs... (Maybe it's because I'm in windows?)
  2. I can write a very explicit check like:
cert_with_bmp_string = base64.b64decode('MIIHKjCCB...Or8iY=')
c = X509_Cert(cert_with_bmp_string)
bmp_field_value = str(c.tbsCertificate.issuer[7].rdn[0].value.val, "utf-16be")
assert bmp_field_value == '[email protected]'

But I really don't like it. So I tried to use the function get_issuer_str but the output was a mess because of the encoding:

c.tbsCertificate.get_issuer_str()
'/C=US/ST=CA/L=LG/O=Websense, Inc./OU=Websense Endpoint/CN=Websense Public Primary Certificate Authority/description=\x001\x002\x004\x006\x001\x008\x003\x005\x001\x004\x00E\x00P\x00@\x00w\x00e\x00b\x00s\x00e\x00n\x00s\x00e\x00.\x00c\x00o\x00m/[email protected]'

This might be something that require deeper modification of the code. This happens because any non utf8 encoding is replaced with the backslash notation using the decode function

x.decode(errors="backslashreplace")

So, in calculation:

  • I think the decoding of ASN1 strings should be done with a method and every type of string can replace the encoding method.
  • After that fix I suggest writing the test as follow:
cert_with_bmp_string = base64.b64decode('MIIHKjCCB...Or8iY=')
c = X509_Cert(cert_with_bmp_string)
assert '[email protected]' in c.tbsCertificate.get_issuer_str()

@guedou, What do you think of my suggestion? should I include it in this PR?

eldadcool avatar Sep 24 '24 07:09 eldadcool

You can test Scapy with UTScapy as described in https://scapy.readthedocs.io/en/latest/development.html#testing-with-utscapy. It is simpler to launch the tests using tox (see https://scapy.readthedocs.io/en/latest/development.html#using-tox-to-test-scapy) as it will take care of many things for you.

I think that your first test is quite fine, and that's what we usually do.

guedou avatar Sep 25 '24 17:09 guedou

test was added and rebased on the updated master

eldadcool avatar Nov 05 '24 10:11 eldadcool