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

비밀번호 암호화 방법 개선 #889

Closed
kijin opened this issue Aug 8, 2014 · 7 comments
Closed

비밀번호 암호화 방법 개선 #889

kijin opened this issue Aug 8, 2014 · 7 comments

Comments

@kijin
Copy link
Contributor

kijin commented Aug 8, 2014

안녕하세요? 약 6년째 XE를 사용하고 있는 프로그래머입니다.

전에도 공식사이트 게시판에서 몇 차례 이와 관련된 논의가 있었던 것으로 기억하는데, 현재 XE의 비밀번호 암호화 방식은 md5($password) 또는 md5(sha1(md5($password))) 둘 중 하나를 선택하도록 되어 있어서 개인정보보호법 기준에 맞지 않는 것은 물론, 만약 DB가 유출될 경우 레인보우 테이블을 사용하여 쉽게 비밀번호를 유추할 수 있는 형편입니다.

예전에는 PHP 4 호환성을 위해 어쩔 수가 없었지만, XE 1.7부터는 반드시 PHP 5.2.4 이상을 사용하도록 하고 있기 때문에 더이상 예전 방식을 유지할 필요가 없다고 생각합니다. 요즘 뜨는 bcrypt까지는 아니더라도, 국제 표준인 PBKDF2 (HMAC-SHA256) 알고리듬 정도는 PHP 5.2.4에서도 충분히 사용이 가능하니까요. 그것조차 지원되지 않는 서버라면 최소한 솔트 정도는 넣어야겠죠.

그래서 좀더 다양한 비밀번호 암호화 방법들을 선택할 수 있도록 제가 member 모듈을 좀 수정해 보려고 합니다. 비슷한 작업을 몇 차례 해본 경험이 있거든요.

