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

Encrypt error when associated query with same column name. #3352

Closed
KomachiSion opened this issue Oct 24, 2019 · 12 comments · Fixed by #6112
Closed

Encrypt error when associated query with same column name. #3352

KomachiSion opened this issue Oct 24, 2019 · 12 comments · Fixed by #6112

Comments

@KomachiSion
Copy link
Member

Bug Report

Which version of ShardingSphere did you use?

4.0.0-RC3-SNAPSHOT

Which project did you use? Sharding-JDBC or Sharding-Proxy?

Sharding-JDBC

Expected behavior

associated query and decrypt successully.

Actual behavior

Exception in thread "main" javax.crypto.IllegalBlockSizeException: Input length must be multiple of 16 when decrypting with padded cipher
	at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:936)
	at com.sun.crypto.provider.CipherCore.doFinal(CipherCore.java:847)
	at com.sun.crypto.provider.AESCipher.engineDoFinal(AESCipher.java:446)
	at javax.crypto.Cipher.doFinal(Cipher.java:2165)
	at org.apache.shardingsphere.core.strategy.encrypt.impl.AESShardingEncryptor.decrypt(AESShardingEncryptor.java:72)
	at org.apache.shardingsphere.core.execute.sql.execute.result.StreamQueryResult.decrypt(StreamQueryResult.java:164)
	at org.apache.shardingsphere.core.execute.sql.execute.result.StreamQueryResult.getValue(StreamQueryResult.java:72)
	at org.apache.shardingsphere.core.merge.dql.common.StreamMergedResult.getValue(StreamMergedResult.java:49)
	at org.apache.shardingsphere.shardingjdbc.jdbc.core.resultset.EncryptResultSet.getString(EncryptResultSet.java:181)
	at org.apache.shardingsphere.example.core.jdbc.repository.OrderItemRepositoryImpl.getOrderItems(OrderItemRepositoryImpl.java:114)
	at org.apache.shardingsphere.example.core.jdbc.repository.OrderItemRepositoryImpl.selectAll(OrderItemRepositoryImpl.java:101)
	at org.apache.shardingsphere.example.core.jdbc.service.OrderServiceImpl.printData(OrderServiceImpl.java:152)
	at org.apache.shardingsphere.example.core.jdbc.service.OrderServiceImpl.processSuccess(OrderServiceImpl.java:95)
	at org.apache.shardingsphere.example.core.api.ExampleExecuteTemplate.run(ExampleExecuteTemplate.java:29)
	at org.apache.shardingsphere.example.orchestration.raw.jdbc.JavaConfigurationExampleMain.main(JavaConfigurationExampleMain.java:60)

Reason analyze (If you can)

the join tables has same column name, the column in first table is encrypted and in second not encrypted, but encryptor will try to decrypt the second table's column.

Steps to reproduce the behavior, such as: SQL to execute, sharding rule configuration, when exception occur etc.

  • encrypt configuration:
    private EncryptRuleConfiguration getEncryptRuleConfiguration() {
        EncryptRuleConfiguration result = new EncryptRuleConfiguration();
        Properties properties = new Properties();
        properties.setProperty("aes.key.value", "123456");
        EncryptorRuleConfiguration aesRuleConfiguration = new EncryptorRuleConfiguration("aes", properties);
        EncryptColumnRuleConfiguration columnConfigAes = new EncryptColumnRuleConfiguration("", "status", "", "status_encryptor");
        Map<String, EncryptColumnRuleConfiguration> columns = new HashMap<>();
        EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration(columns);
        columns.put("status", columnConfigAes);
        tableConfig.getColumns().putAll(columns);
        result.getEncryptors().put("status_encryptor", aesRuleConfiguration);
        result.getTables().put("t_order", tableConfig);
        return result;
    }
  • table structure:
CREATE TABLE IF NOT EXISTS t_order (
  order_id   BIGINT NOT NULL AUTO_INCREMENT,
  user_id    INT    NOT NULL,
  address_id BIGINT NOT NULL,
  status     VARCHAR(50),
  PRIMARY KEY (order_id)
)

CREATE TABLE IF NOT EXISTS t_order_item (
  order_item_id BIGINT NOT NULL AUTO_INCREMENT,
  order_id      BIGINT NOT NULL,
  user_id       INT    NOT NULL,
  status        VARCHAR(50),
  PRIMARY KEY (order_item_id)
)
  • sql:
SELECT i.* FROM t_order o, t_order_item i WHERE o.order_id = i.order_id

Example codes for reproduce this issue (such as a github link).

https://github.com/apache/incubator-shardingsphere-example

module orchestration-raw-jdbc-example

@sunbufu
Copy link
Contributor

sunbufu commented Oct 26, 2019

Hi @KomachiSion , I think this issue is same like #3194. The root cause of this issue is that we can't get tableName by columnIndex correctly.

    /**
     * Get sharding encryptor.
     * 
     * @param columnIndex column index
     * @return sharding encryptor
     * @throws SQLException SQL exception
     */
    public Optional<ShardingEncryptor> getShardingEncryptor(final int columnIndex) throws SQLException {
        final String actualColumn = resultSetMetaData.getColumnName(columnIndex);
        if (null == sqlStatementContext) {
            return Optional.absent();
        }
        final Collection<String> tableNames = sqlStatementContext.getTablesContext().getTableNames();
        for (String each : tableNames) {
            Optional<ShardingEncryptor> result = findShardingEncryptorWithTable(actualColumn, each);
            if (result.isPresent()) {
                return result;
            }
        }
        return Optional.absent();
    }

The complete code is here. You can see SS try to get encryptor with all tableNames in sqlStatementContext, so we can't get encryptor correctly by same column name.
I think maybe we should discuss the issue until get a perfect solution.

@KomachiSion
Copy link
Member Author

Yes, there are no owner table information. JDBC resultSetMetadata do not contain, so we may get the owner table from parsingEngine.
But for i.*, o.* kind type, parsingEngine may be hard to judgement how many columns each table owns.

@sunbufu
Copy link
Contributor

sunbufu commented Oct 28, 2019

Can we just replace * with all column of table ?

@KomachiSion
Copy link
Member Author

Which step should we replace?
I think it's hard to replace * with all columns.

If the we can get TableMetaData, encryptor may calculate the owner table.

@sunbufu
Copy link
Contributor

sunbufu commented Oct 28, 2019

I have a crazy idea, maybe it's unfeasible.
We can generate aliases for columns in DQL like Hibernate.
If user send an sql like SELECT order_id from t_order, we generate an alias for order_id, like t_order_order_id_123 and send the new sql SELECT order_id AS t_order_order_id_123 from t_order to DB. We can exactly get owner table by column name, Because alias is we generated.
In this case, we can replace i.* with order_item_id AS t_order_item_order_item_id_, order_id AS t_order_item_order_id_, user_id AS t_order_item_user_id_, status AS t_order_item_status_.

@KomachiSion
Copy link
Member Author

There are two questions for your idea.

  1. We need to do much work to make sure the rewrite SQL is correct. Hibernate can do this because it generate SQL from JavaBean not SQL. But SS focus on SQL, SS need to parse logicSQL, and then rewrite.
  2. Even we do rewrite for SQL, how to deal with this kind of situation that logic SQL has origin column alias like select i.status as i_status, o.status as o_status from t_order i, t_order_item o?

@sunbufu
Copy link
Contributor

sunbufu commented Oct 29, 2019

  1. Yes, I known this idea is difficult, and maybe it's unfeasible.
  2. I think we can replace user's alias with we generated alias.

@ilemonchn
Copy link

Yes, there are no owner table information. JDBC resultSetMetadata do not contain, so we may get the owner table from parsingEngine.
But for i.*, o.* kind type, parsingEngine may be hard to judgement how many columns each table owns.

Why we can not get the column's owner tableName from resultSetMetaData?
String getTableName(int column) throws SQLException;
I debuged and find this works. @KomachiSion

@KomachiSion
Copy link
Member Author

Yes, there are no owner table information. JDBC resultSetMetadata do not contain, so we may get the owner table from parsingEngine.
But for i.*, o.* kind type, parsingEngine may be hard to judgement how many columns each table owns.

Why we can not get the column's owner tableName from resultSetMetaData?
String getTableName(int column) throws SQLException;
I debuged and find this works. @KomachiSion

It's sound good, I will check it. Thanks.

@sunbufu
Copy link
Contributor

sunbufu commented Nov 25, 2019

Yes, there are no owner table information. JDBC resultSetMetadata do not contain, so we may get the owner table from parsingEngine.
But for i.*, o.* kind type, parsingEngine may be hard to judgement how many columns each table owns.

Why we can not get the column's owner tableName from resultSetMetaData?
String getTableName(int column) throws SQLException;
I debuged and find this works. @KomachiSion

I think you should debug it with oracle driver again.

@big-mountain-z
Copy link
Contributor

I try it with mysql driver. In mysql's ResultSetMetaData, we can find infomation for column's owner tableName(aliasTableName) and originalTableName. But I think this way is not standard and universal.

java.sql.ResultSetMetaData

/**
* Gets the designated column's table name.
*
* @param column the first column is 1, the second is 2, ...
* @return table name or "" if not applicable
* @exception SQLException if a database access error occurs
*/
String getTableName(int column) throws SQLException;

Method getTableName may return a empty String. In this way, if we parse the original sql to relate the aliasTableName and originalTableName, in same time, we can get the relation of tableName and column by DatabaseMetaData. Maybe we can resolve it.

@tristaZero
Copy link
Contributor

@big-mountain-z @sunbufu @KomachiSion
Hi everyone, I fixed this issue in #6112. However, I am not sure it is what you expected. So could anyone help give a test on our master branch?
Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants