Skip to content

Commit

Permalink
8312306: Add more Reference.reachabilityFence() calls to the security…
Browse files Browse the repository at this point in the history
… classes using Cleaner

Reviewed-by: ascarpino
  • Loading branch information
Valerie Peng committed Aug 31, 2023
1 parent 351c31e commit 2436fb0
Show file tree
Hide file tree
Showing 6 changed files with 234 additions and 89 deletions.
67 changes: 44 additions & 23 deletions src/java.base/share/classes/com/sun/crypto/provider/DESKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ final class DESKey implements SecretKey {
public byte[] getEncoded() {
// Return a copy of the key, rather than a reference,
// so that the key data cannot be modified from outside

// The key is zeroized by finalize()
// The reachability fence ensures finalize() isn't called early
byte[] result = key.clone();
Reference.reachabilityFence(this);
return result;
try {
return key.clone();
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}

public String getAlgorithm() {
Expand All @@ -111,25 +111,35 @@ public String getFormat() {
*/
@Override
public int hashCode() {
return Arrays.hashCode(this.key) ^ "des".hashCode();
try {
return Arrays.hashCode(this.key) ^ "des".hashCode();
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}

@Override
public boolean equals(Object obj) {
if (this == obj)
return true;

if (!(obj instanceof SecretKey that))
return false;

String thatAlg = that.getAlgorithm();
if (!(thatAlg.equalsIgnoreCase("DES")))
return false;

byte[] thatKey = that.getEncoded();
boolean ret = MessageDigest.isEqual(this.key, thatKey);
java.util.Arrays.fill(thatKey, (byte)0x00);
return ret;
try {
if (this == obj)
return true;

if (!(obj instanceof SecretKey that))
return false;

String thatAlg = that.getAlgorithm();
if (!(thatAlg.equalsIgnoreCase("DES")))
return false;

byte[] thatKey = that.getEncoded();
boolean ret = MessageDigest.isEqual(this.key, thatKey);
java.util.Arrays.fill(thatKey, (byte)0x00);
return ret;
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}

/**
Expand All @@ -141,7 +151,13 @@ private void readObject(java.io.ObjectInputStream s)
throws java.io.IOException, ClassNotFoundException
{
s.defaultReadObject();
key = key.clone();
byte[] temp = key;
key = temp.clone();
Arrays.fill(temp, (byte)0x00);
// Use the cleaner to zero the key when no longer referenced
final byte[] k = this.key;
CleanerFactory.cleaner().register(this,
() -> java.util.Arrays.fill(k, (byte)0x00));
}

/**
Expand All @@ -154,9 +170,14 @@ private void readObject(java.io.ObjectInputStream s)
*/
@java.io.Serial
private Object writeReplace() throws java.io.ObjectStreamException {
return new KeyRep(KeyRep.Type.SECRET,
try {
return new KeyRep(KeyRep.Type.SECRET,
getAlgorithm(),
getFormat(),
key);
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}
}
74 changes: 48 additions & 26 deletions src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,12 @@ final class DESedeKey implements SecretKey {
}

public byte[] getEncoded() {
// The key is zeroized by finalize()
// The reachability fence ensures finalize() isn't called early
byte[] result = key.clone();
Reference.reachabilityFence(this);
return result;
try {
return key.clone();
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}

public String getAlgorithm() {
Expand All @@ -110,26 +111,36 @@ public String getFormat() {
*/
@Override
public int hashCode() {
return Arrays.hashCode(this.key) ^ "desede".hashCode();
try {
return Arrays.hashCode(this.key) ^ "desede".hashCode();
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}

@Override
public boolean equals(Object obj) {
if (this == obj)
return true;

if (!(obj instanceof SecretKey that))
return false;

String thatAlg = that.getAlgorithm();
if (!(thatAlg.equalsIgnoreCase("DESede"))
&& !(thatAlg.equalsIgnoreCase("TripleDES")))
return false;

byte[] thatKey = that.getEncoded();
boolean ret = MessageDigest.isEqual(this.key, thatKey);
java.util.Arrays.fill(thatKey, (byte)0x00);
return ret;
try {
if (this == obj)
return true;

if (!(obj instanceof SecretKey that))
return false;

String thatAlg = that.getAlgorithm();
if (!(thatAlg.equalsIgnoreCase("DESede"))
&& !(thatAlg.equalsIgnoreCase("TripleDES")))
return false;

byte[] thatKey = that.getEncoded();
boolean ret = MessageDigest.isEqual(this.key, thatKey);
java.util.Arrays.fill(thatKey, (byte)0x00);
return ret;
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}

/**
Expand All @@ -141,7 +152,13 @@ private void readObject(java.io.ObjectInputStream s)
throws java.io.IOException, ClassNotFoundException
{
s.defaultReadObject();
key = key.clone();
byte[] temp = key;
this.key = temp.clone();
java.util.Arrays.fill(temp, (byte)0x00);
// Use the cleaner to zero the key when no longer referenced
final byte[] k = this.key;
CleanerFactory.cleaner().register(this,
() -> java.util.Arrays.fill(k, (byte)0x00));
}

/**
Expand All @@ -154,9 +171,14 @@ private void readObject(java.io.ObjectInputStream s)
*/
@java.io.Serial
private Object writeReplace() throws java.io.ObjectStreamException {
return new KeyRep(KeyRep.Type.SECRET,
getAlgorithm(),
getFormat(),
key);
try {
return new KeyRep(KeyRep.Type.SECRET,
getAlgorithm(),
getFormat(),
key);
} finally {
// prevent this from being cleaned for the above block
Reference.reachabilityFence(this);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -129,7 +129,7 @@ byte[] protect(PrivateKey key)
SecretKey sKey = null;
PBEWithMD5AndTripleDESCipher cipher;
try {
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false);
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
// encrypt private key
cipher = new PBEWithMD5AndTripleDESCipher();
cipher.engineInit(Cipher.ENCRYPT_MODE, sKey, pbeSpec, null);
Expand Down Expand Up @@ -193,7 +193,7 @@ Key recover(EncryptedPrivateKeyInfo encrInfo)

// create PBE key from password
PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password);
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false);
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
pbeKeySpec.clearPassword();

// decrypt private key
Expand Down Expand Up @@ -339,7 +339,7 @@ SealedObject seal(Key key)
SecretKey sKey = null;
Cipher cipher;
try {
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false);
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
pbeKeySpec.clearPassword();

// seal key
Expand All @@ -366,8 +366,7 @@ Key unseal(SealedObject so, int maxLength)
try {
// create PBE key from password
PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password);
sKey = new PBEKey(pbeKeySpec,
"PBEWithMD5AndTripleDES", false);
sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
pbeKeySpec.clearPassword();

SealedObjectForKeyProtector soForKeyProtector = null;
Expand Down

1 comment on commit 2436fb0

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.