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

make this workshop more interactive #7

Open
jsms90 opened this issue Aug 2, 2017 · 19 comments
Open

make this workshop more interactive #7

jsms90 opened this issue Aug 2, 2017 · 19 comments

Comments

@jsms90
Copy link
Contributor

jsms90 commented Aug 2, 2017

Since you can complete the exercises without understanding the information above, for me at least, I don't know how much of the reading I would remember. Especially if I struggled with the exercise itself at all.

The reading that students have to do before the exercises is really really clear 👍 It's a really nice introduction to some potentially confusing concepts, and I don't think it will take students long to read through it before they get to the exercises. So maybe it just depends what kind of learner you are as to whether you take information in this way.

But we have seen this pattern before, where students have a Q&A with mentors after going through some reading material (the intro to HTTP requests & APIs from week 3) or a lot of reading and then a couple of exercises at the bottom (the error handling workshop from week 5).

The progress that students make and the feedback on how these went seem to suggest that something more practical is preferable. See the issue raised about the error handling workshop and FAC11's stop go continue from week 5. (FAC11's stop go continue from week 3 still to be uploaded).

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 3, 2017

@ronanyeah I really believe that making the workshop more interactive would help the concepts sink in. Right now, I imagine that the students might use the bcryptjs docs to look up "async hash" and just copy this code without properly understanding what it is doing.

But I haven't submitted a PR because this suggestion is heavily reliant on having a pre-existing database #6. Considering the fact that we're not in agreement over that being a good idea, I thought I would explain exactly what my suggestion is, before actually working on making the changes.

For this to work, we would need to add a database with a users table, containing usernames and passwords stored in plain text. We would also need tests for each new step.


Here is an outline for a more practical, hands-on version:

Set the scene

  • You've just joined a project with a pre-existing database of users
  • The previous devs were using some very bad practices, in terms of password storage.
  • Your task is to rectify the security flaw by protecting the passwords.

