쏚쏚스포이드

최근 팀원과 프로젝트 진행 중 배포 후 구글폼과 함께 배포된 사이트를 피드백받고자 홍보를 했었다.

홍보 후 들어왔던 피드백중 하나였는데,

“검색창에 select를 치면 모든 게시글이 나와요.“

라는 피드백이였고, 너무 놀란 나머지 최대한 빨리 원인분석을 해보았다. 원인분석 이후 도달한 결론은 “문제없음” 이였다.

이유는 우리의 injection방어로직은 아래와 같은데,

    /**
     * SQL Injection 방어하기 위하여 구현
     */
    private String changeNoSqlInjection(String query) {
        query = SPECIAL_CHARS.matcher(query).replaceAll("");
        for (String s : STRING_SET) {
            if (query.contains(s)) {
                return "";
            }
        }
        return query;
    }

해당 코드에서는 sql문인 select, update, insert, delete의 문자가 들어오면 무조건 빈 문자열을 리턴하기 때문에 “select”를 포함한 문자열을 검색했을 시 빈 문자열로 검색을 돌렸기에 위와 같은 상태가 되었던 것이었기에 문제가 없는 것이다.

하지만 이번 일로 인해 프로젝트를 진행하면서 중요하게 놓치고 있는 것을 다시한번 깨닫게 되었는데, 이유는 아래와 같다.

• 내가 팀원이 구현한 해당 sql Injection관련 코드를 봤었는데 그냥 그렇구나~ 하고 가볍게 넘긴 것
• 해당 기능이 구현된 후 직접 테스트해보지 않은 것

평소에 코드리뷰를 하려고 pull request 후에 리뷰, approve하는 형식인데 어느순간부터 아무 생각없이 approve만 하는 습관이 생기는 것 같다 물론 내가 바쁜 것도 바쁜 것이지만, 상대방의 코드를 통해 배울 점도, 부족한 점이 있다면 서로 피드백을 하면서 확인하는 것이 내가 성장하기에 가장 좋을텐데 간과하고 있었던 것이 이렇게 돌아올줄이야… 또, SQL injection이라면 기본적이면서도, 중요한 문제인데 이를 구현된 코드만 보고 가볍게 넘어가고 테스트해보지 않은 것이 문제가 있다고 생각했고, 더 신경써서 테스트를 해봐야 한다고 다시한번 느꼈다.

스프링 시큐리티 공식문서를 참고해서 정리해본 시큐리티 관련 내용

인증(비밀번호)

Password Storage :: Spring Security

스프링 시큐리티에 있는 passwordEncoder인터페이스는 비밀번호의 단방향 변환의 기능을 갖고있다.

유저데이터의 비밀번호 보안 역사

제일 처음 유저데이터의 정보는 평문으로 저장했었다. why? : 유저데이터는 DB에 들어가있으니 DB 어드민이 털리지 않는 이상 문제 없을거라 판단.

이후 악의적인 사용자는 SQL Injection같은 공격을 통해 사용자 이름과 비밀번호가 포함된 데이터를 얻을 방법을 찾아냈고, 이러한 방식으로 데이터의 보안이 뚫리기 시작하자 새로운 암호화 방식인 단방향해시 SHA-256을 통해 정보를 해시화하고 해당 해시를 저장하도록 권장되었다.

이러한 방식은 사용자가 인증을 시도하면 해시된 비밀번호가 입력한 비밀번호의 해시와 비교해서 일치한다면 로그인이 되는데, 이러한 암호화를 뚫기 위해 Rainbow Tables라는 조회 테이블이 생겼고 해당 테이블은 비밀번호의 입력값을 미리 해시화해서 테이블에 넣어두고 역인코딩으로 값을 받아온다

때문에 해당 방식으로도 값을 뚫리게 되고 이를 막기위해 개발자는 소금을 치기 시작한다(특정하지 않은 랜덤한 문자열을 해시문에 포함시킴) 이러한 형식으로 Rainbow Tables 조회테이블에 당하지 않는게 현재 상황이다.

이러한 보안상의 이슈를 스프링 시큐리티에서 자체적으로 지원을 해주는데, 추가로 생각해야 하는 것이 비밀번호 검증을 할 때 의도적으로 리소스(CPU, 메모리, 등등)를 많이 사용하는것이다.

비밀번호를 확인하는데 1초의 지연시간을 주는 이유는 공격자가 암호를 무차별확인을 하면서 해독할 수 있는 시간을 주지 않는것이 주요 목표이다.

때문에 스프링 시큐리티 인증 설정을 할 때 해당 부분을 잘 신경써서 같이 구현해야 할 것이다.

또 위와같이 데이터를 찾는데 시간을 주지않기 위해 장기자격증명이 아닌 단기자격증명으로 지속체크를 하는 것이 좋을 것이다.

장기자격증명 → 사용자 이름 및 비밀번호

단기자격증명 → 세션, Oauth토큰

User.UserBuilder users = User.withDefaultPasswordEncoder();

or 

UserBuilder users = User.withDefaultPasswordEncoder();
UserDetails user = users
  .username("user")
  .password("password")
  .roles("USER")
  .build();
