Basic OS login program

Question

I have just finished the sololearn C++ course and wanted to create a basic login program in the vein of a basic operating system.

#include "ubuntu_login"
#include <iostream>
#include <string>
using namespace std;

Ubuntu_Login::Ubuntu_Login()
{
    cout << "\t Welcome to Ubuntu!\n" << endl;
}

void Ubuntu_Login::usr_login()
{

cout << "Please enter a username" << endl;
getline(cin,username);

cout << "Please enter a password" << endl;
getline(cin,password);

while(username_input != username || password_input != password)
  {
    cout << "\n|Ubuntu Login|" << endl;

    cout << "Please enter your username" << endl;
    getline(cin, username_input);

    cout << "Please enter your password" << endl;
    getline(cin, password_input);

    if(username_input != username || password_input != password)
      {
        cout << "Incorrect username or password" << endl;
      }
  }
}




#include "ubuntu_login.h"
#include <iostream>
#include <string>
using namespace std;

int main()
{
  Ubuntu_Login UO;
  UO.usr_login();
}

Header file:

#ifndef UBUNTU_LOGIN_H
#define UBUNTU_LOGIN_H
#include <string>
using namespace std;

class Ubuntu_Login
{
public:
  Ubuntu_Login();
  void usr_login();
private:
  string username;
  string password;
  string username_input;
  string password_input;
};

#endif // UBUNTU_LOGIN_H

Any help on improving my code by getting rid of useless code or replacing it with better code would be greatly appreciated.

Answer 1

Never write login code

This is much harder than you think. Leave it to the experts you will get some security piece wrong.

Code Review

Don't use this:

using namespace std;

See Why is “using namespace std;” considered bad practice?.

The reason the "standard namespace" is called std is so that it is not a big deal to type std:: before each standard object/function/type. Don't be lazy type the extra 5 characters.

I don't see declarations for username or password I hope they are not global variables.

cout << "Please enter a username" << endl;
getline(cin,username);

cout << "Please enter a password" << endl;
getline(cin,password);

You have used a while loop.

while(username_input != username || password_input != password)
{
}

You may want to look at a do while loop. As the test is not done till the end of the input code. If somebody had an empty username and password then it would match before the loop was entered and you would never get asked.

do
{
}
while(username_input != username || password_input != password);

Hint

Passwords should NEVER be stored (or encrypted). You should only ever store the hash of the password (properly salted). When a user enters a password hash it with the appropriate salt and compare it to the stored hash of the password.

A hash can be though of as one way encryption. You can encrypt it, but because it is lossy you can not decrypt the hash back to a password. So if people get hold of the your password file all they have is a set of hashes and no passwords.

Answer 2

Usability

When you enter the password, it's visible on the screen. That's unacceptable from a login program.

Class design

Why is Ubuntu_Login a class? What is the purpose of such "login object", and what can I do with it? Would I ever want to have 2 or more instances? This class doesn't seem to make much sense. A simple function could do this job.

All the data contained in the class (username, username_input, ...) are only used in the usr_login function, and as such it would be better to make them all local variables.

The job of a constructor is to initialize the state of the new object being created. Printing a welcome message doesn't belong in a constructor.

using namespace std

Others already told you to not do this again. If you want to save typing, this alternative is acceptable:

using std::cin;
using std::cout;
using std::endl;
using std::string;

Answer 3

Also, you be careful to clean out any storage that you are using. For example in your case the user name and password will be hanging around in memory probably till your program exits. If a malicious actor were to dump the process memory he could be able to recover this information.

At a minimum, you should overwrite those field with garbage data as soon as you are done with them to protect against attacks such as this.

I will second Loki's point that writing secure and correct log-in code (as well as any security related code) is very challenging.

Answer 4

1. You are not using PAM. Pluggable authentication modules allows you to change authentication backends, which allows for different security for different needs.

2. You are using passwords. Pet peeve of mine. Passwords have had security failures longer than they have been named. The problems with them certainly predate computers. Unfortunately I don't know what to recommend instead that does not involve a soldering iron.