http://www.perlmonks.org?node_id=886096

hakkum has asked for the wisdom of the Perl Monks concerning the following question:

Hello everyone,

I am new to both Perl and PerlMonks and have created my first script/program. This is my first post to PerkMonks as well. I am looking for a review/critique of the script. I posted it to Arch Linux's forum before discovering this Perl Monastery.

I am actually new to programming in general. I have dabbled a bit in PHP, Python, C++, and a few others, but only enough to comprehend the very basic of concepts. Perl has been quite fun to learn...the reason I've actually stuck with it.

Anyways, below is 'passgen.pl'. It is a random password generator. Of course there's plenty of these floating around, but I needed one and it seemed to be a good starting point. Running the program alone will generate a 14-character password consisting of letters, numbers, and symbols. Passing the -L -N -S options will suppress letters, numbers, and/or symbols accordingly. Also, passing -h will show a short usage summary, showing a few other options.

Please let me know what you think about both code style and usability. Thank you!

passgen.pl

#!/usr/bin/env perl use strict; use warnings; ###################################################################### +######## # # File : passgen.pl # Version : 0.10 # Author : H Bakkum (hakkum) # Email : bakkum.h (at) gmail (dot) com # History : 02-feb-2011 (hakkum) added options and created documen +tation # 27-jan-2011 (hakkum) program/script created # ###################################################################### +######## =head1 NAME passgen - generate random alphanumeric password =head1 SYNOPSIS B<passgen> [-LNSp] [-l I<length>] =head1 DESCRIPTION B<passgen> generates a random password of the specified I<length> and according to the rules specified by the command line options. If no password length is given, a default length of 14 will be generated. + If no options are provided, letters, numbers, and symbols will be used to cr +eate password. =head1 OPTIONS =over 4 =item B<-L> Prevent letters, both uppercase and lowercase, from being used during +the password generation. =item B<-N> Prevents numbers, 0 through 9, from being used during the password gen +eration. =item B<-S> Prevents the following symbols from being used during the password gen +eration: ` ~ ! @ # $ % ^ & * ( ) _ + - = \ , . / < > ? ; ' : " [ ] { } =item B<-p> Prevents "Password: " from being sent to STDOUT. This allows for pass +word to be used in a pipe. =item B<-l> I<length> Specifies the length of the generated password. =item B<-h> Print a summary of options and exit. =back =head1 AUTHOR H Bakkum, <bakkum.h (at) gmail (dot) com> =cut ###################################################################### +######## use Getopt::Std; my %cli_options; getopts("LNSl:ph", \%cli_options); USAGE: { if ($cli_options{h}) { print<<EOF_USAGE; Usage: $0 [OPTION] Generate random password using OPTION rules. -L prevent letters during generation -N prevent numbers during generation -S prevent symbols during generation -l length override default password length -p allow password to be piped -h display this help and exit Created by H Bakkum (hakkum) Report bugs to <bakkum.h (at) gmail (dot) com> EOF_USAGE exit 0; } } MAIN: { print "Password: " if not $cli_options{p}; print random_password() . "\n"; } #--------------------------------------------------------------------- +-------- # &random_password() #--------------------------------------------------------------------- +-------- sub random_password { # define variables my @chars_pool; my $pass_len; my $password; my $random_number; # characters in password @chars_pool = character_pool(); # default password length $pass_len = 14; # if command line argument, use as password length if ($cli_options{l}) { $pass_len = $cli_options{l} if $cli_options{l} =~ /^\d+$/; $pass_len = 14 if $pass_len <= 0; $pass_len = 100 if $pass_len >= 100; } # generate password for (1..$pass_len) { $random_number = int rand @chars_pool; $password .= $chars_pool[$random_number]; } $password; } #--------------------------------------------------------------------- +-------- # &character_pool() #--------------------------------------------------------------------- +-------- sub character_pool { # define variables my @strings; my @digits; my @symbols; my @chars_pool; @strings = ('a'..'z', 'A'..'Z'); @digits = (0..9); @symbols = ( '`', '~', '!', '@', '#', '$', '%', '^', '&', '*', '(', ')', '_', '+', '-', '=', '\\', ',', '.', '/', '<', '>', '?', ';', '\'', ':', '"', '[', ']', '{', '}', ); push @chars_pool, @strings if not $cli_options{L}; push @chars_pool, @digits if not $cli_options{N}; push @chars_pool, @symbols if not $cli_options{S}; @chars_pool ? @chars_pool : "-"; }