현재 계획은 다음과 같습니다.

  1. 관리모듈의 설정 > 일반 > 고급 카테고리에 옵션을 추가하여 비밀번호 암호화 알고리듬을 선택할 수 있도록 하고, 설정값은 다른 설정값들과 같이 db.config.php에 저장되도록 합니다. member 모듈에서 기존의 $useSha1 변수가 수행하던 기능은 이것으로 대체합니다.
  2. 선택 가능한 알고리듬은 md5(), md5(sha1(md5())), MySQL의password()old_password()` (여기까지는 기존에 사용하던 알고리듬들), SHA256, SHA512, MD5+솔트, SHA1+솔트, SHA256+솔트, SHA512+솔트, PBKDF2, 그리고 PHP 5.3.7 이상인 경우 bcrypt로 합니다.
  3. member.model.php를 수정하여 위와 같은 알고리듬으로 암호화된 비번도 체크할 수 있도록 합니다.
  4. 설정된 알고리듬과 다른 방식으로 암호화된 비번이 발견될 경우 자동으로 다시 암호화하도록 합니다. (현재 $useSha1을 사용할 경우 다시 암호화하는 방식과 유사합니다. 단, 다른 프로그램과의 호환성을 위해 일부러 예전 알고리듬을 사용하기를 원하는 경우 다시 암호화하지 않도록 하는 옵션을 추가하는 것도 고려해 보아야겠습니다.)
  5. member.controller.php를 수정하여 신규 회원가입, 회원정보 변경, 비번 변경, 비번 리셋 등의 경우에도 위에서 설정한 것과 동일한 알고리듬을 사용하도록 합니다.
  6. member 테이블의 password 필드는 현재 60자밖에 되지 않아 SHA512, PBKDF2 등 일부 알고리듬의 해시값을 저장하기에 충분하지 않습니다. 이것을 250자로 늘려서 추후 어떤 알고리듬이라도 넉넉하게 사용할 수 있도록 합니다.
  7. 위에서 중복되는 코드가 많이 발생할 경우 해당 부분을 별도의 클래스로 빼냅니다. classes/security 폴더에 Password.class.php라는 파일을 새로 만들어도 될까요?

모듈이나 애드온으로 만들어 배포하지 왜? 라고 생각하시는 분도 있겠지만, 회원의 개인정보 보호는 워낙 중요한 문제인데다가 비번 암호화는 원래 XE Core에서 담당하는 부분이기 때문에 XE Core 자체를 수정하지 않으면 개선하기 어렵습니다. XE 마켓에서 회원관리 확장 기능을 제공하는 모듈들을 봐도, 절대 다수가 member 모듈을 상속받아 사용할 뿐, 비번 암호화 관련 기능을 직접 구현하지는 않고 있습니다.

비번 암호화 방법 개선에 더하여 아래와 같은 기능도 구현할 수 있겠으나, 일단 중요한 것이 아니고 다른 모듈과의 호환성이 더 복잡해질 수 있으므로 미루어 두겠습니다.

  1. 설치할 때의 서버 환경을 분석하여 현재 서버가 지원하는 가장 강력한 암호화 알고리듬을 자동으로 선택합니다.
  2. 게시판 등 다른 모듈에서도 강력한 암호화 알고리듬을 사용할 수 있도록 합니다.

우선 작업을 해서 며칠 안에 pull request를 넣도록 하겠습니다.

건드리는 부분이 많다 보니 당장 XE Core에 포함시키기에는 어렵겠지만, XE 1.8에는 강력한 비밀번호 암호화 기능이 꼭 포함되었으면 좋겠습니다. 코어 개발자 분들의 꼼꼼한 검사와 꾸지람, 많은 테스트를 부탁드리며, 제가 적은 것보다 더 좋은 아이디어가 있으시다면 얼마든지 댓글 달아 주세요.

성기진 @kijin 올림

@ghost
Copy link

ghost commented Aug 8, 2014

사실 보안이 굉장히 중요한 요소중 하나이기 때문에 이런 이런 움직임은 굉장히 가치있다고 생각합니다.
풀리퀘, 기다리고있겠습니다.

@kijin
Copy link
Contributor Author

kijin commented Aug 8, 2014

며칠까지 걸릴 것도 없을 것 같습니다. XE가 워낙 깔끔하게 디자인되어 있어서, 몇 군데만 고치면 나머지는 모두 따라오네요.

위의 1~7번에 해당하는 작업은 일단 마쳤습니다. 관리 페이지에서 설정 하나만 바꿔주면 기존에 md5(), sha1(), md5(sha1(md5())), MySQL PASSWORD() 등으로 암호화된 비번도 정상 인식되고, 새로 가입하거나 비번을 변경하는 경우 새로 설정한 알고리듬을 사용하며, 기존 알고리듬을 사용하던 회원들도 다음번 로그인할 때 새 알고리듬으로 자동 변환됩니다.

현재 진행 상황은 https://github.com/kijin/xe-core/tree/feature/pwhash 에서 보실 수 있습니다.

아직 몇 가지 부족한 부분이 있어 풀리퀘를 넣지 않고 있습니다.

  • member 테이블 스키마의 password 필드를 250자로 늘리긴 했는데, 모듈 업데이트를 하지 않으면 이게 자동으로 적용되지는 않겠지요? 이 부분은 커미터 분들의 도움이 필요합니다. 60자 필드로는 sha256조차 감당이 안됩니다 ㅠㅠ (bcrypt와 pbkdf2는 됩니다. 특히 pbkdf2는 뒤가 몇글자 잘려버려도 괜찮은 도마뱀같은 속성이 있어요.)
  • 기존 비번 암호화 알고리듬과의 호환성을 위해 config/func.inc.php 파일에 이미 있는 mysql_pre4_hash_password() 함수를 활용하려고 했는데, 이게 실제 MySQL의 OLD_PASSWORD() 함수와는 전혀 다른 결과를 내놓네요. 해당소스 blame을 해보니 XE 코딩 스타일에 맞게 여러 번 고친 흔적이 있는데, 언젠가 연산자 하나가 삐뚤어진 것 같습니다. 일단 제대로 된 함수를 구해서 새로 만든 classes/security/Password.class.php에 포함시켰습니다.
  • 제 개발환경은 PHP 5.3, 5.4, 5.5밖에 없어서 PHP 5.2에서 테스트해주실 분이 필요합니다. PHP 5.2에서 실행할 경우 bcrypt는 아예 옵션으로 주어지지도 않습니다만, pbkdf2를 비롯한 나머지 옵션들은 정상 작동해야 합니다.

@eondcom
Copy link
Contributor

eondcom commented Aug 8, 2014

보안 기능만 잘돼도 XE를 선택하고 사용하는 사용자들이 늘어날 것 같네요.

@kijin
Copy link
Contributor Author

kijin commented Aug 9, 2014

어제 아직 부족하다고 말씀드렸던 부분들에 대한 업데이트입니다.

password 필드 60자 제한 문제

우선 60자의 제한 안에서도 문제없이 다양한 암호화 알고리듬들을 사용할 수 있도록 하였습니다. 예를 들어 sha256의 경우 해시값이 64바이트이지만, 60바이트에서 잘려버린 경우에는 처음 60바이트만 비교하여 통과시켜 주는 식입니다. (잘리지 않은 경우에는 끝까지 비교합니다.)

물론 이 상태로 계속 사용한다면 최고의 보안을 누릴 수는 없겠지만, 가장 강력한 보안을 제공하는 pbkdf2와 bcrypt는 60자 이내에서도 사용이 가능하므로 sha256을 최대한 활용하지 못한다고 해서 크게 아쉬운 것은 없습니다. 어차피 솔트가 없는 알고리듬들은 다 거기서 거기입니다.

OLD_PASSWORD() 함수 에뮬레이션 문제

기존 소스의 mysql_pre4_hash_password() 함수는 32비트 환경에서만 올바른 결과를 반환하는군요. 반면, 제가 Password.class.php에 새로 추가한 함수는 64비트 환경에서만 제대로 동작하네요. 아무래도 정수의 비트들을 직접 조작하는 알고리듬이다 보니 이런 한계를 가진 듯 합니다.

일단 서버의 비트수에 따라 올바른 함수를 선택하도록 조치하였습니다. 사실 구 제로보드 회원DB를 그대로 가져온 경우를 제외하면 OLD_PASSWORD() 함수를 사용하는 경우는 드물기 때문에, 이렇게 약간의 비효율을 감수하는 것도 큰 문제는 아니라고 봅니다. 기존 소스가 이 함수로 암호화된 비번을 지원하고 있기 때문에 새 버전에서도 마찬가지로 100% 지원해야겠다고 생각한 것 뿐입니다.

PHP 5.2 환경 테스트 부족

가상서버에서 CentOS 5.10, PHP 5.2.17 환경을 구축하여 테스트해본 결과, bcrypt를 제외한 모든 알고리듬들이 정상 동작하는 것을 확인하였습니다.

주말인데다 휴가철이라 다른 개발자분들이 별로 계시지 않아 다소 아쉽습니다만, 조금 더 테스트해 보고 pull request를 넣도록 하겠습니다. 더 고쳐야 할 부분이 있다면 일단 풀리퀘 넣고 나서 고쳐도 되겠지요.

@kijin
Copy link
Contributor Author

kijin commented Aug 10, 2014

다른 모듈들과의 호환성 점검 결과입니다.

회원 비번 암호화 알고리듬만 개선하고, 게시판이나 방명록의 글·댓글 등에 비회원이 입력한 비번에는 어떠한 부작용도 일으키지 않는 것이 목적입니다.

문서 (document) 및 댓글(comment) 모듈 : 비회원이 쓴 글·댓글의 비번을 저장할 때는 md5()를 사용하도록 하드코딩되어 있으므로 제가 수정한 부분의 영향을 받지 않습니다. 그러나 이렇게 저장된 비번을 확인하는 것은 글·댓글을 사용하는 각 모듈의 몫입니다.

게시판 (board) 모듈 : 저장할 때는 문서와 댓글 모듈을 그대로 사용하므로 md5()가 됩니다. 확인할 때는 $oMemberModel->isValidPassword() 메소드를 호출합니다. 이 함수는 제가 많이 고쳤기 때문에 영향을 받을 소지가 있습니다.

그러나 제가 고친 $oMemberModel->isValidPassword() 함수는 현재의 비번 암호화 설정과 무관하게 기존에 md5()로 암호화된 결과도 항상 체크가 가능하도록 되어 있고, 글·댓글 등의 비번을 확인할 때는 $member_srlnull로 주어지므로 비번 저장 알고리듬을 강제로 업그레이드하려고 시도하지도 않습니다. 따라서 기존에 저장된 글·댓글의 비번을 확인할 때도 100% 호환되고, 비회원이 새로 작성하는 글·댓글도 100% 호환됩니다.

위키 (wiki), 지식인 (kin), 포럼 (forum), 카페 (cafe) 모듈 : 게시판과 동일합니다.

방명록 (guestbook) 모듈 : 글을 저장할 때와 확인할 때 모두 md5()를 사용하도록 하드코딩되어 있습니다. 따라서 제가 수정한 부분의 영향을 받지 않습니다.

텍스타일 (textyle) 모듈 : 게시판과 동일합니다. 업글타일 (upgletype) 등 텍스타일 기반으로 만들어진 다른 모듈들도 마찬가지입니다. 단, 방명록 기능은 md5()를 사용하도록 하드코딩되어 있으므로 전혀 영향을 받지 않습니다.

다른 분들이 만든 모듈 : XE 자료실에서 아약스보드 (ajaxboard), xiso 게시판 확장모듈, xiso 회원관리 확장모듈, 닉네임 변경 관리모듈, 누리고 비번찾기모듈, 로그인 기록모듈 등을 확인해 보았습니다. 모두 기존의 게시판이나 회원관리 모듈을 확장할 뿐, 비번 암호화와 관련된 부분을 직접 건드리지는 않으므로 문제 없습니다.

비번 확인시에는 자체적으로 처리하지 않고 $oMemberModel->isValidPassword() 메소드를 호출하는 경우가 대부분이어서 이 메소드만 확실하게 호환성을 유지해 주면 문제가 발생할 일은 없어 보입니다. 그러나 각 모듈에서 비번을 암호화할 때는 md5()로 하드코딩되어 있는 부분이 상당히 많습니다. XE Core에서 비번 확인 메소드는 제공하지만 비번 암호화 메소드는 별도로 제공하지 않는 점이 이런 현상을 불러일으키고 있다고 생각됩니다.

@kijin
Copy link
Contributor Author

kijin commented Aug 10, 2014

풀리퀘 넣었습니다. 변경한 부분에 대한 자세한 설명도 함께 적었습니다. #894

@akasima
Copy link
Member

akasima commented Aug 13, 2014

#894 에서 처리하고 이슈는 닫도록 하겠습니다.

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

No branches or pull requests

3 participants