Skip to content

Conversation

@yanovich
Copy link
Contributor

OpenSSL PKCS12 module sets all-zeros initial vector on encryption and doesn't change it on decryption.

This patch addresses kuznyechik-ctr-acpkm-omac behavior differences in two places:

  1. in gost2015_acpkm_omac_init() IV is initialized with a random value on encryption, thus overwriting user-defined value

  2. in gost_grasshopper_cipher_init IV is initialized with a random value, thus overwriting assumed default all-zeros value

This patch also implements 3 ctrl operations required by PKCS12 module:

  • EVP_CTRL_AEAD_GET_TAG
  • EVP_CTRL_AEAD_SET_TAG
  • EVP_CTRL_AEAD_TLS1_AAD

Signed-off-by: Sergei Ianovich [email protected]

@beldmit
Copy link
Contributor

beldmit commented Sep 16, 2022

When CMS files are created, IV (and, hence, these parameters) shouldn't be zeroes.

@yanovich
Copy link
Contributor Author

Если прямо обязательно, чтобы была рандомная строка по умолчанию, то можно добавить в структуру контекста поле has_kdf_seed, а потом в gost_grasshopper_cipher_init_ctracpkm_omac() его проверять, и при отсутствии в случае зашифрования делать рандомную инициализацию (которая раньше делалась всегда, и которую я тоже убрал).

@yanovich
Copy link
Contributor Author

Просто оставить в инициализации не получается, потому что init() вызывается после set_attributes(). И получается, что в ините затирается значение, которое ранее установил пользователь.

@beldmit
Copy link
Contributor

beldmit commented Sep 16, 2022

Да, это вариант
Я не помню, что я тут делал в коммерческой реализации

@yanovich
Copy link
Contributor Author

Просто убрал своё удаление инициализации. Так тоже ключи шифруются и расшифровываются стандартной библиотекой.

gost_grasshopper_cipher_ctx_ctr *ctr = EVP_CIPHER_CTX_get_cipher_data(ctx);
if (init_zero_kdf_seed(ctr->kdf_seed) == 0)
return -1;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

этот фрагмент дублирует инициализацию. заполнение kdf_seed рандомной строкой есть в init()

@yanovich
Copy link
Contributor Author

Просто оставить в инициализации не получается, потому что init() вызывается после set_attributes(). И получается, что в ините затирается значение, которое ранее установил пользователь.

тут я оказался не прав

@yanovich
Copy link
Contributor Author

Правильно было бы использовать NID_id_tc26_cipher_gostr3412_2015_kuznyechik_ctracpkm_omac, но так не работает.

if (RAND_bytes(kdf_seed, 8) != 1)
return 0;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Так где сейчас берётся случайный IV?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unsigned char *tag = ptr;

gost_grasshopper_cipher_ctx *c = EVP_CIPHER_CTX_get_cipher_data(ctx);
if (c->c.type != GRASSHOPPER_CIPHER_MGM)
Copy link
Contributor

Choose a reason for hiding this comment

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

MGM тоже нужен. Или этого #define не сохранилось?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

этот код был под #if 0

Sergei Ianovich added 4 commits November 3, 2022 01:55
OpenSSL PKCS12 module sets all-zeros initial vector on encryption and
doesn't change it on decryption.

This patch addresses `kuznyechik-ctr-acpkm-omac` behavior differences in
two places:

1. in `gost2015_acpkm_omac_init()` IV is initialized with a random value
   on encryption, thus overwriting user-defined value

2. in `gost_grasshopper_cipher_init` IV is initialized with a random
   value, thus overwriting assumed default all-zeros value

This patch also implements 3 ctrl operations required by PKCS12 module:
- EVP_CTRL_AEAD_GET_TAG
- EVP_CTRL_AEAD_SET_TAG
- EVP_CTRL_AEAD_TLS1_AAD

Signed-off-by: Sergei Ianovich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants