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

PasswordProtectedFiles #5067

Merged
merged 35 commits into from
Sep 23, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
56418a3
PasswordProtectedFiles
dapocalypse Jul 28, 2018
ccea176
Added changelog for passwordprotectedfiles.
dapocalypse Jul 28, 2018
5ae29af
Password Protection Fixes
dapocalypse Jul 28, 2018
19ba053
Password Protection Fixes
dapocalypse Jul 28, 2018
093914b
fixes
dapocalypse Jul 28, 2018
d06f0a5
Makes password check a proc
dapocalypse Jul 29, 2018
b609ece
Proc Password Proctection Fixes
dapocalypse Jul 29, 2018
8af142c
Fixes typo acess
dapocalypse Jul 30, 2018
121103e
Allows to encrypt or decrypt files outside of file creation
dapocalypse Jul 30, 2018
2b4bd2b
Fix to encryp decrypt stuff
dapocalypse Jul 31, 2018
45fa2d4
Merge branch 'development' into development
dapocalypse Jul 31, 2018
f1da4d8
UGh, Fixes
dapocalypse Aug 1, 2018
fb816e4
Merge branch 'development' into development
dapocalypse Aug 1, 2018
3c00491
merge
dapocalypse Aug 1, 2018
49dc58a
Merge branch 'development' into development
dapocalypse Aug 3, 2018
7c83d5f
Fixes
dapocalypse Aug 3, 2018
8faefc1
Again?
dapocalypse Aug 3, 2018
28de024
forgot
dapocalypse Aug 3, 2018
43ec5f7
Merge branch 'development' into development
dapocalypse Aug 3, 2018
1ec8efc
Merge branch 'development' into development
dapocalypse Aug 3, 2018
8218fcb
Merge branch 'development' into development
dapocalypse Aug 4, 2018
17852bb
Merge branch 'development' into development
dapocalypse Aug 5, 2018
75e5820
Merge branch 'development' into development
dapocalypse Aug 5, 2018
af8cc38
Merge branch 'development' into development
dapocalypse Aug 7, 2018
f23c03d
Merge pull request #1 from Aurorastation/development
dapocalypse Aug 8, 2018
b867b90
Merge branch 'development' into development
dapocalypse Aug 11, 2018
d789693
Merge pull request #3 from Aurorastation/development
dapocalypse Aug 27, 2018
0fb1088
Yea, fixes shit
dapocalypse Aug 27, 2018
c5eac59
Merge pull request #4 from Aurorastation/development
dapocalypse Sep 2, 2018
65faf44
Fixes USB shit
dapocalypse Sep 2, 2018
5de0a28
Merge pull request #5 from Aurorastation/development
dapocalypse Sep 15, 2018
4d5c59f
Proably not the last commit
dapocalypse Sep 15, 2018
e26199a
Merge branch 'development' into development
dapocalypse Sep 16, 2018
38d917f
Merge branch 'development' into development
dapocalypse Sep 16, 2018
79ae330
Merge branch 'development' into development
skull132 Sep 23, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion code/modules/modular_computers/file_system/computer_file.dm
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ var/global/file_uid = 0
var/size = 1 // File size in GQ. Integers only!
var/obj/item/weapon/computer_hardware/hard_drive/holder // Holder that contains this file.
var/unsendable = 0 // Whether the file may be sent to someone via NTNet transfer or other means.
var/undeletable = 0 // Whether the file may be deleted. Setting to 1 prevents deletion/renaming/etc.
var/undeletable = 0 // Whether the file may be deleted. Setting to 1 prevents deletion/renaming/etc.
var/password = "" // Placeholder for password protected files.
var/uid // UID of this file

/datum/computer_file/New()
Expand All @@ -31,6 +32,7 @@ var/global/file_uid = 0
temp.unsendable = unsendable
temp.undeletable = undeletable
temp.size = size
temp.password = password
if(rename)
temp.filename = filename + "(Copy)"
else
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[LocalizedFileNames]
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary file

file_browser.dm=@file_browser.dm,0
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,18 @@

if(href_list["PRG_openfile"])
. = 1
open_file = href_list["PRG_openfile"]
var/obj/item/weapon/computer_hardware/hard_drive/HDD = computer.hard_drive
var/datum/computer_file/file = HDD.find_file_by_name(href_list["PRG_openfile"])
var/checkpass = ""
var/currentpass = file.password
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsafe access of file.
This whole password check should be put inside ui_interact() around line 225

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I make a proc to check for the password? Also I dont understand what you mean by unsafe acess of file.

Copy link
Contributor

@skull132 skull132 Jul 29, 2018

Choose a reason for hiding this comment

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

You should make password protection validation a proc, yes. This method of doing:

		if (!currentpass)
			// do one thing
		else
			// check password and do the other

is bad. Very bad. A file should have a proc proc/can_access_file(mob/user, prompt_for_password = FALSE, input_password = "")

Rough contents would look like this:

/datum/computer_file/proc/can_access_file(mob/user, prompt_for_password = FALSE, input_password = "")
	if (!password)
		return TRUE

	if (prompt_for_password)
		if (!ismob(user))
			return FALSE

		input_password = sanitize(input(user, "Please enter a password to access file '[name]':"))

	return password == input_password

This allows for 100x cleaner functionality:

		if (!file.can_access_file(usr, TRUE))
			return 1

		open_file = href_list["PRG_openfile"]

if(!currentpass)
open_file = href_list["PRG_openfile"]
else
checkpass = sanitize(input(usr, "Please enter the password:"))
if(checkpass == currentpass)
open_file = href_list["PRG_openfile"]
else
return 1
if(href_list["PRG_newtextfile"])
. = 1
var/newname = sanitize(input(usr, "Enter file name or leave blank to cancel:", "File rename"))
Expand All @@ -28,28 +39,48 @@
var/obj/item/weapon/computer_hardware/hard_drive/HDD = computer.hard_drive
if(!HDD)
return 1
var/newpassword = sanitize(input(usr, "Enter a password or leave blank to leave unprotected:"))
var/datum/computer_file/data/F = new/datum/computer_file/data()
F.filename = newname
F.filetype = "TXT"
F.password = newpassword
Copy link
Contributor

Choose a reason for hiding this comment

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

should do if (newpassword)

HDD.store_file(F)
if(href_list["PRG_deletefile"])
. = 1
var/obj/item/weapon/computer_hardware/hard_drive/HDD = computer.hard_drive
if(!HDD)
return 1
var/datum/computer_file/file = HDD.find_file_by_name(href_list["PRG_deletefile"])
var/checkpass = ""
var/currentpass = file.password
if(!file || file.undeletable)
return 1
HDD.remove_file(file)
if(!currentpass)
Copy link
Contributor

Choose a reason for hiding this comment

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

deleting shouldn't require password

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disagree, bald greytide names steve breaks into the captains office and logs onto his computer, baldie mcsteve finds file names "realy important information i need to remember and cannot be deleted" baldie mcsteve deleted the file and runs to his greytide layer. The captain returns and finds his file has been deleted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Password protected files IRL ask for a password every time you so much as breathe on it. It requires a password when you move the file or delete the file. It makes sense that password protected files ask for a password every time you modify it.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would depend on what the actual underlying mentality of the password protection is. There's two ways to handle password protection:

  • OS level. This is what you're referring to, in that the OS enforces password protection to even breathe on the file.
  • Encrypted file. This just means that the contents of the file are encrypted and require a key to decrypt. You can move/copy/delete the file without issues, tho. Because it's just a random text file with garbled contents.

I don't really have a preference, @Arrow768 might have one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am thinking of revealing encrypted ciphertext during an emag then having the player decrypt the file.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding password required for deletion:
Most operating systems do not care weather a file is encrypted or not. The actual encryption / decryption is handled by a application interacting with the file.
The operating system only uses RBAC or something simmilar, but there are no special checks if a file is encrypted or not.

I believe the second option is our way to go in that case:
Viewing / editing requires a key (saved with the file datum)
but moving / copying / deleting does not.
This leaves room to add file owners and permissions later on.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding Baldie McSteve.
Physical Security is a important aspect.
If you allow someone else physical access to a computer system, then they can do almost anything with it.
Therefore I would allow other people to tamper with it, if they can get access.
It should be the responsibility of the player to restrict the access to the computer system accordingly or keep backups

HDD.remove_file(file)
else
checkpass = sanitize(input(usr, "Please enter the password:"))
if(checkpass == currentpass)
HDD.remove_file(file)
else
return 1
if(href_list["PRG_usbdeletefile"])
. = 1
var/obj/item/weapon/computer_hardware/hard_drive/RHDD = computer.portable_drive
Copy link
Contributor

Choose a reason for hiding this comment

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

again, this change needs to be explained.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was reverted from the previos push

if(!RHDD)
return 1
var/datum/computer_file/file = RHDD.find_file_by_name(href_list["PRG_usbdeletefile"])
var/checkpass = ""
var/currentpass = file.password
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't know if the file exists. Move this below line 76.

if(!file || file.undeletable)
return 1
RHDD.remove_file(file)
if(!currentpass)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about deleting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the thing about deleting files in flash drive as you cant find them or make them i dont think.

RHDD.remove_file(file)
else
checkpass = sanitize(input(usr, "Please enter the password:"))
if(checkpass == currentpass)
RHDD.remove_file(file)
else
return 1
if(href_list["PRG_closefile"])
. = 1
open_file = null
Expand All @@ -60,21 +91,40 @@
if(!HDD)
return 1
var/datum/computer_file/F = HDD.find_file_by_name(href_list["PRG_clone"])
var/checkpass = ""
var/currentpass = F.password
Copy link
Contributor

Choose a reason for hiding this comment

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

You're accessing F.password without checking if F actually exists. Line 97 checks if F exists and exits if it doesn't. So this has to be below it.

if(!F || !istype(F))
return 1
var/datum/computer_file/C = F.clone(1)
HDD.store_file(C)
if(!currentpass)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you opened file, you already know password, so closing it shouldn't ask password too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for cloning the file, not closing it.

HDD.store_file(C)
else
checkpass = sanitize(input(usr, "Please enter the password:"))
if(checkpass == currentpass)
HDD.store_file(C)
else
return 1
if(href_list["PRG_rename"])
. = 1
var/obj/item/weapon/computer_hardware/hard_drive/HDD = computer.hard_drive
if(!HDD)
return 1
var/datum/computer_file/file = HDD.find_file_by_name(href_list["PRG_rename"])
var/checkpass = ""
var/currentpass = file.password
Copy link
Contributor

Choose a reason for hiding this comment

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

Below line 117.

if(!file || !istype(file))
return 1
var/newname = sanitize(input(usr, "Enter new file name:", "File rename", file.filename))
if(file && newname)
file.filename = newname
if(!currentpass)
Copy link
Contributor

Choose a reason for hiding this comment

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

Renaming shouldn't need password

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Baldie McSteve will strike again.

if(file && newname)
file.filename = newname
else
checkpass = sanitize(input(usr, "Please enter the password:"))
if(checkpass == currentpass)
if(file && newname)
file.filename = newname
else
return 1
if(href_list["PRG_edit"])
. = 1
if(!open_file)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,37 @@
################################
# Example Changelog File
#
# Note: This file, and files beginning with ".", and files that don't end in ".yml" will not be read. If you change this file, you will look really dumb.
#
# Your changelog will be merged with a master changelog. (New stuff added only, and only on the date entry for the day it was merged.)
# When it is, any changes listed below will disappear.
#
# Valid Prefixes:
# bugfix
# wip (For works in progress)
# tweak
# soundadd
# sounddel
# rscadd (general adding of nice things)
# rscdel (general deleting of nice things)
# imageadd
# imagedel
# maptweak
# spellcheck (typo fixes)
# experiment
# balance
#################################

# Your name.
author: N3X15

# Optional: Remove this file after generating master changelog. Useful for PR changelogs that won't get used again.
delete-after: True

# Any changes you've made. See valid prefix list above.
# INDENT WITH TWO SPACES. NOT TABS. SPACES.
# SCREW THIS UP AND IT WON'T WORK.
# Also, all entries are changed into a single [] after a master changelog generation. Just remove the brackets when you add new entries.
# Please surround your changes in double quotes ("), as certain characters otherwise screws up compiling. The quotes will not show up in the changelog.
changes:
- rscadd: "Added a changelog editing system that should cause fewer conflicts and more accurate timestamps."
- rscdel: "Killed innocent kittens."
################################
# Example Changelog File
#
# Note: This file, and files beginning with ".", and files that don't end in ".yml" will not be read. If you change this file, you will look really dumb.
#
# Your changelog will be merged with a master changelog. (New stuff added only, and only on the date entry for the day it was merged.)
# When it is, any changes listed below will disappear.
#
# Valid Prefixes:
# bugfix
# wip (For works in progress)
# tweak
# soundadd
# sounddel
# rscadd (general adding of nice things)
# rscdel (general deleting of nice things)
# imageadd
# imagedel
# maptweak
# spellcheck (typo fixes)
# experiment
# balance
#################################

# Your name.
author: dapocalypse
Copy link
Contributor

Choose a reason for hiding this comment

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

DO NOT EDIT example.yml

Copy link
Member

Choose a reason for hiding this comment

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

It´s now a different file


# Optional: Remove this file after generating master changelog. Useful for PR changelogs that won't get used again.
delete-after: True

# Any changes you've made. See valid prefix list above.
# INDENT WITH TWO SPACES. NOT TABS. SPACES.
# SCREW THIS UP AND IT WON'T WORK.
# Also, all entries are changed into a single [] after a master changelog generation. Just remove the brackets when you add new entries.
# Please surround your changes in double quotes ("), as certain characters otherwise screws up compiling. The quotes will not show up in the changelog.
changes:
- rscadd: "Added a password protection system for files on modular computers."