Skip to content
This repository has been archived by the owner on Sep 16, 2022. It is now read-only.

Add checksum reporting to critical system files #412

Merged
merged 3 commits into from
Oct 3, 2019

Conversation

rptrchv
Copy link
Contributor

@rptrchv rptrchv commented Sep 18, 2019

@vpetersson
Copy link
Contributor

@rptrchv can we change the Issues section down to it's own section? These are important as they also will need to be converted into recommended action items.

@a-martynovich
Copy link
Contributor

Also as @vpetersson told us 2 weeks ago the checksum should be a tooltip hidden behind an icon, just like those (?) icons near Open Connections and Download Device Certificate, not a column in the table.

@rptrchv
Copy link
Contributor Author

rptrchv commented Sep 18, 2019

@rptrchv can we change the Issues section down to it's own section? These are important as they also will need to be converted into recommended action items.

I don't quite understand how it should look. Can you provide more detail or maybe even some hand-drawn picture?
If I add a new row "Audited files issues" below existing "Audited files" then we'll lose issues-file relation

@rptrchv
Copy link
Contributor Author

rptrchv commented Sep 18, 2019

Also as @vpetersson told us 2 weeks ago the checksum should be a tooltip hidden behind an icon, just like those (?) icons near Open Connections and Download Device Certificate, not a column in the table.

This won't allow a user to copy the checksum

@vpetersson
Copy link
Contributor

@rptrchv In order to speed things up, let's keep the structure that you had and iterate later. However, let's change it to look more like this:

65130149-d8e7a300-da0d-11e9-8dc3-95da0f72f133

Re-use the same warning icon as the other ones (I took a shortcut there)..

@rptrchv
Copy link
Contributor Author

rptrchv commented Sep 18, 2019

@vpetersson ok. what about checksum? should I leave it as is?

@vpetersson
Copy link
Contributor

@vpetersson ok. what about checksum? should I leave it as is?

Yeah leave that as is for now.

@vpetersson
Copy link
Contributor

But also, please pay attention to and make sure to use Title Case in headings.

@vpetersson
Copy link
Contributor

@rptrchv Keep in mind that I put in "OpenSSH Audit" in as a placeholder for this.

Copy link
Contributor

@a-martynovich a-martynovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rptrchv Please state what a commit does instead of "Improvements requested in the review"

if v == 'yes':
secure_value = 'no'
else:
secure_value = '2' # Support only 'Protocol' now. TODO: improve this.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs improvement indeed. Not only the "Protocol" field should be explicitly handled, but also PermitRootLogin may have other values than "yes".

@@ -25,3 +25,5 @@
IS_DEV = True

EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'

ALLOWED_HOSTS = ['*']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's needed for making it accessible from my own device (from local network) when run with docker-compose. I hope this addition won't break anything in our prod infrastructure

Roman P added 3 commits October 3, 2019 10:08
- moved 'System File Audit' section to the proper place
- added 'Configuration Audit' section with only OpenSSH issues for now
@vpetersson vpetersson merged commit 22986cf into WoTTsecurity:master Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add checksum reporting to critical system files.
3 participants