-- hakkum

...never forget the hakkum bakkum,
the hakkum bakkum never forgets...

Replies are listed 'Best First'.
Re: Please Review First Program: Random Password Generator
by toolic (Bishop) on Feb 04, 2011 at 01:17 UTC
    That's pretty impressive coding style for your first Perl script.

    Here are a few minor style points:

    • Rather than rolling my own usage, I find Pod::Usage handy.
    • A lot of people quickly out-grow Getopt::Std. Consider switching to Getopt::Long.
    • Explicit return's from your subs are customary and can make your code more readable.
    • It is common practice to declare and initialize your variables right before you use them. For example:
    # characters in password my @chars_pool = character_pool(); # default password length my $pass_len = 14; ... # generate password my $password; for (1..$pass_len) { my $random_number = int rand @chars_pool; $password .= $chars_pool[$random_number]; }

      Thanks toolic. I am pretty OCD so code cleanliness is almost required for me. I am still new to using modules and Getopt:Long seemed a bit more complicated to use...I was kind of coding in a hurry. so Getopt::Std suited me well at the time. I definitely plan to implement Getopt::Long shortly. I was unaware of Pod::Usage (and 99% of the included modules) so thank you for that; I will check it out. As for the return's, I have made the corrections; thanks again for the customary norms. I'm not too sure where I picked up on pre-declaring my variables, but you're right, it does shorten the overall length and doesn't reduce readability at all.

      I appreciate all the tips. I really want to take my time learning this stuff properly up front as opposed to having to correct ingrained bad habits later down the road.

      -- hakkum

      ...never forget the hakkum bakkum,
      the hakkum bakkum never forgets...
Re: Please Review First Program: Random Password Generator
by BrowserUk (Patriarch) on Feb 04, 2011 at 03:30 UTC

    This is not critism. Style is a very subjective thing, and more people are likely to adhere more closely to yours than mine.

    But ... what advantages do you see for your program over this which I believe to be roughly functionally equivalent? :

    #! perl -slw use strict; use List::Util qw[ shuffle ]; our( $L, $N, $S, $h, $p ); our $l //= 14; if( $h ) { print "$0 [-L -N -S -p -l=nn -h]\n", <DATA>; exit; } $l = $l <= 0 ? 14 : $l > 100 ? 100 : $l; my @chars; push @chars, 'a'..'z', 'A'..'Z' unless $L; push @chars, 0 .. 9 unless $N; push @chars, split'', q[`~!@#$%^&*()_+-=\,./<>?;':"[]{}] #' unless $S; printf "%s%s\n", $p ? '' : 'Password: ', join'', (shuffle @chars)[ 0 .. $l-1 ]; __DATA__ -h this help -p supress "Password:" -l=nn length to generate: 1 .. 100; default:14 -L supress letters (upper and lower) -N supress digits -S supress symbols `~!@#$%^&*()_+-=\,./<>?;':"[]{}

    And which do you find easier to follow?


    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    "Science is about questioning the status quo. Questioning authority".
    In the absence of evidence, opinion is indistinguishable from prejudice.

      Hey thanks for taking the time to whip up that quick rendition of my script. You are correct, I never thought of doing it that way. It has much to do with my inexperience and much to do with my lack of creativity. However, as this is my first script, I wanted to make it a bit more complicated than it needed to be. I do realize that I could have created the same functionality without the subroutines, blocks, and whatnot, but I know those are vital for future more complex scripts. I wanted to get my hands dirty a bit.

      I must say this. I like a lot of the things you have done here. Your use of split, printf, join, shuffle, and ternary are superb. I guarantee I will be refering back to this.

      I do have a question for you though. in your split function you use q[ ... ] with symbols [ and ] in there. Why does that not cause a syntax error? (I did test that and it does indeed work fine...just curious as to why/how) And what does that #' comment at the line end mean to you?

      Thanks again for the short lesson!

      -- hakkum

      ...never forget the hakkum bakkum,
      the hakkum bakkum never forgets...
        1. you use q[ ... ] with symbols [ and ] in there. Why does that not cause a syntax error?

          For the why I can only answer with a generic: because Perl's parser is (mostly :) well thought out and implemented.

          I assume that the parser must be doing a maximal munch at that point.

          The 'balanced quoting' facilities of Perl are one (of many) things that I really miss in other languages. They save so much mess & hassle with escaping stuff.

          Mind you, I abhor their overuse in Perl--eg. q[] instead of a simple ''--almost as much as I miss them when they aren't available.

        2. And what does that #' comment at the line end mean to you?

          That's simply a crutch for my editor's less capable syntax highlighting parser. It see's the single quote embedded in the construct and starts highlighting the following text as a string constant, but doesn't stop until it sees a second single quote.

          The comment prevents that erroneous highlighting from running on to subsequent lines.


        Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
        "Science is about questioning the status quo. Questioning authority".
        In the absence of evidence, opinion is indistinguishable from prejudice.
Re: Please Review First Program: Random Password Generator
by ikegami (Patriarch) on Feb 03, 2011 at 23:21 UTC

    I'd force the inclusion of a certain amount from each set. You could end up with just letters even if you allow others. This can be done as follows:

    1. Pick x letters.
    2. Pick y digits.
    3. Pick z symbols.
    4. Shuffle the picks.

      Thanks ikegami. If I understand you correctly, you mean to place each included character randomly in the character pool list first, then randomly select a position from that list (per iteration). That makes a lot of sense. I will implement that.

      -- hakkum

      ...never forget the hakkum bakkum,
      the hakkum bakkum never forgets...
        Maybe? Not sure. I mean
        use List::Util qw( shuffle ); my @passwd; push @passwd, $letters[ rand(@letters) ] for 1..$num_letters; push @passwd, $digits[ rand(@digits ) ] for 1..$num_digits; push @passwd, $symbols[ rand(@symbols) ] for 1..$num_symbols; push @passwd, $chars[ rand(@chars) ] for 1..($length-$num_letters- +$num_digits-$num_symbols); my $passwd = join '', shuffle @passwd;
      If you want a truly random password, each symbol should be chosen uniformly from the set of legal symbols. Otherwise, a cracking program can instantly eliminate all-letter passwords. Passwords work best when every possible password string is equally likely.

        If you want a truly random password

        Why would you want that? "password" is a perfectly valid password in that world.

        You assume that the cracker knows and cares that your password is truly random. That's a bad assumption to make.

        Otherwise, a cracking program can instantly eliminate all-letter passwords.

        That's exactly what my suggestion defends against. Before my change, someone who has the hashes of all alphabetic passwords could crack some of the generated passwords instantly. With my change, if configured correctly (probably x=1,y=1,z=1, but least one for each), no such fluke is possible.

        A reply falls below the community's threshold of quality. You may see it by logging in.
      I'd force the inclusion of a certain amount from each set.

      This will decrease the number of possible password combinations, and actually make password weaker. What password is stronger? The one which includes 8 characters from the [A-Z0-9] set, or one which includes 6 characters from the [A-Z] and 2 from [0-9]?

        This will decrease the number of possible password combinations,

        Yes.

        and actually make password weaker

        No.

        In general, the second does not necessarily follow from the first. Blacklisting password "password" decreases the number of possible combinations, for example.

        In this particular case, it also makes the password stronger since the hashes of all alphabetic passwords of length 14 or less are known. (I don't remember for which hashing algorithm.)

        What password is stronger? The one which includes 8 characters from the [A-Z0-9] set, or one which includes 6 characters from the [A-Z] and 2 from [0-9]?

        The one that includes 1 from [A-Z], 1 from [0-9] and 6 from [A-Z0-9].

Re: Please Review First Program: Random Password Generator
by chrestomanci (Priest) on Feb 04, 2011 at 09:57 UTC

    Where will these passwords be used?

    If the passwords are truly random and not meant to be memorised by humans, then a very simple design is possible. Something like:

    head -c 12 /dev/random | perl -e 'use MIME::Base64; print encode_base64(<>)'

    On the other hand, if these passwords are meant to be used and memorised by humans, then I think you should look a the physcology of passwords, because if you give people random passwords they will either change them to easy to remember (and easy to crack) alternatives, or they will write them down, possibly on sticky notes attached to their monitors.

    I have on my desk a copy of Security Engineering by Ross Anderson. Assuming you don't have access to a paper copy, then chaper 2, which includes a good discussion on passwords and password psychology is online.

    From that book one good suggestion I saw was to randomly generate passwords in the form of alternating constanants and vowels, eg: cVcVcVcVc. That gets you a password that has a good amount of entropy, and cannot easily be cracked as it is not in a dictonary, but at the same time is easy to remember because it can be pronounced.

      I did orignally intend to replace all my current web passwords with randomly generated passwords. However, after doing the research to create this script I did realize that it'd not be ideal to do so (unless I wanted to refer to some file containing all my passwords every time one was needed). Though I no longer needed a random generator, I still had the program idea in mind and went forth anyways. It's definitely a great learning experience.

      I guess at this point I'd only implement this to generate initial passwords for new accounts of somekind. Although, most likely I'd use a much simpler script like your suggestion or the one suggested by BrowserUk.

      Thank you for your input and reference. Though at this point, security is not a huge priority of mine, it's good to know I have a starting point when that time comes.

      -- hakkum

      ...never forget the hakkum bakkum,
      the hakkum bakkum never forgets...
Re: Please Review First Program: Random Password Generator
by roboticus (Chancellor) on Feb 04, 2011 at 11:27 UTC

    hakkum:

    Just a few minor notes:

    • You're processing a command-line argument inside of random_password. That gives you a hidden data path that can be difficult to work with in a larger program.
    • Comments like # define variables are useless, as it's quite obvious that you're doing just that. If the code is obvious, you don't need the comment. If the code isn't obvious, then usually it's better to make it obvious rather than comment it. For example:
      # default password length $pass_len = 14;
      Would be better as:
      $dflt_pass_len = 14;
    • Declaring your variables at the point of use is better practice.
    • I prefer using return expression rather than expression to return a value to the caller.

    Here's a slight update of the random_password subroutine for demonstration:

    sub random_password { my ($pass_len) = @_; my @chars_in_pass = character_pool(); my $password=''; for (1..$pass_len) { my $random_number = int rand @chars_in_pass; $password .= $chars_pool[$random_number]; } return $password; }

    I don't mean to indicate that your program is bad in any way. Overall, it's an impressive first program.

    ...roboticus

    When your only tool is a hammer, all problems look like your thumb.

      Thank you for the suggestions. I have a question however. In terms of executing random_password() without a command-line argument in it, I assume you mean to rather call it with the hash key, as in: print random_password($cli_options{l}) . "\n";. Is that correct? And if so, as opposed to using my ($pass_len) = @_; to store it, shouldn't I rather do my $pass_len = shift;? Or am I missing something?

      toolic also pointed out declaring my variables at initial use as well as explicitly using return; I thank you both for the pointers. As a side note, I am quite glad I found PerlMonks. Everyone here seems quite friendly and helpful...the perfect environment for me to grow up in.

      -- hakkum

      ...never forget the hakkum bakkum,
      the hakkum bakkum never forgets...

        hakkum:

        I assume you mean to rather call it with the hash key, as in: print random_password($cli_options{l}) . "\n";. Is that correct?

        Exactly right. In a small program it may seem awkward, but when you have a program with many bits of code that access information from anywhere, it becomes harder to understand the program, making it harder to change it. Additionally, if you decide you want to transplant a function into a new program or module, it's easier to extract it when there aren't any hidden interactions.

        And if so, as opposed to using my ($pass_len) = @_; to store it, shouldn't I rather do my $pass_len = shift;? Or am I missing something?

        Either form is just fine. In fact, until recently I *always* used shift. Normally, there's little difference between them:

        my log_msg_1 { my $FH = shift; my $msg = shift; print $FH $msg; } my log_msg_2 { my ($FH, $msg) = @_; print $FH $msg; }

        The log_msg_1 and log_msg_2 subroutines are equivalent. I prefer the second form primarily because it makes the argument list obvious, so I can remember the argument list with a brief glance at the subroutine. The fact that it makes the code a little smaller is a happy side-effect. In fact, if I could make any single change to the syntax of perl, I think it would be to allow function declarations to be of the form:

        my sub log_msg ($FH, $msg, @args) { print $FH $msg, @args; }

        be equivalent to:

        sub log_msg { my ($FH, $msg, @args) = @_; print $FH $msg, @args; }

        Having said all that, there is a *small* difference between the two forms: The version using shift alters the @_ array. This won't normally make a difference to you, but if you review perldoc perlsub, you'll see that if you call a subroutine like &subname;, then subname will receive the current @_ array as its argument list. So you can do something like this:

        #!/usr/bin/perl use strict; use warnings; use POSIX qw(strftime); printit("Foo", "bar", "baz"); log_time("Foo", "bar"); sub log_time { my $time=strftime "%H:%M:%S :", localtime; unshift @_, $time; &printit; } sub printit { my (@args) = @_; print join(" ",@args), "\n"; }

        Here, the printit subroutine just puts a space between all the arguments and prints them with a linefeed at the end[1], and the log_time function simply adds the formatted time to the start of @_ and then calls printit with the current argument list. Since shift takes the first item off the @_ array, then calling another function like &subname; will call that function with fewer arguments. If you don't expect it, it can be quite surprising. When you're aware of it, though, you can find interesting ways to use it.

        [1] I originally tried using the print function instead of printit to simplify the code, but it complained about &main::print not existing, so rather than figure out how to use the trick with print, I just made my own version of print. I suspect a more experienced/knowledgeable monk will chime in with a clarification on that point.

        ...roboticus

        When your only tool is a hammer, all problems look like your thumb.

Re: Please Review First Program: Random Password Generator
by Ratazong (Monsignor) on Feb 04, 2011 at 12:43 UTC

    Nice!

    When I got my first password at university, it was also generated by a program, and it consisted of letters and special characters. Unfortunately, the password I got contained a backslash (\) - and unfortunately there was no way for me to enter the backslash when trying to log in. Probably the login-processing interpreted it as an escape-sequence. (Using \\ didn't work either ... after some time with the admin I got the \ replaced it by an !)

    So I recommend to be careful when choosing which special characters you allow in your passwords.

    Rata

      I did initially plan on excluding that. But when I was looking around the web, I saw that most (well actually all the pages I came across) accepted backslashes in their passwords. I'm now curious: is there a place that details which symbols are definitely accepted as input (including, as ikegami stated, the symbols which are standard on all keyboards)?

      -- hakkum

      ...never forget the hakkum bakkum,
      the hakkum bakkum never forgets...
      Good point. Along those lines, one has to keep international keyboards in mind. One of my passwords has a backtick in it, and I couldn't find it on the ca-fr keyboard. (Finally found it as grave accent on a space.)
Re: Please Review First Program: Random Password Generator
by andreas1234567 (Vicar) on Feb 04, 2011 at 12:23 UTC
    It is surprisingly difficult to create a good pseudorandom number generator. There are programs that aim to test the relative "quality" (i.e. statistical characteristics) of PRNGs. If you do test your algorithm, let us know how it went!
    --
    No matter how great and destructive your problems may seem now, remember, you've probably only seen the tip of them. [1]