Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. I'm going to disagree with this. The number of iterations should vary depending on the computer thatthat's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as youyour machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. I'm going to disagree with this. The number of iterations should vary depending on the computer that encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as you can.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!
Okay, be warned, I know a little Java syntax, but I am really unfamiliar with the objects of Java. So if I get something incorrect, I'd appreciate a friendly reminder!
I'll see what I can point out...
So at first I notice you're supplying a static number of iterations. The number of iterations should vary depending on the computer that's encrypting. There's a good Q&A on Sec.SE that explains how you should determine the number of iterations you should use. Basically though, it boils down to as many as your machine can do without affecting performance dramatically. After you determine how many iterations suits you best, that number should be the static number.
This next one I had to do a little research for, but let's quickly note this line:
SecureRandom.getInstance("SHA1PRNG")
Here's an article (a bit old, but I believe relevant) which gives some background on this method. The author proposes that it's best to include the provider to prevent different PRNGs on different installations.
As said at the beginning, I'm not familiar with this area of Java, but it looks like you're doing what's desired, which would be encrypt then MAC. If I read your code wrong, let me know!
In regards to your third question, I think it would make more sense to destroy and then clear.
In regards to your fourth, I can't see a benefit or a downside. However, I'm not very familiar with Java vulnerabilities! So there may be some sort of leakage I'm not aware of, which could compromise the security of PBEKeySpec's password. But you clear the password with clearPassword(). My best guess would be that it's not completely necessary!
Other than that, the largest flaw I can see is the lack of formatting.
What I mean by this are things such as the two-lined try/catch:
try { SecureRandom.getInstance("SHA1PRNG").nextBytes(nextRandomSalt); }
catch(NoSuchAlgorithmException e) { throw new IllegalStateException(e); }
Compressing this into these two lines only made it harder to read. I suggest you give the code some space to breathe!
It's Java convention to have the first brace on the same line as the statement. So your two lines of
try
{
really should just be one. But that's just nit-picking!
I don't see a reason not to have the last for loop like
for (int i = 0; i < password.length; i++) {
password[i] = '0';
}
Also note that I added a space between the for and (.
The opening brace after the last catch should be on the same line as the catch!