Hashing and salting before storage

  • 1st (mini) task:

    • query the database for the contents of the users table (or just the password column) - use result to point out to students that the previous devs were storing passwords in plain text
  • Readme explains

  • 2nd task:

    • query the database for the contents of the users table (here, point out that the previous devs were storing passwords in plain text & explain why that's a bad idea)
    • create a hashPassword function that uses crypto to convert a plain text password to a hashed password
    • run acceptance test (pre-written within the workshop) on hashPassword function - to make sure students have written it correctly
    • update database with newly hashed password instead of plain text version
  • 3rd (mini) task:

    • students query the database for the passwords of 2 users (we specify 2 users, pre-written into the database, who happen to have chosen the same password)
  • Readme

    • uses the result to point out to students that identical passwords produce identical hashes.
      Then explain what look up table & rainbow table attacks are (link to these raindbow tables)

      In fact, huge databases of pre-computed hashes of the most common passwords already exist. These are known as 'rainbow tables'. 6.5 million LinkedIn passwords were hacked in 2012. While they were hashed, they were not 'salted' and were therefore eventually all cracked.

    • explains what a salt is

      This is where you add something known as a 'salt'. A salt is a long string of random bytes, added to the password before hashing, to alter the resulting hash.

    • explains how it helps to protect against these types of attack Missing explainations of key terms / terms that might need more explanation #9

  • 4th task:

    • generate a salt i.e. using crypto to generate a random string (explain importance of string length) -
      (hash with a fixed salt)
    • add the salt into the hashPassword function that you wrote earlier
    • run tests (pre-written within the workshop) on hashPassword function to make sure students have written it correctly
    • update database with newly salted passwords instead of plain hashed version
  • Readme explains

    • how you would compare a user inputted password to the (salted & hashed) version in the database
      i.e. need to store the salt in the database so this can be used when comparing

      the salt would be stored in your database or in an environment variable

    • the security risk of a fixed salt (once your security has been breached, having a single salt used on all passwords makes it much faster to then crack the passwords)

  • 5th task:

    • put const randomString = crypto.randomBytes(12).toString('hex'); into a new function called generateSalt, which takes one parameter (hashed password) & returns a password that has been hashed and salted
    • change hashPassword function so that it uses generateSalt
    • run (pre-written) acceptance test on hashPassword function
    • update database with newly salted passwords instead of plain hashed version
  • Readme explains

    • how the salt would be stored differently

      You create the salt, create the hash, then store both of them in the database together to be used when a user tries to log in.

    • reinforce why generating a new salt each time is better

      This means that even in the event of an attacker getting a database dump, each password would have to be brute forced individually.

Using a password hashing algorithm

Password hashing functions

  • Readme explains what's happening under the hood of bcrypt

    • adding a secret key to the hash - can be encrypted using a cipher like AES (crypto has cipher methods), or the secret key can be included in the hash using a keyed hash algorithm e.g. HMAC
    • what key-stretching is & why we do it - making password cracking slower
    • explain the concept of a work factor ("rounds") on a hash function & why 10 is the default
  • 1st task:

    • change the hashPassword function so that it uses bcrypt instead of crypto
    • run pre-written acceptance test on new hashPassword
    • write your own test for your hashPassword function - change the input parameter (the password that the user inputted) and therefore the expected value
  • 2nd task:

    • use bcrypt's create a function called comparePasswords which takes two strings and returns a true/false boolean (asychronously!)
    • run pre-written acceptance test
    • write your own test

What do you think @ronanyeah @des-des @eliasCodes @sohilpandya @bo-bok @alexis-l8 ?

@ronanyeah
Copy link
Member

+1 to more excercises, still definitely against a database

@ronanyeah
Copy link
Member

-1 to 'setting scenes'

@ronanyeah
Copy link
Member

The reading that students have to do before the exercises is really really clear

That's exactly what I wanted. If there is confusion, then the readme can be read again.

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 3, 2017

@ronanyeah

If there is confusion, then the readme can be read again.

Reading about cryptography isn't nearly as engaging as it would be to interact with actual data. Surely the whole point of FAC vs regular education is that being told something isn't half as memorable as discovering something by doing it yourself. So why not make it more practical and less reading-heavy?

still definitely against a database

How come? They've just finished database week. Why would we not want to use the context they've just been given? Why would we not help that to solidify in their minds by making it assumed knowledge within future workshops?

@ronanyeah
Copy link
Member

So why not make it more practical and less reading-heavy

Sure, we could add more (2 max, and small) hashing exercises.

They've just finished database week.

Yeah and they will have a project at the end of the week putting all of these new skills to work with a database. If they have these skills. So lets focus on putting those skills in their hands. This is not a database usage workshop. So a database is a distraction and unnecessary intellectual overhead.

@ronanyeah
Copy link
Member

Why would we not want to use the context they've just been given?

They also know how to use github pages, should we slot that in here somewhere?

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 3, 2017

Hahahaha. It's not about using things they know for the hell of it 🙃

New information is easier to absorb when it is contextualised. The entire reason you want to hash a password is so that it is more secure when you then store it. Using a database for a password security workshop isn't purely about applying prior knowledge. It is directly relevant to whole purpose of learning to hash & salt passwords.

If you introduce concepts without providing the context in which they are used, even if the concept is understood, it is more of an intellectual overhead for someone to then provide the context for themselves.

@ronanyeah
Copy link
Member

The entire reason you want to hash a password is so that it is more secure when you then store it.

Nope, you could be running a microservice that sends the hashed password off to another microservice that handles your users. So thats an AJAX request, and there are no databases to be seen. Or you could be writing a hashing utils module for yourself which will quite rightly have no idea where it is being called from, or that it's results are going into a database at some point.

And by the way, if they are writing their handlers properly, there shouldn't be ANY files that share database and bcrypt code, but now they are writing an unnecessary module which has the cost of totally obfuscating the FOCUS of the workshop, and the skills required for it to be completed.

What I mean is, this workshop automatically becomes less reusable and inclusive to outsiders hopping in and giving it a go if it requires the 'external state' of having completed previous 'weeks' and having certain totally arbitrary programs installed on their computers. (people who have an interest in security perhaps, but hate databases) Not to mention how less bothered people will be to repeat it!

@saralk spoke to me in Gaza about the possibility of the FAC workshops, and even weeks being self-contained, and more module-like. It's something I think about a lot. The curriculum should be like Lego. The PROJECTS are where it comes together.

@ronanyeah
Copy link
Member

2019 Curriculum Meeting:

'OK that's it settled, FAC is now adopting DonkeyDB.'

'Aw that's annoying we've hardcoded Postgres all over the curriculum.'

@sohilpandya
Copy link
Member

Sorry I haven’t read all of this. Lots of back and forth but I have read your top comment and suggestion.

I am with @ronanyeah on this one. We should not have databases as part of this. It should just be about hashing and crypto. IMO you can’t make everything interactive and a deep dive. Hashing is just necessary there isn’t anything pretty about it but it’s necessary.

We need them to think for themselves and the database part can be left to the side as it is not the focus.

Also setting a scene can lead to the thinking that this is the only way it is done and so if they come across a different implementation they may be confused too.

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 4, 2017

In order to complete the exercises, all they need to do is go directly to this part of the docs, copy the lines there & wrap those few lines in a function which takes a callback.

The only thing that might make this exercise take longer than 15 mins is if they:

  • don't fully understand callbacks
  • don't understand exactly what is being asked of them i.e. how hashPassword fits into the bigger picture of building an app

They can just copy the lines and then implement it in their projects without understanding or remembering the conceptual information in the readme above.

If we make each of the sections above more interactive, they're more likely to retain the information long-term.

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 4, 2017

@ronanyeah This is a good point though:

'Aw that's annoying we've hardcoded Postgres all over the curriculum.'

If you don't want any of the weeks to rely on each other (in this case to rely on students knowing postgres) then @des-des' PR to insert a mock database #10 is a way to solve that.

But even if you don't put this into context with a database at all, you can make the workshop more interactive just by typing the crpyto lines into the terminal and looking at the output. The workshop then just becomes more of a terminal-based code along. It would then be up to the mentor to explain what each crypto line is doing & explain all the concepts verbally.

This could actually work really well. But I think the minute you make it more mentor-led (e.g. code alongs / readme Q&As), you put the mentor in the position where students are going to ask them lots of conceptual questions. It puts pressure on the mentor to know this topic inside out, in order to deliver it well.

I'm not convinced that that is the best move here, considering the fact that none of the mentors in any of our 3 campuses had this week delivered to them. They have to spend even more time than usual trying to get their head around the material.

@ronanyeah
Copy link
Member

ronanyeah commented Aug 4, 2017

But even if you don't put this into context with a database at all, you can make the workshop more interactive just by typing the crypto lines into the terminal and looking at the output.

Good suggestion.

Right so we can do with more interactivity. If there's time some can be added, but like Sohil said:

IMO you can’t make everything interactive and a deep dive.

Sometimes you just have to read a paragraph to learn what a rainbow table is.

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 5, 2017

@alexis-l8 is delivering the portion before the exercise as a presentation (talking through the concepts in the readme) / "code-along" (demo-ing the crypto lines in the terminal). Could you share the slides here please Alexis?

We should probably record this, to help Nazareth & Gaza. @sohilpandya can we count on your camera again please? 🙏

@jsms90
Copy link
Contributor Author

jsms90 commented Aug 6, 2017

@des-des were you making a PR for this?

@m4v15
Copy link

m4v15 commented Dec 5, 2018

hey so I added a workshop to follow this and put stuff in context, if any of you still care a year later, cos I think @jsms90 was right:

https://github.com/m4v15/ws-password-hashing

@m4v15
Copy link

m4v15 commented Dec 5, 2018

actually tbf I think both of you were kind of right, hence why I think when we do this in Khalil (we have a campus in the west bank now btw) we will do this workshop first and then go on to my one (which is implementing the stuff here)

@m4v15
Copy link

m4v15 commented Dec 9, 2018

Update after delivery:

  • We did the presentation/readme of this workshop, and then did the exercises of the other workshop (https://github.com/m4v15/ws-password-hashing)
  • Went well, definitely wasn't too much "intellectual overhead" which I was also concerned about
  • people seemed to get all of the learning objectives
  • total time (presention + exercises) ~ 1hr 30m:
    • presentation was about 30 minutes
    • spent quite a lot of time asking questions and casting during the exercises - all is documented in the mentor-notes.md of that repo.

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

4 participants