UserDetails admin = users
  .username("admin")
  .password("password")
  .roles("USER","ADMIN")
  .build();

기본 Deprecated되어있는데 이유는 메모리와 컴파일된 소스코드에 비밀번호가 노출되어있기 때문에 해당 비밀번호를 외부에서 해시를 해야 안전해짐

때문에 사용x

위의 사진은 스프링 시큐리티 클래스의 User클래스 내부 코드중 일부인데 이런식으로 생성자를 통해서도 가입이 가능한듯?

패스워드 인코더는 자바 자체에서 지원하는데,

• bcrypt
• ldap
• MD4
• MD5
• noop
• pbkdf2
• scrypt
• SHA-1
• SHA-256
• sha256

그냥 이런게 있구나하고 인지하면될듯

주요한 것은 스프링시큐리티에 있는

• BCryptPasswordEncoder
• Argon2PasswordEncoder
• Pbkdf2PasswordEncoder
• SCryptPasswordEncoder

이 네가지 인코더인데 상황이나 여건에 따라서 네가지중 하나를 사용하면서 비밀번호를 확인하는 시간을 약 1초가 걸리도록 지연하게 하는것이 안전하다

이유는 레인보우테이블 공격을 통해 무차별 인코딩확인을 막기 위해.

PasswordEncoder 또한 지원을 하긴 하는데 현재는 안전하지 않다는 것이 검증되었다. 지원하는 이유는 이전 버전에서 만들어진 코드는 해당 인코더를 사용했었기 때문에

만들어진 비밀번호를 변경하는 방법은

http
    .passwordManagement(Customizer.withDefaults())

http
    .passwordManagement((management) -> management
        .changePasswordPage("/update-password")
    )

위의 방식으로 사용하면 될 것 같음

추가로

약관에 동의하더라도 개인정보는 평문으로 저장하면 안된다

이 때문에 스프링 시큐리티의 도움을 반드시 받는것이 중요할 것 같다.

혹시나 서비스할 때 항상 잊지않고 생각해야하는 부분

비밀번호 해시 관련 지식참조 사이트

https://starplatina.tistory.com/entry/비밀번호-해시에-소금치기-바르게-쓰기

@RequiredArgsConstructor를 처음엔

그냥 생성자를 알아서 주입해주는구나~ 생각만 가볍게 해두고 공식처럼 사용해왔다.

발단

그렇게 스프링부트 프로젝트를 협업을 통해 진행한지 좀 지났을 때

리팩토링을 위해 코드를 개선하다가 문제가 발생했다.

하나의 도메인에서 다른Repository를 불러오는 것이 응집도를 올린다고 생각했고, 또 Repository에 직접 접근하는 것은 막아야 한다고 생각했기에 xxxxRepository를 그대로 써오는 것에서 xxxxService로 변경해 사용하는 것으로 바꿔 생각했다.

이 과정에서 우리는 Service부분의 클래스 생성을 xxxxRepository → xxxxService로 변경, 생성자 주입을 시켰는데 문제가 발생했다..!

결론부터 말하자면 @RequiredArgsConstructor의 특성을 제대로 생각하지 못한 것이 문제였고 자세한 이유는 아래에서 다루겠다.

그래서

처음에 원인 파악을 할 때 해당 클래스들의 생성자 주입에서 final이 빠졌다고 인지를 아예 못 한 상황이였고, 때문에 한참을 뻘짓 한 것 같다.

그 때 당시에 코드변경 후 직접 실행해 확인하지 않고, 바로 테스트코드를 작성하고 있어서 해당 UserService 자체가 문제라고 인지하지 못했고,

테스트코드 작성도 익숙치 않은 상태라 테스트코드를 못짜서 그렇다고 판단을 했고, 테스트 환경을 좀 더 공부하는 뻘짓(?)을 했고, 해당 뻘짓을 할 때 디버그를 통해 알아낸 정보는 아래의 사진과 같았다.

Service에 있는 클래스가 모두 null로 되어있는 것… 이걸 발견하고도 한참 뻘짓하기도 하고 기초중의 기초적인 실수였지만....

그래도! 발견한 것에 의미를 두자! 라고 생각을 하기로 했다.

정리해서 말하자면 @RequiredArgsConstructor 어노테이션의 특성이 생성자 주입 시 final선언을 해줘야 자동으로 생성자 주입을 하고 스프링 빈을 생성하는데, final이 없어 생성자 주입을 하지 않았기 때문에 각 Service들이 null상태로 있었던 것이고, 이러한 문제가 발생했었던 것이다.

기존 UserService.java 생성자주입 예시

private final UserRepository userRepository;
private final UserTagRepository userTagRepository;
private final FriendRepository friendRepository;

xxxxService로 변경했을 때 문제가 있던 코드 필드

private final UserRepository userRepository;
private UserTagService userTagService;
private FriendService friendService;

 또 만약에 또 이런 문제가 생겼을 때 디버그를 통해 필드객체가 null로 되어있다면 final선언을 했는지 한번 더 확인할 수 있는 계기가 되었다.