Backport tests #60
Backport tests #60
Conversation
|
Does this actually work? There is no utils folder at all |
|
(FYI for future - just run the code before creating a pull request) |
|
|
||
| from centinel.experiment import Experiment | ||
|
|
||
|
|
There was a problem hiding this comment.
I meant for there to be 2 lines here. This is part of pep8 and I think it makes it easier to differentiate between portions of the code, i.e. imports and actual code.
There was a problem hiding this comment.
sounds good to me, but i'd rather have consistency so either add two lines everywhere or remove these.
There was a problem hiding this comment.
PEP 8 is 2 lines between imports/constants and first function/code and 2 lines between each function. I am ok with 1 line between functions but I would like 2 lines between the imports and the start of code. This is the same structure as every other file I have written for the project.
There was a problem hiding this comment.
sure i like this style -- I just wanted consistency. if this true for every file in centinel then I'm on board.
|
tls.py should be in "primitives" folder not "utils" |
ah i just saw that you re created the utils folder. nvm 👍 |
|
@gsathya, something really weird is going on. I have a utils folder, it has other stuff in it, and it is committed as part of the repo (git status shows nothing for that folder). I think I have some code left over from an older version of the repo. Did you refactor the utils directory at some point? Do you have a preference for me to use something other than utils? |
|
@gsathya, I understand now. I have a bunch of folders left after your refactor because I have .pyc and *~ files in them, which I have git set to ignore, so it won't delete those folders. Weird. |
Yes. See 0d89801 The files in utils should be moved to primitives folder |
| self.host, self.port = line[0].strip(), int(line[1].strip()) | ||
| self.fprs = [] | ||
| for entry in line[2:]: | ||
| self.fprs.append(entry.strip().lower()) |
There was a problem hiding this comment.
self.tls_fingerprints = [fpr.strip().lower() for fpr in line[2:]]
There was a problem hiding this comment.
actually make this -
self.tls_fingerprints = set([fpr.strip().lower() for fpr in line[2:]])
|
|
||
| logging.info("Getting TLS Certificate from %s on port %s " % | ||
| (self.host, self.port)) | ||
| fpr, cert = tls.get_fingerprint(self.host, self.port) |
|
@gsathya, I have fixed everything requested. merge? |
| def get_fingerprint(host, port): | ||
| cert = ssl.get_server_certificate((host, port)) | ||
| x509 = M2Crypto.X509.load_cert_string(cert, M2Crypto.X509.FORMAT_PEM) | ||
| fpr = x509.get_fingerprint('sha1') |
| cert = ssl.get_server_certificate((host, port)) | ||
| x509 = M2Crypto.X509.load_cert_string(cert, M2Crypto.X509.FORMAT_PEM) | ||
| fpr = x509.get_fingerprint('sha1') | ||
| return fpr.lower(), cert |
There was a problem hiding this comment.
i thought i added these two comments before, sorry about making you do this again
There was a problem hiding this comment.
I must have missed it. my b.
|
looks good 👍 feel free to merge it once you change fpr to fingerprint (i would also recommend you try to run it once before though) |
@gsathya, this is a pull request to backport 2 tests from the summer version of Centinel. This has already been code reviewed and it works. Will you review/merge it?